Re: [Qemu-devel] [RFC] Convert AioContext to Gsource sub classes

[Michael Roth]

Quoting Paolo Bonzini (2013-08-12 02:30:28)
> > 1) rename AioContext to AioSource.
> >   This is my major purpose, which declare it is not a "context" concept,
> > and GMainContext is the entity represent the thread's activity.
> 
> Note that the nested event loops in QEMU are _very_ different from
> glib nested event loops.  In QEMU, nested event loops only run block
> layer events.  In glib, they run all events.  That's why you need
> AioContext.

Would it be possible to use glib for our nested loops as well by just
setting a higher priority for the AioContext GSource?

Stefan and I were considering how we could make use of his "drop
ioflush" patches to use a common mechanism to register fd events, but
still allow us to distinguish between AioContext and non-AioContext
for nested loops. I was originally thinking of using prepare() functions
to filter out non-AioContext events, but that requires we implement
on GSource's with that in mind, and non make use of pre-baked ones
like GIOChannel's, and bakes block stuff into every event source
implementation.

Priorities didn't cross my mind though, but it seems pretty
straightfoward...

AioContext could then just be a container of sorts for managing
bottom-halves and AioContext FDs and binding them to the proper
GMainContext/MainLoop, but the underlying GSources could
still be driven by a normal glib-based mainloop, just with a specific
priority in the nested case.

> 
> > 2) Break AioSource into FdSource and BhSource.
> >   This make custom code less and simpler, one Gsource for one kind of
> > job. It is not necessary but IMHO it will make things clear when add
> > more things into main loop: add a new Gsource sub class, avoid to
> > always have relationship with AioContext.
> 
> But this is only complicating things work since users rely on both file-
> descriptor APIs and bottom half APIs.

Taking things a step further, maybe AioContext can stop being a
block-specific construct, but actually be the "QContext" we've
discussed in the past for managing multiple event loops. All
the block stuff would be hidden away in the GSource priority.

For instance,

#ifndef _WIN32

qemu_aio_set_fd_handler(fd, ...):
    aio_set_fd_handler(qemu_aio_context, fd, ..., QEMU_PRIORITY_BLOCK)

qemu_set_fd_handler(fd, ...):
    aio_set_fd_handler(qemu_aio_context, fd, ..., G_PRIORITY_DEFAULT)

#else

qemu_add_wait_object(fd, ...):
    add_wait_object(qemu_aio_context, fd, ...)

qemu_set_fd_handler(fd, ...):
    set_socket_handler(qemu_aio_context, fd, ..., G_PRIORITY_DEFAULT)

#endif

qemu_bh_schedule:
    bh_schedule(qemu_aio_context, ...)

etc...

I'll be sending patches this week for moving
add_wait_object/qemu_set_fd_handler to GSources, the non-global ones use
GMainContext * to specify a non-default thread/context, but can be easily
changed, or we can just do aioctx->g_main_context at the call sites.
There's some nice possibilities in using the former though: avoiding
O(n) lookups for stuff like finding the GSource for a particular
event/event type, for instance, by storing pointers to the GSource or
some kind of hashmap lookup. But probably better to discuss that aspect
with some context so I'll try to get those patches out soon.

> 
> > >>    More reasons:
> > >>    When I thinking how to bind library code to a thread context, it may
> > >> need to add Context's concept into API of block.c. If I use AioContext,
> > >> there will need a wrapper API to run the event loop. But If I got
> > >> glib's GmainContext, things become simple.
> 
> You already have it because AioContext is a GSource.  You do not need
> to expose the AioContext, except as a GSource.
> 
> Paolo