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 <egg...@cs.ucla.edu> > 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 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. But perhaps it's simpler to add "#undef recv" and "#undef select" near the top of the file, in addition to this change. Thanks, Paolo
