[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: gnulib poll() returns POLLERR on Win32 platform, breaking libvirt
From: |
Daniel P. Berrange |
Subject: |
Re: gnulib poll() returns POLLERR on Win32 platform, breaking libvirt |
Date: |
Wed, 19 Apr 2017 16:50:11 +0100 |
User-agent: |
Mutt/1.7.1 (2016-10-04) |
On Wed, Apr 19, 2017 at 05:36:54PM +0200, Paolo Bonzini wrote:
>
>
> On 18/04/2017 17:07, Daniel P. Berrange wrote:
> > We're seeing a failure in libvirt on Win32 platforms whereby poll() often
> > returns POLLERR. I traced this down to the rpl_recv() function calling
> > FD_TO_SOCKET() and getting INVALID_SOCKET back. This is propagated back
> > to windows_compute_revents_socket() which sets POLLERR, because
> > WSAGetLastError() returns 0.
> >
> > The windows_compute_revents_socket() is indeed passing a bad file descriptor
> > to recv, because it is actually passing in a SOCKET handle.
> >
> > After bisecting gnulib, I found the root cause of this problem is this
> > commit:
> >
> > commit 17f1e64f00011fb745019119e21b26e4aba65e4b
> > Author: Paul Eggert <address@hidden>
> > Date: Tue Feb 24 16:16:19 2015 -0800
> >
> > poll: port to MSVC v18 on MS-Windows 8.1
> >
> > Problem reported by Gisle Vanem in:
> > http://lists.gnu.org/archive/html/bug-gnulib/2015-02/msg00139.html
> > * lib/poll.c: Always include <sys/select.h> and <sys/socket.h>.
> > * modules/poll (Depends-on) [!HAVE_POLL || REPLACE_POLL]:
> > Add sys_socket.
> >
> >
> > by unconditionally pulling in sys/select.c & sys/socket.c, we are causing
> > poll() to call gnulib's replacement rpl_recv() wrapper, rather than the
> > original bare Winsock recv() method.
> >
> > Reverting that change fixes libvirt, but obviously that's not a valid
> > approach, since it'll re-break MSVC v18 on Win8.1
> >
> >
> > So I tried the following change to make it pass in an FD into the rpl_recv
> > method instead of a HANDLE
> >
> > @@ -571,7 +568,7 @@
> > if (FD_ISSET ((SOCKET) h, &xfds))
> > ev.lNetworkEvents |= FD_OOB;
> >
> > - happened = windows_compute_revents_socket (pfd[i].fd,
> > pfd[i].events,
> > + happened = windows_compute_revents_socket ((SOCKET) h,
> > pfd[i].events,
> > ev.lNetworkEvents);
> > }
> > else
>
> You mean you applied the reverse of this patch, of course? For a
Sigh, yes.
> complete patch you'd also change windows_compute_revents_socket's first
> argument to "int fd", and change it to use errno instead of WSAGetLastError.
Yep, I changed the arg, but didn't try the WSAGetLastError change.
> But perhaps it's simpler to add "#undef recv" and "#undef select" near
> the top of the file, in addition to this change.
If we add the undef's then we don't need to change anything related to
windows_compute_revents_socket() as it'd be calling the native recv()
which expects the SOCKET rather than fd.
If we undef those functions, would that not re-introduce the bug mentioned
in http://lists.gnu.org/archive/html/bug-gnulib/2015-02/msg00139.html ?
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 :|