On Wed, 2020-01-22 at 17:45 +0100, Max Reitz wrote:
> When nbd_close() is called from a coroutine, the connection_co never
> gets to run, and thus nbd_teardown_connection() hangs.
>
> This is because aio_co_enter() only puts the connection_co into the main
> coroutine's wake-up queue, so this main coroutine needs to yield and
> wait for connection_co to terminate.
Took me a while to understand this back then, good that I explained
my thoughts in the review comment, although the commit message
is alright when you already understand qemu co-routines I guess.
Reviewed-by: Maxim Levitsky <mlevi...@redhat.com>
Best regards,
Maxim Levitsky
>
> Suggested-by: Kevin Wolf <kw...@redhat.com>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> block/nbd.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index d085554f21..6d3b22f844 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -70,6 +70,7 @@ typedef struct BDRVNBDState {
> CoMutex send_mutex;
> CoQueue free_sema;
> Coroutine *connection_co;
> + Coroutine *teardown_co;
> QemuCoSleepState *connection_co_sleep_ns_state;
> bool drained;
> bool wait_drained_end;
> @@ -203,7 +204,15 @@ static void nbd_teardown_connection(BlockDriverState *bs)
> qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
> }
> }
> - BDRV_POLL_WHILE(bs, s->connection_co);
> + if (qemu_in_coroutine()) {
> + s->teardown_co = qemu_coroutine_self();
> + /* connection_co resumes us when it terminates */
> + qemu_coroutine_yield();
> + s->teardown_co = NULL;
> + } else {
> + BDRV_POLL_WHILE(bs, s->connection_co);
> + }
> + assert(!s->connection_co);
> }
>
> static bool nbd_client_connecting(BDRVNBDState *s)
> @@ -395,6 +404,9 @@ static coroutine_fn void nbd_connection_entry(void
> *opaque)
> s->ioc = NULL;
> }
>
> + if (s->teardown_co) {
> + aio_co_wake(s->teardown_co);
> + }
> aio_wait_kick();
> }
>
