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

Pavel Hrdina phrdina at redhat.com
Fri Jan 17 09:42:30 UTC 2020 

On Fri, Jan 17, 2020 at 09:38:42AM +0000, Daniel P. Berrangé wrote:
> 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.

I see, in that case with the dup/dup2 fixes:

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/20200117/335a7827/attachment-0001.sig>

More information about the libvir-list mailing list