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

[Anthony Liguori <anthony@codemonkey.ws>]

On 09/06/2011 09:31 AM, Paolo Bonzini wrote:
> 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()...

Hrm, okay.

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

So it should be possible to add a new Source type that just selects on a 
file descriptor and avoid GIOChannels?

Regards,

Anthony Liguori

>
> Paolo