Re: [Qemu-devel] [PATCH 2/2] main: switch qemu_set_fd_handler to g_io_add_watch

[Paolo Bonzini <pbonzini@redhat.com>]

On 08/22/2011 03:47 PM, Paolo Bonzini wrote:
> On 08/22/2011 03:45 PM, Anthony Liguori wrote:
>>>
>>> Almost: in Win32 you need to use g_io_channel_win32_new_socket. But
>>> indeed on Windows you can only use qemu_set_fd_handler for sockets too.
>>
>> I think that's really only for read/write though. If you're just
>> polling on I/O, it shouldn't matter IIUC.
>>
>> If someone has a Windows box, they can confirm/deny by using qemu
>> -monitor tcp:localhost:1024,socket,nowait with this patch.
>
> Actually you're right, it works automagically:
>
> * On Win32, this can be used either for files opened with the MSVCRT
> * (the Microsoft run-time C library) _open() or _pipe, including file
> * descriptors 0, 1 and 2 (corresponding to stdin, stdout and stderr),
> * or for Winsock SOCKETs. If the parameter is a legal file
> * descriptor, it is assumed to be such, otherwise it should be a
> * SOCKET. This relies on SOCKETs and file descriptors not
> * overlapping. If you want to be certain, call either
> * g_io_channel_win32_new_fd() or g_io_channel_win32_new_socket()
> * instead as appropriate.
>
> So this patch would even let interested people enable exec migration on
> Windows.

Hmmm, after reading documentation better, this unfortunately is 
completely broken under Windows, for two reasons:

1) in patch 1/2 you're using the glib pollfds and passing them to 
select().  Unfortunately under Windows they are special and can only be 
passed to g_poll().  Unfortunately, this can be fixed by changing the 
QEMU main loop to use poll() instead of select()...

2) ... because glib IO channels cannot be used just for watches under 
Windows:

/* Create an IO channel for C runtime (emulated Unix-like) file
  * descriptors. After calling g_io_add_watch() on a IO channel
  * returned by this function, you shouldn't call read() on the file
  * descriptor. This is because adding polling for a file descriptor is
  * implemented on Win32 by starting a thread that sits blocked in a
  * read() from the file descriptor most of the time. All reads from
  * the file descriptor should be done by this internal GLib
  * thread. Your code should call only g_io_channel_read().
  */

So, I believe the right solution would be to drop this patch for now and 
make 1/2 conditional on !_WIN32.

Paolo