Re: [PULL 09/14] qmp: Move dispatcher to a coroutine

[Kevin Wolf]

Am 17.10.2020 um 10:15 hat Volker Rümelin geschrieben:
> > From: Kevin Wolf <kwolf@redhat.com>
> >
> > This moves the QMP dispatcher to a coroutine and runs all QMP command
> > handlers that declare 'coroutine': true in coroutine context so they
> > can avoid blocking the main loop while doing I/O or waiting for other
> > events.
> >
> > For commands that are not declared safe to run in a coroutine, the
> > dispatcher drops out of coroutine context by calling the QMP command
> > handler from a bottom half.
> 
> Hi Kevin,
> 
> since commit 9ce44e2ce2 "qmp: Move dispatcher to a coroutine" I see
> the following error on Windows whenever I close the QEMU window or
> shut down the guest.
> 
> $ ./qemu-system-x86_64.exe -machine pc,accel=tcg -display gtk
> **
> ERROR:../qemu/util/aio-win32.c:337:aio_poll: assertion failed: 
> (in_aio_context_home_thread(ctx))
> Bail out! ERROR:../qemu/util/aio-win32.c:337:aio_poll: assertion failed: 
> (in_aio_context_home_thread(ctx))
> 
> I wonder if you forgot to apply the changes to util/aio-posix.c to
> util/aio-win32.c too? This solves the problem on my Windows host. But
> I have to admit I don't know the code here.

Hi Volker,

yes, you're right. The assertion in the Windows code was added only in
commit 5710a3e09f9 after I had posted some versions of the patch series,
so while I did check this initially, I missed during the rebase for
later versions of the series that I would have to update the patches for
Windows.

Would you like to send a patch for this? I could send one myself if you
prefer, but I can only compile-test Windows patches, so I'd have to rely
on your testing anyway.

Kevin

> > diff --git a/util/aio-posix.c b/util/aio-posix.c
> > index 280f27bb99..30f5354b1e 100644
> > --- a/util/aio-posix.c
> > +++ b/util/aio-posix.c
> > @@ -15,6 +15,7 @@
> >  
> >  #include "qemu/osdep.h"
> >  #include "block/block.h"
> > +#include "qemu/main-loop.h"
> >  #include "qemu/rcu.h"
> >  #include "qemu/rcu_queue.h"
> >  #include "qemu/sockets.h"
> > @@ -558,8 +559,13 @@ bool aio_poll(AioContext *ctx, bool blocking)
> >       * There cannot be two concurrent aio_poll calls for the same 
> > AioContext (or
> >       * an aio_poll concurrent with a GSource prepare/check/dispatch 
> > callback).
> >       * We rely on this below to avoid slow locked accesses to 
> > ctx->notify_me.
> > +     *
> > +     * aio_poll() may only be called in the AioContext's thread. 
> > iohandler_ctx
> > +     * is special in that it runs in the main thread, but that thread's 
> > context
> > +     * is qemu_aio_context.
> >       */
> > -    assert(in_aio_context_home_thread(ctx));
> > +    assert(in_aio_context_home_thread(ctx == iohandler_get_aio_context() ?
> > +                                      qemu_get_aio_context() : ctx));
> >  
> >      qemu_lockcnt_inc(&ctx->list_lock);
> >  
>