Re: [Qemu-devel] [PATCH v2 1/2] main-loop: Pass AioContext into qemu_poll_ns

[Stefan Hajnoczi <stefanha@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1759 bytes --]

On Tue, Sep 30, 2014 at 11:35:17AM +0800, Fam Zheng wrote:
> diff --git a/main-loop.c b/main-loop.c
> index d2e64f1..4641ef4 100644
> --- a/main-loop.c
> +++ b/main-loop.c
> @@ -234,7 +234,8 @@ static int os_host_main_loop_wait(int64_t timeout)
>          spin_counter++;
>      }
>  
> -    ret = qemu_poll_ns((GPollFD *)gpollfds->data, gpollfds->len, timeout);
> +    ret = qemu_poll_ns(qemu_aio_context, (GPollFD *)gpollfds->data,
> +                       gpollfds->len, timeout);
>  
>      if (timeout) {
>          qemu_mutex_lock_iothread();
> @@ -439,7 +440,8 @@ static int os_host_main_loop_wait(int64_t timeout)
>      poll_timeout_ns = qemu_soonest_timeout(poll_timeout_ns, timeout);
>  
>      qemu_mutex_unlock_iothread();
> -    g_poll_ret = qemu_poll_ns(poll_fds, n_poll_fds + w->num, poll_timeout_ns);
> +    g_poll_ret = qemu_poll_ns(qemu_aio_context,
> +                              poll_fds, n_poll_fds + w->num, poll_timeout_ns);
>  
>      qemu_mutex_lock_iothread();
>      if (g_poll_ret > 0) {

This is a hack.

The immediate problem is that we're not holding the QEMU global mutex
but are accessing qemu_aio_context.  What if other threads touch
qemu_aio_context while they hold the QEMU global mutex?

You didn't indicate what thread-safety rules need to be obeyed so I
wonder whether you looked into this.

I'm also concerned that this is somewhat abusing AioContext.  You want
to stash fields in there but this poll call is about more than just
AioContext, it also includes non-AioContext file descriptors.  So it
seems like a kludge to put the fields into AioContext.

Can you put the state into a separate struct so it's easy to understand
its thread-safety characteristics?

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]