[libvirt PATCH v2 33/56] src: introduce helper API for creating GSource for socket
Pavel Hrdina
phrdina at redhat.com
Wed Jan 29 16:42:52 UTC 2020
On Wed, Jan 29, 2020 at 04:34:46PM +0000, Daniel P. Berrangé wrote:
> On Wed, Jan 29, 2020 at 05:33:20PM +0100, Pavel Hrdina wrote:
> > On Tue, Jan 28, 2020 at 01:11:14PM +0000, Daniel P. Berrangé wrote:
> > > We need to be able to create event loop watches using the
> > > GSource API for sockets. GIOChannel is able todo this, but
> > > we don't want to use the GIOChannel APIs for reading/writing,
> > > and testing shows just using its GSource APIs is unreliable
> > > on Windows.
> > >
> > > This patch thus creates a standalone helper API for creating
> > > a GSource for a socket file descriptor. This impl is derived
> > > from code in QEMU's io/channel-watch.c file that was written
> > > by myself & Paolo Bonzini & thus under Red Hat copyright.
> > >
> > > Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> > > ---
> > > build-aux/syntax-check.mk | 3 +
> > > src/util/Makefile.inc.am | 2 +
> > > src/util/vireventglibwatch.c | 248 +++++++++++++++++++++++++++++++++++
> > > src/util/vireventglibwatch.h | 48 +++++++
> > > 4 files changed, 301 insertions(+)
> > > create mode 100644 src/util/vireventglibwatch.c
> > > create mode 100644 src/util/vireventglibwatch.h
> > >
> > > diff --git a/src/util/vireventglibwatch.c b/src/util/vireventglibwatch.c
> > > new file mode 100644
> > > index 0000000000..f7b087e2ec
> > > --- /dev/null
> > > +++ b/src/util/vireventglibwatch.c
> > > @@ -0,0 +1,248 @@
> > > +/*
> > > + * vireventglibwatch.c: GSource impl for sockets
> > > + *
> > > + * Copyright (C) 2015-2020 Red Hat, Inc.
> > > + *
> > > + * This library is free software; you can redistribute it and/or
> > > + * modify it under the terms of the GNU Lesser General Public
> > > + * License as published by the Free Software Foundation; either
> > > + * version 2.1 of the License, or (at your option) any later version.
> > > + *
> > > + * This library is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > > + * Lesser General Public License for more details.
> > > + *
> > > + * You should have received a copy of the GNU Lesser General Public
> > > + * License along with this library. If not, see
> > > + * <http://www.gnu.org/licenses/>.
> > > + */
> > > +
> > > +#include <config.h>
> > > +
> > > +#include "vireventglibwatch.h"
> > > +
> > > +#ifndef WIN32
> > > +typedef struct virEventGLibFDSource virEventGLibFDSource;
> > > +struct virEventGLibFDSource {
> > > + GSource parent;
> > > + GPollFD pollfd;
> > > + int fd;
> > > + GIOCondition condition;
> > > +};
> > > +
> > > +
> > > +static gboolean
> > > +virEventGLibFDSourcePrepare(GSource *source G_GNUC_UNUSED,
> > > + gint *timeout)
> > > +{
> > > + *timeout = -1;
> > > +
> > > + return FALSE;
> > > +}
> > > +
> > > +
> > > +static gboolean
> > > +virEventGLibFDSourceCheck(GSource *source)
> > > +{
> > > + virEventGLibFDSource *ssource = (virEventGLibFDSource *)source;
> > > +
> > > + return ssource->pollfd.revents & ssource->condition;
> > > +}
> > > +
> > > +
> > > +static gboolean
> > > +virEventGLibFDSourceDispatch(GSource *source,
> > > + GSourceFunc callback,
> > > + gpointer user_data)
> > > +{
> > > + virEventGLibSocketFunc func = (virEventGLibSocketFunc)callback;
> > > + virEventGLibFDSource *ssource = (virEventGLibFDSource *)source;
> > > +
> > > + return (*func)(ssource->fd,
> > > + ssource->pollfd.revents & ssource->condition,
> > > + user_data);
> > > +}
> > > +
> > > +
> > > +static void
> > > +virEventGLibFDSourceFinalize(GSource *source G_GNUC_UNUSED)
> > > +{
> > > +}
> > > +
> > > +
> > > +GSourceFuncs virEventGLibFDSourceFuncs = {
> > > + .prepare = virEventGLibFDSourcePrepare,
> > > + .check = virEventGLibFDSourceCheck,
> > > + .dispatch = virEventGLibFDSourceDispatch,
> > > + .finalize = virEventGLibFDSourceFinalize
> > > +};
> > > +
> > > +
> > > +GSource *virEventGLibCreateSocketWatch(int fd,
> > > + GIOCondition condition)
> > > +{
> > > + GSource *source;
> > > + virEventGLibFDSource *ssource;
> > > +
> > > + source = g_source_new(&virEventGLibFDSourceFuncs,
> > > + sizeof(virEventGLibFDSource));
> > > + ssource = (virEventGLibFDSource *)source;
> > > +
> > > + ssource->condition = condition;
> > > + ssource->fd = fd;
> > > +
> > > + ssource->pollfd.fd = fd;
> > > + ssource->pollfd.events = condition;
> > > +
> > > + g_source_add_poll(source, &ssource->pollfd);
> > > +
> > > + return source;
> > > +}
> > > +
> > > +#else /* WIN32 */
> > > +
> > > +# define WIN32_LEAN_AND_MEAN
> > > +# include <winsock2.h>
> > > +
> > > +typedef struct virEventGLibSocketSource virEventGLibSocketSource;
> > > +struct virEventGLibSocketSource {
> > > + GSource parent;
> > > + GPollFD pollfd;
> > > + int fd;
> > > + SOCKET socket;
> > > + HANDLE event;
> > > + int revents;
> > > + GIOCondition condition;
> > > +};
> > > +
> > > +
> > > +static gboolean
> > > +virEventGLibSocketSourcePrepare(GSource *source G_GNUC_UNUSED,
> > > + gint *timeout)
> > > +{
> > > + *timeout = -1;
> > > +
> > > + return FALSE;
> > > +}
> > > +
> > > +
> > > +/*
> > > + * NB, this impl only works when the socket is in non-blocking
> > > + * mode on Win32
> > > + */
> > > +static gboolean
> > > +virEventGLibSocketSourceCheck(GSource *source)
> > > +{
> > > + static struct timeval tv0;
> > > +
> > > + virEventGLibSocketSource *ssource = (virEventGLibSocketSource *)source;
> > > + WSANETWORKEVENTS ev;
> > > + fd_set rfds, wfds, xfds;
> > > +
> > > + if (!ssource->condition)
> > > + return 0;
> > > +
> > > + WSAEnumNetworkEvents(ssource->socket, ssource->event, &ev);
> > > +
> > > + FD_ZERO(&rfds);
> > > + FD_ZERO(&wfds);
> > > + FD_ZERO(&xfds);
> > > + if (ssource->condition & G_IO_IN)
> > > + FD_SET(ssource->socket, &rfds);
> > > + if (ssource->condition & G_IO_OUT)
> > > + FD_SET(ssource->socket, &wfds);
> > > + if (ssource->condition & G_IO_PRI)
> > > + FD_SET(ssource->socket, &xfds);
> > > +
> > > + ssource->revents = 0;
> > > + if (select(0, &rfds, &wfds, &xfds, &tv0) == 0)
> > > + return 0;
> > > +
> > > + if (FD_ISSET(ssource->socket, &rfds))
> > > + ssource->revents |= G_IO_IN;
> > > +
> > > + if (FD_ISSET(ssource->socket, &wfds))
> > > + ssource->revents |= G_IO_OUT;
> > > +
> > > + if (FD_ISSET(ssource->socket, &xfds))
> > > + ssource->revents |= G_IO_PRI;
> > > +
> > > + return ssource->revents;
> > > +}
> > > +
> > > +
> > > +static gboolean
> > > +virEventGLibSocketSourceDispatch(GSource *source,
> > > + GSourceFunc callback,
> > > + gpointer user_data)
> > > +{
> > > + virEventGLibSocketFunc func = (virEventGLibSocketFunc)callback;
> > > + virEventGLibSocketSource *ssource = (virEventGLibSocketSource *)source;
> > > +
> > > + return (*func)(ssource->fd, ssource->revents, user_data);
> > > +}
> > > +
> > > +
> > > +static void
> > > +virEventGLibSocketSourceFinalize(GSource *source)
> > > +{
> > > + virEventGLibSocketSource *ssource = (virEventGLibSocketSource *)source;
> > > +
> > > + WSAEventSelect(ssource->socket, NULL, 0);
> > > +}
> > > +
> > > +
> > > +GSourceFuncs virEventGLibSocketSourceFuncs = {
> > > + .prepare = virEventGLibSocketSourcePrepare,
> > > + .check = virEventGLibSocketSourceCheck,
> > > + .dispatch = virEventGLibSocketSourceDispatch,
> > > + .finalize = virEventGLibSocketSourceFinalize
> > > +};
> > > +
> > > +
> > > +GSource *virEventGLibCreateSocketWatch(int fd,
> > > + GIOCondition condition)
> > > +{
> > > + GSource *source;
> > > + virEventGLibSocketSource *ssource;
> > > +
> > > + source = g_source_new(&virEventGLibSocketSourceFuncs,
> > > + sizeof(virEventGLibSocketSource));
> > > + ssource = (virEventGLibSocketSource *)source;
> > > +
> > > + ssource->condition = condition;
> > > + ssource->fd = fd;
> > > + ssource->socket = _get_osfhandle(fd);
> > > + ssource->event = CreateEvent(NULL, FALSE, FALSE, NULL);
> >
> > Based on going through QEMU code and windows documentation it looks
> > like we have to free the event using CloseHandle().
> >
> > QEMU is doing it in io/channel.c in qio_channel_finalize() which is
> > probably called by unrefing ssource->ioc in
> > qio_channel_fd_pair_source_finalize().
> >
> > That would mean we have to call CloseHandle() in
> > virEventGLibSocketSourceFinalize() to make sure we will not leak any
> > resources.
>
> Yes, you are correct.
With that fixed and a hopefully nothing else was missed
Reviewed-by: Pavel Hrdina <phrdina at redhat.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200129/26505955/attachment-0001.sig>
More information about the libvir-list
mailing list