25.06.2020 17:25, Vladimir Sementsov-Ogievskiy wrote:
client_close doesn't guarantee that client is closed: nbd_trip() keeps reference to it. Let's wait for nbd_trip to finish.Without this fix, the following crash is possible: - export bitmap through unternal Qemu NBD server - connect a client - shutdown Qemu On shutdown nbd_export_close_all is called, but it actually don't wait for nbd_trip() to finish and to release its references. So, export is not release, and exported bitmap remains busy, and on try to remove the bitmap (which is part of bdrv_close()) the assertion fairs: bdrv_release_dirty_bitmap_locked: Assertion `!bdrv_dirty_bitmap_busy(bitmap)' failed Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> --- nbd/server.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/nbd/server.c b/nbd/server.c index 20754e9ebc..5e27a8d31a 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1419,6 +1419,8 @@ static void client_close(NBDClient *client, bool negotiated) qio_channel_shutdown(client->ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);+ AIO_WAIT_WHILE(client->exp->ctx, client->recv_coroutine);+ /* Also tell the client, so that they release their reference. */ if (client->close_fn) { client->close_fn(client, negotiated); @@ -2450,6 +2452,7 @@ static coroutine_fn void nbd_trip(void *opaque)trace_nbd_trip();if (client->closing) { + client->recv_coroutine = NULL; nbd_client_put(client); return; }
I have a doubt, isn't it possible to hang in AIO_WAIT_WHILE() after this patch? Do we need aio_wait_kick() in the nbd_trip()? Or may be, something like aio_wait_kick(), but to wake exactly client->exp->ctx aio context? Also, why in block/io.c we kick the main context, but not bs->aio_context? -- Best regards, Vladimir
