Re: [RFC PATCH 19/22] block/export: Move strong user reference to block_exports

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3776 bytes --]

Am 19.08.2020 um 13:56 hat Max Reitz geschrieben:
> On 13.08.20 18:29, Kevin Wolf wrote:
> > The reference owned by the user/monitor that is created when adding the
> > export and dropped when removing it was tied to the 'exports' list in
> > nbd/server.c. Every block export will have a user reference, so move it
> > to the block export level and tie it to the 'block_exports' list in
> > block/export/export.c instead. This is necessary for introducing a QMP
> > command for removing exports.
> > 
> > Note that exports are present in block_exports even after the user has
> > requested shutdown. This is different from NBD's exports where exports
> > are immediately removed on a shutdown request, even if they are still in
> > the process of shutting down. In order to avoid that the user still
> > interacts with an export that is shutting down (and possibly removes it
> > a second time), we need to remember if the user actually still owns it.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  include/block/export.h | 8 ++++++++
> >  block/export/export.c  | 4 ++++
> >  blockdev-nbd.c         | 5 -----
> >  nbd/server.c           | 2 --
> >  4 files changed, 12 insertions(+), 7 deletions(-)
> 
> With this patch, there’s an abort in iotest 281.  Perhaps because
> blk_exp_unref() is now done by blk_exp_request_shutdown() outside of
> where the AIO context is locked?

I have two fixes locally that were related to failing qemu-iotests. I
guess the first one might be for what you're seeing?

Kevin

diff --git a/block/export/export.c b/block/export/export.c
index 71d17bd440..d021b98b74 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -105,9 +105,14 @@ void blk_exp_unref(BlockExport *exp)
 {
     assert(exp->refcount > 0);
     if (--exp->refcount == 0) {
+        AioContext *aio_context = exp->ctx;
+
+        aio_context_acquire(aio_context);
         QLIST_REMOVE(exp, next);
         exp->drv->delete(exp);
         blk_unref(exp->blk);
+        aio_context_release(aio_context);
+
         g_free(exp->id);
         g_free(exp);
         aio_wait_kick();
diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out
index e1eaaedb55..31ce9e6fe0 100644
--- a/tests/qemu-iotests/223.out
+++ b/tests/qemu-iotests/223.out
@@ -45,7 +45,7 @@ exports available: 0
 {"execute":"nbd-server-add", "arguments":{"device":"nosuch"}}
 {"error": {"class": "GenericError", "desc": "Cannot find device=nosuch nor node_name=nosuch"}}
 {"execute":"nbd-server-add", "arguments":{"device":"n"}}
-{"error": {"class": "GenericError", "desc": "NBD server already has export named 'n'"}}
+{"error": {"class": "GenericError", "desc": "Block export id 'n' is already in use"}}
 {"execute":"nbd-server-add", "arguments":{"device":"n", "name":"n2", "bitmap":"b2"}}
 {"error": {"class": "GenericError", "desc": "Enabled bitmap 'b2' incompatible with readonly export"}}
 {"execute":"nbd-server-add", "arguments":{"device":"n", "name":"n2", "bitmap":"b3"}}
@@ -126,7 +126,7 @@ exports available: 0
 {"execute":"nbd-server-add", "arguments":{"device":"nosuch"}}
 {"error": {"class": "GenericError", "desc": "Cannot find device=nosuch nor node_name=nosuch"}}
 {"execute":"nbd-server-add", "arguments":{"device":"n"}}
-{"error": {"class": "GenericError", "desc": "NBD server already has export named 'n'"}}
+{"error": {"class": "GenericError", "desc": "Block export id 'n' is already in use"}}
 {"execute":"nbd-server-add", "arguments":{"device":"n", "name":"n2", "bitmap":"b2"}}
 {"error": {"class": "GenericError", "desc": "Enabled bitmap 'b2' incompatible with readonly export"}}
 {"execute":"nbd-server-add", "arguments":{"device":"n", "name":"n2", "bitmap":"b3"}}


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]