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

[Paolo Bonzini]

Il 10/08/2013 05:24, Wenchao Xia ha scritto:
> Hi folks,
>   I'd like form a series which remove AioContext's concept and
> bind to glib's main loop more closely. Since changed place will be
> a bit much so want to know your opinion before real coding:

I'm not sure I understand...  What does it buy you to split AioContext
this way?  First, BhSource and FdSource are needed by block drivers, and
BhSource needs the notifier to interrupt the main loop.  Second,
AioContext is _already_ a GSource exactly to integrate closely with
GLib's main loop.  Look at the series that introduced AioContext back in
October 2012.  The main AioContext is already added as a GSource to the
iothread's main loop; aio_wait is used in dataplane for simplicity, but
it could also use a separate GMainLoop and add AioContext there as a
GSource.

Paolo

> changes:
> **before patch:
> typedef struct AioContext {
>     GSource source;
>     int walking_handlers;
>     QemuMutex bh_lock;
>     struct QEMUBH *first_bh;
>     int walking_bh;
>     EventNotifier notifier;
>     GArray *pollfds;
>     struct ThreadPool *thread_pool;
> } AioContext;
> 
> **After patch:
> typedef struct BhSource {
>     GSource source;
>     QemuMutex bh_lock;
>     struct QEMUBH *first_bh;
>     int walking_bh;
> } BhSource;
> 
> typedef struct FdSource {
>     GSource source;
>     int walking_handlers;
>     EventNotifier notifier;
>     GArray *pollfds;
>     struct ThreadPool *thread_pool;
> } FdSource;
> 
> Benefits:
>   Original code have a mix of Gsource and GMainContext's concept, we
> may want to add wrapper functions around GMainContext's functions, such
> as g_main_context_acquire(), g_main_context_prepare(), which brings
> extra effort if you want to form a good and clear API layer. With
> this changes, all qemu's custom code is attached under Gsource, we
> have a clear GMainContext's API layer for event loop, no wrapper is
> needed, and the event's loop API is glib's API, a clear layer let
> me form a library or adding more things.
> 
> before:
> qemu's mainloop caller,  BH user, fd user
>                       |
>                    AioContext
>                       |
>                 GMainContext
> 
> 
> after:
> qemu's mainloop caller
>     |                                   BH user    fd user
> GmainContext                               |         |
>     |--------------------------------|--BhSource     |
>                                      |-------------FdSource
> 
> 
> Note:
>   FdSource could be split more into ThreadSource and FdSource, which
> distinguish more. It can be done easily if the change of this series
> is online, when found necessary.
> 
>   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.