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

[Anthony Liguori <anthony@codemonkey.ws>]

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.  Both libevent and glib have interfaces that 
essentially boil down to:

handle add_fd_event(loop, fd, ConditionMask, callback, opaque)
remove_event(loop, handle)

I think that's what we should move to.  All the stuff in our current 
loop around allowing suppressing of read events is terrible as it forces 
the main loop to poll.  That makes it impossible to use other main loops 
because it's completely unusual.

Regards,

Anthony Liguori

> The older function, qemu_set_fd_handler2(), is now a wrapper to
> assign_fd_handlers()  and can be deprecated by converting the existing
> usage to assign_fd_handlers().
>
> v2: Address comments from Gerd:
> - add comments to new interfaces
> - enable all specified handlers by default in assign_fd_handlers()
> - Add comments for TODO items on deprecation of older interfaces.
>
> Please apply.
>
> Amit Shah (5):
>    iohandlers: Avoid code duplication
>    iohandlers: Introduce assign_fd_handlers() and remove_fd_handlers
>    iohandlers: Allow each iohandler to be enabled/disabled individually
>    iohandlers: Enable an iohandler only if the associated handler exists
>    iohandlers: Add IOHandlerOps struct
>
>   qemu-char.h |    7 ++
>   vl.c        |  197 ++++++++++++++++++++++++++++++++++++++++++++++++----------
>   2 files changed, 170 insertions(+), 34 deletions(-)
>
>