diff options
author | Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 2020-07-14 19:22:33 +0300 |
---|---|---|
committer | Kevin Wolf <kwolf@redhat.com> | 2020-07-17 14:20:57 +0200 |
commit | 453cc6be0a954ecdb4e0cdbf5b5b87f07b3e1075 (patch) | |
tree | d2e7585e7af3bf1cee1442ef760f490619514b29 /nbd | |
parent | f21f12936f9db6de4eddfcf9a086b6efd5b093cc (diff) |
nbd: make nbd_export_close_all() synchronous
Consider nbd_export_close_all(). The call-stack looks like this:
nbd_export_close_all() -> nbd_export_close -> call client_close() for
each client.
client_close() doesn't guarantee that client is closed: nbd_trip()
keeps reference to it. So, nbd_export_close_all() just reduce
reference counter on export and removes it from the list, but doesn't
guarantee that nbd_trip() finished neither export actually removed.
Let's wait for all exports actually removed.
Without this fix, the following crash is possible:
- export bitmap through internal 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 fails:
bdrv_release_dirty_bitmap_locked: Assertion `!bdrv_dirty_bitmap_busy(bitmap)' failed
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200714162234.13113-2-vsementsov@virtuozzo.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Diffstat (limited to 'nbd')
-rw-r--r-- | nbd/server.c | 8 |
1 files changed, 8 insertions, 0 deletions
diff --git a/nbd/server.c b/nbd/server.c index 5357f588f0..4752a6c8bc 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -102,6 +102,8 @@ struct NBDExport { }; static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports); +static QTAILQ_HEAD(, NBDExport) closed_exports = + QTAILQ_HEAD_INITIALIZER(closed_exports); /* NBDExportMetaContexts represents a list of contexts to be exported, * as selected by NBD_OPT_SET_META_CONTEXT. Also used for @@ -1659,6 +1661,7 @@ void nbd_export_close(NBDExport *exp) g_free(exp->name); exp->name = NULL; QTAILQ_REMOVE(&exports, exp, next); + QTAILQ_INSERT_TAIL(&closed_exports, exp, next); } g_free(exp->description); exp->description = NULL; @@ -1722,7 +1725,9 @@ void nbd_export_put(NBDExport *exp) g_free(exp->export_bitmap_context); } + QTAILQ_REMOVE(&closed_exports, exp, next); g_free(exp); + aio_wait_kick(); } } @@ -1742,6 +1747,9 @@ void nbd_export_close_all(void) nbd_export_close(exp); aio_context_release(aio_context); } + + AIO_WAIT_WHILE(NULL, !(QTAILQ_EMPTY(&exports) && + QTAILQ_EMPTY(&closed_exports))); } static int coroutine_fn nbd_co_send_iov(NBDClient *client, struct iovec *iov, |