Re: [PATCH 2/3] nbd/server: Fix drained_poll to wake coroutine in right AioContext

[Eric Blake]

On Wed, May 17, 2023 at 05:28:33PM +0200, Kevin Wolf wrote:
> nbd_drained_poll() generally runs in the main thread, not whatever
> iothread the NBD server coroutine is meant to run in, so it can't
> directly reenter the coroutines to wake them up.
> 
> The code seems to have the right intention, it specifies the correct
> AioContext when it calls qemu_aio_coroutine_enter(). However, this
> functions doesn't schedule the coroutine to run in that AioContext, but
> it assumes it is already called in the home thread of the AioContext.
> 
> To fix this, add a new thread-safe qio_channel_wake_read() that can be
> called in the main thread to wake up the coroutine in its AioContext,
> and use this in nbd_drained_poll().
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  include/io/channel.h | 10 ++++++++++
>  io/channel.c         | 33 +++++++++++++++++++++++++++------
>  nbd/server.c         |  3 +--
>  3 files changed, 38 insertions(+), 8 deletions(-)
>

Lots of support code...

> +++ b/nbd/server.c
> @@ -1599,8 +1599,7 @@ static bool nbd_drained_poll(void *opaque)
>               * enter it here so we don't depend on the client to wake it up.
>               */
>              if (client->recv_coroutine != NULL && client->read_yielding) {
> -                qemu_aio_coroutine_enter(exp->common.ctx,
> -                                         client->recv_coroutine);
> +                qio_channel_wake_read(client->ioc);
>              }

...for what boils down to a deceptively simple alternative call.  But
your reasoning made sense to me, and I can see how your new function
solves the issue.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org