On 19.08.20 16:23, Kevin Wolf wrote: > 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 <kw...@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) {
If this is done without locking the context, should this be an atomic operation? > + 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); > + But for the crash I was seeing, this should be sufficient, yes. Max
signature.asc
Description: OpenPGP digital signature