Re: [Qemu-devel] Re: [PATCH v2 0/5] iohandlers: Add support for enabling/disabling individual handlers

[Michael Roth <mdroth@linux.vnet.ibm.com>]

On 01/17/2011 08:57 AM, Anthony Liguori wrote:
> On 01/17/2011 04:18 AM, Amit Shah wrote:
>> On (Thu) Jan 13 2011 [13:17:22], Anthony Liguori wrote:
>>> On 01/13/2011 09:00 AM, Amit Shah wrote:
>>>> Hi,
>>>>
>>>> This patchset adds new interfaces to work with iohandlers. It adds:
>>>>
>>>> int assign_fd_handlers(int fd, IOHandlerOps *ops, void *opaque)
>>>> -- Specify io handlers for an fd
>>>> int remove_fd_handlers(int fd)
>>>> -- Remove fd handlers for fd (mark ioh for deletion)
>>>> int set_read_poll_fd_action(int fd, bool enable)
>>>> -- Enable or disable the fd_read_poll fd handler
>>>> int set_read_fd_action(int fd, bool enable)
>>>> -- Enable or disable the fd_read fd handler
>>>> int set_write_fd_action(int fd, bool enable)
>>>> -- Enable or disable the fd_read fd handler
>>>>
>>>> A new struct, IOHandlerOps, is added, to collect all the ops together
>>>> instead of passing individual ones to functions.
>>> Instead of inventing new interfaces, I think we should steal^Wlearn
>>> from established interfaces.
>> Agreed.
>>
>> I do also think it'll be worthwhile pulling in one of the libraries to
>> reduce the amount of qemu-specific code we have in the other cases as
>> well.
>>
>>> Both libevent and glib have interfaces
>>> that essentially boil down to:
>>>
>>> handle add_fd_event(loop, fd, ConditionMask, callback, opaque)
>>> remove_event(loop, handle)
>> This is quite similar to the Linux polling API.
>>
>> I don't know what the 'loop' parameter would do, though.
>
> It's to allow for multiple I/O threads. There's one loop context per
> thread and you can register events specifically to a thread.
>

In case you're interested, to make the fd handler stuff accessible to 
qemu tools I have some code that moves these interfaces into backend 
functions in another file, qemu-ioh.c, where the IOHandlerRecord list is 
supplied by the caller to track state.

This would extend naturally to threads by just having them maintain 
their own IOHandlerRecord lists as well, and stuff that is meant to 
interact with vl.c:io_handlers can be done via wrapper functions for 
backward-compatibility.

If that seems like a reasonable approach maybe you could leverage these 
patches remove some of the scope for your proposed changes:

http://repo.or.cz/w/qemu/mdroth.git/patch/56a391e85124382ddcbdb64dcc05aadfc965f000

http://repo.or.cz/w/qemu/mdroth.git/patch/f4428e2cdf5da231bb30ce1ed9eb5ce567a9dc55