10:42 a.m.
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(a)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(a)redhat.com>