From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39783) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1acsG4-0000QQ-F4 for qemu-devel@nongnu.org; Mon, 07 Mar 2016 05:19:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1acsG0-0005GH-1s for qemu-devel@nongnu.org; Mon, 07 Mar 2016 05:19:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56488) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1acsFz-0005G1-Qp for qemu-devel@nongnu.org; Mon, 07 Mar 2016 05:19:19 -0500 Date: Mon, 7 Mar 2016 10:19:14 +0000 From: "Daniel P. Berrange" Message-ID: <20160307101914.GA13034@redhat.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] broken socket events on win32 qemu Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andrew Baumann Cc: Stefan Weil , QEMU Developers , Paolo Bonzini On Mon, Mar 07, 2016 at 07:23:12AM +0000, Andrew Baumann wrote: > Hi Daniel, >=20 > This commit ("char: convert from GIOChannel to QIOChannel"): > https://github.com/qemu/qemu/commit/9894dc0cdcc397ee5b26370bc53da6d360a= 363c2 > ... 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. >=20 > Without having looked very closely at the code, I suspect the problem m= ay > be that we've lost the special-case treatment of socket handles as dist= inct > from file descriptors on Win32 (they are different namespaces, and diff= erent > APIs are needed). The previous version of qemu-char.c special-cased soc= kets > in io_channel_from_socket(): >=20 > -#ifdef _WIN32 > - chan =3D g_io_channel_win32_new_socket(fd); > -#else > - chan =3D g_io_channel_unix_new(fd); > -#endif >=20 > ... but I don't see anything equivalent in io/channel-socket.c. Am I lo= oking > 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 socket= s > 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 potential= ly problematic=C2=B7 The QEMU socket_error() method is quite crude - it sim= ply 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 =3D=3D Exxxx values, whi= ch 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 --=20 |: http://berrange.com -o- http://www.flickr.com/photos/dberrange= / :| |: http://libvirt.org -o- http://virt-manager.or= g :| |: http://autobuild.org -o- http://search.cpan.org/~danberr= / :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vn= c :|