[libvirt PATCH v2 06/13] util: introduce compat wrappers for Winsock2

Daniel P. Berrangé berrange at redhat.com
Fri Jan 17 09:38:42 UTC 2020


On Fri, Jan 17, 2020 at 10:27:05AM +0100, Pavel Hrdina wrote:
> On Thu, Jan 16, 2020 at 03:24:41PM +0000, Daniel P. Berrangé wrote:
> > Windows sockets take a SOCKET HANDLE object instead of a
> > file descriptor. Wrap them in the same way that gnulib
> > does so that they use C runtime file descriptors.
> > 
> > While we could in theory use GSocket, it is hard to get
> > the exact same semantics libvirt has for its current
> > socket usage. Wrapping the Winsock2 APIs is thus the
> > easiest approach in the short term.
> > 
> > Reviewed-by: Pavel Hrdina <phrdina at redhat.com>
> > Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> > ---
> >  src/util/Makefile.inc.am |   2 +
> >  src/util/virsocket.c     | 369 +++++++++++++++++++++++++++++++++++++++
> >  src/util/virsocket.h     |  92 ++++++++++
> >  src/util/virutil.c       |  29 ++-
> >  4 files changed, 488 insertions(+), 4 deletions(-)
> >  create mode 100644 src/util/virsocket.c
> >  create mode 100644 src/util/virsocket.h
> > 
> > diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am
> > index 23de4a6375..528c9f6cfe 100644
> > --- a/src/util/Makefile.inc.am
> > +++ b/src/util/Makefile.inc.am
> > @@ -188,6 +188,8 @@ UTIL_SOURCES = \
> >  	util/virseclabel.h \
> >  	util/virsecret.c \
> >  	util/virsecret.h \
> > +	util/virsocket.c \
> > +	util/virsocket.h \
> >  	util/virsocketaddr.c \
> >  	util/virsocketaddr.h \
> >  	util/virstorageencryption.c \
> > diff --git a/src/util/virsocket.c b/src/util/virsocket.c
> > new file mode 100644
> > index 0000000000..2d770b931b
> > --- /dev/null
> > +++ b/src/util/virsocket.c
> > @@ -0,0 +1,369 @@
> > +/*
> > + * Copyright (C) 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 "virsocket.h"
> > +
> > +#ifdef WIN32
> > +
> > +# include <fcntl.h>
> > +
> > +# define FD2SK(fd) _get_osfhandle(fd)
> > +# define SK2FD(sk) (_open_osfhandle((intptr_t) (sk), O_RDWR | O_BINARY))
> > +
> > +# define GET_HANDLE(fd) \
> > +
> > +# define RETURN_ERROR(call) \
> > +    if ((call) < 0) { \
> > +        set_errno(); \
> > +        return -1; \
> > +    }
> > +
> > +# undef accept
> > +# undef bind
> > +# undef closesocket
> > +# undef connect
> > +# undef dup
> > +# undef dup2
> 
> We are not calling these two here so probably no need to undef here.
> 
> > +# undef getpeername
> > +# undef getsockname
> > +# undef getsockopt
> > +# undef ioctlsocket
> > +# undef listen
> > +# undef setsockopt
> > +# undef socket
> 
> [...]
> 
> > diff --git a/src/util/virsocket.h b/src/util/virsocket.h
> > new file mode 100644
> > index 0000000000..e9ef5380a3
> > --- /dev/null
> > +++ b/src/util/virsocket.h
> > @@ -0,0 +1,92 @@
> > +/*
> > + * Copyright (C) 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/>.
> > + */
> > +
> > +#pragma once
> > +
> > +#include "internal.h"
> > +
> > +#ifdef WIN32
> > +
> > +# define WIN32_LEAN_AND_MEAN
> > +# include <errno.h>
> > +# include <winsock2.h>
> > +# include <ws2tcpip.h>
> > +# include <io.h>
> > +
> > +int vir_accept(int fd, struct sockaddr *addr, socklen_t *addrlen);
> > +int vir_bind(int fd, const struct sockaddr *addr, socklen_t addrlen);
> > +int vir_closesocket(int fd);
> > +int vir_connect(int fd, const struct sockaddr *addr, socklen_t addrlen);
> > +int vir_dup(int oldfd);
> > +int vir_dup2(int oldfd, int newfd);
> 
> These two are not implemented anywhere as you are using the _dup and
> _dup2 from io.h.

Opps, left over from earlier version when I did indeed replace dup/dup2.

> 
> > +int vir_getpeername(int fd, struct sockaddr *addr, socklen_t *addrlen);
> > +int vir_getsockname(int fd, struct sockaddr *addr, socklen_t *addrlen);
> > +int vir_listen(int fd, int backlog);
> > +int vir_ioctlsocket(int fd, int cmd, void *arg);
> > +int vir_getsockopt(int fd, int level, int optname,
> > +                   void *optval, socklen_t *optlen);
> > +int vir_setsockopt(int fd, int level, int optname,
> > +                   const void *optval, socklen_t optlen);
> > +int vir_socket(int domain, int type, int protocol);
> 
> One nit that I missed in v1, the function declaration doesn't follow
> syntax style where each argument should be on separate line, but I don't
> care that much for these wrappers.

We've generally ignored that in the .h files in practice.

> 
> > +
> > +
> > +/* Get rid of GNULIB's replacements */
> > +# undef accept
> > +# undef bind
> > +# undef closesocket
> > +# undef connect
> > +# undef dup
> > +# undef dup2
> > +# undef getpeername
> > +# undef getsockname
> > +# undef getsockopt
> > +# undef ioctlsocket
> > +# undef listen
> > +# undef setsockopt
> > +# undef socket
> > +
> > +/* Provide our own replacements */
> > +# define accept vir_accept
> > +# define bind vir_bind
> > +# define closesocket vir_closesocket
> > +# define connect vir_connect
> > +# define dup _dup
> > +# define dup2 _dup2
> > +# define ioctlsocket vir_ioctlsocket
> > +# define getpeername vir_getpeername
> > +# define getsockname vir_getsockname
> > +# define getsockopt vir_getsockopt
> > +# define listen vir_listen
> > +# define setsockopt vir_setsockopt
> > +# define socket vir_socket
> > +
> > +#else
> > +
> > +# include <sys/socket.h>
> > +# include <unistd.h>
> > +# include <sys/ioctl.h>
> > +# include <arpa/inet.h>
> > +# include <netinet/in.h>
> > +# include <netinet/udp.h>
> > +# include <netinet/tcp.h>
> > +# include <sys/un.h>
> > +
> > +# define closesocket close
> > +# define ioctlsocket ioctl
> > +
> > +#endif
> > diff --git a/src/util/virutil.c b/src/util/virutil.c
> > index a0fd7618ee..9ea9e2946d 100644
> > --- a/src/util/virutil.c
> > +++ b/src/util/virutil.c
> 
> Changes to this should be in a separate patch but otherwise it looks
> good.

I did have it separately before, but it breaks bisectability, because
the GNULIB socket wrappers & non-blocking impl are mutually dependant.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




More information about the libvir-list mailing list