qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/4] nbd: make client_close synchronous


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH 3/4] nbd: make client_close synchronous
Date: Mon, 29 Jun 2020 10:55:06 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0

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 <vsementsov@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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]