Re: [Qemu-devel] [PATCH] [RFC] Add glib support to main loop

[Anthony Liguori <aliguori@us.ibm.com>]

On 07/27/2011 03:43 PM, Blue Swirl wrote:
> On Wed, Jul 27, 2011 at 3:06 AM, Anthony Liguori<aliguori@us.ibm.com>  wrote:
>> This allows GSources to be used to register callback events in QEMU.  This is
>> useful as it allows us to take greater advantage of glib and also because it
>> allows us to write code that is more easily testable outside of QEMU since we
>> can make use of glib's main loop in unit tests.
>>
>> All new code should use glib's callback mechanisms for registering fd events
>> which are very well documented at:
>>
>> http://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html
>>
>> And:
>>
>> http://developer.gnome.org/gio/stable/
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> ---
>>   vl.c |   74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 74 insertions(+), 0 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 4b6688b..19774ac 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -111,6 +111,8 @@ int main(int argc, char **argv)
>>   #define main qemu_main
>>   #endif /* CONFIG_COCOA */
>>
>> +#include<glib.h>
>> +
>>   #include "hw/hw.h"
>>   #include "hw/boards.h"
>>   #include "hw/usb.h"
>> @@ -1309,6 +1311,75 @@ void qemu_system_vmstop_request(int reason)
>>      qemu_notify_event();
>>   }
>>
>> +static GPollFD poll_fds[1024 * 2]; /* this is probably overkill */
>> +static int n_poll_fds;
>> +static int max_priority;
>> +
>> +static void glib_select_fill(int *max_fd, fd_set *rfds, fd_set *wfds,
>> +                             fd_set *xfds, struct timeval *tv)
>> +{
>> +    GMainContext *context = g_main_context_default();
>> +    int i;
>> +    int timeout = 0, cur_timeout;
>> +
>> +    g_main_context_prepare(context,&max_priority);
>> +
>> +    n_poll_fds = g_main_context_query(context, max_priority,&timeout,
>> +                                      poll_fds, ARRAY_SIZE(poll_fds));
>> +    g_assert(n_poll_fds<= ARRAY_SIZE(poll_fds));
>
> The use of g_assert() means that production code should define
> G_DISABLE_ASSERT in addition to NDEBUG. Should we make both default
> for stable versions?

Oh, this is just me being lazy.  There's a way to do this without using 
the assert.  I put the assert there to make sure that I didn't leave a 
latent bug.

But in general, I don't think we should ever define NDEBUG.

>> +
>> +    for (i = 0; i<  n_poll_fds; i++) {
>> +        GPollFD *p =&poll_fds[i];
>> +
>> +        if ((p->events&  G_IO_IN)) {
>> +            FD_SET(p->fd, rfds);
>> +            *max_fd = MAX(*max_fd, p->fd);
>> +        }
>> +        if ((p->events&  G_IO_OUT)) {
>> +            FD_SET(p->fd, wfds);
>> +            *max_fd = MAX(*max_fd, p->fd);
>> +        }
>> +        if ((p->events&  G_IO_ERR)) {
>> +            FD_SET(p->fd, xfds);
>> +            *max_fd = MAX(*max_fd, p->fd);
>> +        }
>> +    }
>> +
>> +    cur_timeout = (tv->tv_sec * 1000) + ((tv->tv_usec + 500) / 1000);
>> +    if (timeout>= 0&&  timeout<  cur_timeout) {
>> +        tv->tv_sec = timeout / 1000;
>> +        tv->tv_usec = (timeout % 1000) * 1000;
>> +    }
>> +}
>> +
>> +static void glib_select_poll(fd_set *rfds, fd_set *wfds, fd_set *xfds,
>> +                             bool err)
>> +{
>> +    GMainContext *context = g_main_context_default();
>> +
>> +    if (!err) {
>> +        int i;
>> +
>> +        for (i = 0; i<  n_poll_fds; i++) {
>> +            GPollFD *p =&poll_fds[i];
>> +
>> +            if ((p->events&  G_IO_IN)&&  FD_ISSET(p->fd, rfds)) {
>> +                p->revents |= G_IO_IN;
>> +            }
>> +            if ((p->events&  G_IO_OUT)&&  FD_ISSET(p->fd, wfds)) {
>> +                p->revents |= G_IO_OUT;
>> +            }
>> +            if ((p->events&  G_IO_ERR)&&  FD_ISSET(p->fd, xfds)) {
>> +                p->revents |= G_IO_ERR;
>> +            }
>> +        }
>> +    }
>> +
>> +    if (g_main_context_check(context, max_priority, poll_fds, n_poll_fds)) {
>> +        g_main_context_dispatch(context);
>> +    }
>> +}
>> +
>
> The above functions do not seem to be referenced anywhere.

They are in the code below :-)

>>   void main_loop_wait(int nonblocking)
>>   {
>>      fd_set rfds, wfds, xfds;
>> @@ -1334,8 +1405,10 @@ void main_loop_wait(int nonblocking)
>>      FD_ZERO(&rfds);
>>      FD_ZERO(&wfds);
>>      FD_ZERO(&xfds);
>> +
>>      qemu_iohandler_fill(&nfds,&rfds,&wfds,&xfds);
>>      slirp_select_fill(&nfds,&rfds,&wfds,&xfds);
>> +    glib_select_fill(&nfds,&rfds,&wfds,&xfds,&tv);
>>
>>      qemu_mutex_unlock_iothread();
>>      ret = select(nfds + 1,&rfds,&wfds,&xfds,&tv);
>> @@ -1343,6 +1416,7 @@ void main_loop_wait(int nonblocking)
>>
>>      qemu_iohandler_poll(&rfds,&wfds,&xfds, ret);
>>      slirp_select_poll(&rfds,&wfds,&xfds, (ret<  0));
>> +    glib_select_poll(&rfds,&wfds,&xfds, (ret<  0));
>>
>>      qemu_run_all_timers();

Regards,

Anthony Liguori

>> --
>> 1.7.4.1
>>
>>
>>