Re: [Qemu-devel] broken socket events on win32 qemu

[Daniel P. Berrange]

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 :|