On Mon, Mar 07, 2016 at 07:23:12AM +0000, Andrew Baumann wrote: > Hi Daniel, > > This commit ("char: convert from GIOChannel to QIOChannel"): > https://github.com/qemu/qemu/commit/9894dc0cdcc397ee5b26370bc53da6d360a363c2 > ... appears to have broken socket events for character devices on Win32. > For example, I can no longer connect to a GDB stub (started with: > "-gdb tcp:127.0.0.1:1234"), since tcp_chr_accept is never called. > > Without having looked very closely at the code, I suspect the problem may > be that we've lost the special-case treatment of socket handles as distinct > from file descriptors on Win32 (they are different namespaces, and different > APIs are needed). The previous version of qemu-char.c special-cased sockets > in io_channel_from_socket(): > > -#ifdef _WIN32 > - chan = g_io_channel_win32_new_socket(fd); > -#else > - chan = g_io_channel_unix_new(fd); > -#endif > > ... but I don't see anything equivalent in io/channel-socket.c. Am I looking > in the wrong place?
No, you are correct, this is broken for the reason you describe. Seems this is the one key feature I forgot to add unit test coverage for :-( > BTW, The same change introduces another problem on win32: server sockets > like the GDB example above fail on getpeername() with "Unable to query > remote socket address: Unknown error". This seems to be caused by a > definition of ENOTCONN that is not WSAENOTCONN. I'm still trying to > figure out why that is, and how to best fix it. Can you say how you are building QEMU ? Are you using mingw to do a cross compile for Win32, or something else ? Looking at my local Mingw64 install, the errno definitions look potentially problematic· The QEMU socket_error() method is quite crude - it simply expands to WSAGetLastError(), so any code calling it is assuming that the WSAExxxxx constants match the Exxxx constants. QEMU has a header which sets up such a mapping, but it only does so conditionally. eg in include/sysemu/os-win32.h #ifndef ENOTCONN # define ENOTCONN WSAENOTCONN #endif The current versions of mingw64 I have installed though has a winerror.h which defines #define WSABASEERR 10000 #define WSAENOTCONN (WSABASEERR + 57) And a separate errno.h that defines #ifndef ENOTCONN #define ENOTCONN 126 #endif This obviously does not match the WSAENOTCONN value So my guess would be that QEMU is pulling in the mingw64 errno.h values and so QEMU's own os-win32.h hack is not getting activated. Really, I think the problem is QEMU's socket_error() compat wrapper. It is fundamentally not reliable to assume WSAExxxx == Exxxx values, which is what socket_error() forces callers todo. I think we should we re-implement socket_error() for win32 to do this int socket_error(void) { switch (WSAGetLastError()) { case WSAENOTCONN: return ENOTCONN; case WSAECONNREFUSED: return ECONNREFUSED; case WSAEBADF: return EBADF; ....etc for the other errno mpappings w eneed to care about... } } this is what GLib itself does internally for addressing this problem (see g_io_error_from_win32_error in gio/gioerror.c) Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|