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@xxxxxxxxxx> > > > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx> > > > --- > > > 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@xxxxxxxxxx>
Attachment:
signature.asc
Description: PGP signature
