Re: [libvirt PATCH v2 33/56] src: introduce helper API for creating GSource for socket

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(a)redhat.com&gt; > --- > 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.