From e5fb29d5d001dd5f300ddb4ad48e11c3ab2d35ec Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Tue, 15 Mar 2022 00:32:25 +0300 Subject: [PATCH] qapi: nbd-export: allow select bitmaps by node/name pair Hi all! Current logic of relying on search through backing chain is not safe neither convenient. Sometimes it leads to necessity of extra bitmap copying. Also, we are going to add "snapshot-access" driver, to access some snapshot state through NBD. And this driver is not formally a filter, and of course it's not a COW format driver. So, searching through backing chain will not work. Instead of widening the workaround of bitmap searching, let's extend the interface so that user can select bitmap precisely. Note, that checking for bitmap active status is not copied to the new API, I don't see a reason for it, user should understand the risks. And anyway, bitmap from other node is unrelated to this export being read-only or read-write. Signed-off-by: Vladimir Sementsov-Ogievskiy Message-Id: <20220314213226.362217-3-v.sementsov-og@mail.ru> [eblake: Adjust S-o-b to Vladimir's new email, with permission] Reviewed-by: Eric Blake Signed-off-by: Eric Blake --- blockdev-nbd.c | 8 +++++- nbd/server.c | 63 +++++++++++++++++++++++++++--------------- qapi/block-export.json | 5 +++- qemu-nbd.c | 11 ++++++-- 4 files changed, 61 insertions(+), 26 deletions(-) diff --git a/blockdev-nbd.c b/blockdev-nbd.c index 9840d25a82..7f6531cba0 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -211,8 +211,14 @@ void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp) QAPI_CLONE_MEMBERS(BlockExportOptionsNbdBase, &export_opts->u.nbd, qapi_NbdServerAddOptions_base(arg)); if (arg->has_bitmap) { + BlockDirtyBitmapOrStr *el = g_new(BlockDirtyBitmapOrStr, 1); + + *el = (BlockDirtyBitmapOrStr) { + .type = QTYPE_QSTRING, + .u.local = g_strdup(arg->bitmap), + }; export_opts->u.nbd.has_bitmaps = true; - QAPI_LIST_PREPEND(export_opts->u.nbd.bitmaps, g_strdup(arg->bitmap)); + QAPI_LIST_PREPEND(export_opts->u.nbd.bitmaps, el); } /* diff --git a/nbd/server.c b/nbd/server.c index c5644fd3f6..4cdbc062c1 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -1643,7 +1643,7 @@ static int nbd_export_create(BlockExport *blk_exp, BlockExportOptions *exp_args, uint64_t perm, shared_perm; bool readonly = !exp_args->writable; bool shared = !exp_args->writable; - strList *bitmaps; + BlockDirtyBitmapOrStrList *bitmaps; size_t i; int ret; @@ -1709,40 +1709,59 @@ static int nbd_export_create(BlockExport *blk_exp, BlockExportOptions *exp_args, } exp->export_bitmaps = g_new0(BdrvDirtyBitmap *, exp->nr_export_bitmaps); for (i = 0, bitmaps = arg->bitmaps; bitmaps; - i++, bitmaps = bitmaps->next) { - const char *bitmap = bitmaps->value; + i++, bitmaps = bitmaps->next) + { + const char *bitmap; BlockDriverState *bs = blk_bs(blk); BdrvDirtyBitmap *bm = NULL; - while (bs) { - bm = bdrv_find_dirty_bitmap(bs, bitmap); - if (bm != NULL) { - break; + switch (bitmaps->value->type) { + case QTYPE_QSTRING: + bitmap = bitmaps->value->u.local; + while (bs) { + bm = bdrv_find_dirty_bitmap(bs, bitmap); + if (bm != NULL) { + break; + } + + bs = bdrv_filter_or_cow_bs(bs); } - bs = bdrv_filter_or_cow_bs(bs); + if (bm == NULL) { + ret = -ENOENT; + error_setg(errp, "Bitmap '%s' is not found", + bitmaps->value->u.local); + goto fail; + } + + if (readonly && bdrv_is_writable(bs) && + bdrv_dirty_bitmap_enabled(bm)) { + ret = -EINVAL; + error_setg(errp, "Enabled bitmap '%s' incompatible with " + "readonly export", bitmap); + goto fail; + } + break; + case QTYPE_QDICT: + bitmap = bitmaps->value->u.external.name; + bm = block_dirty_bitmap_lookup(bitmaps->value->u.external.node, + bitmap, NULL, errp); + if (!bm) { + ret = -ENOENT; + goto fail; + } + break; + default: + abort(); } - if (bm == NULL) { - ret = -ENOENT; - error_setg(errp, "Bitmap '%s' is not found", bitmap); - goto fail; - } + assert(bm); if (bdrv_dirty_bitmap_check(bm, BDRV_BITMAP_ALLOW_RO, errp)) { ret = -EINVAL; goto fail; } - if (readonly && bdrv_is_writable(bs) && - bdrv_dirty_bitmap_enabled(bm)) { - ret = -EINVAL; - error_setg(errp, - "Enabled bitmap '%s' incompatible with readonly export", - bitmap); - goto fail; - } - exp->export_bitmaps[i] = bm; assert(strlen(bitmap) <= BDRV_BITMAP_MAX_NAME_SIZE); } diff --git a/qapi/block-export.json b/qapi/block-export.json index 1e34927f85..1de16d2589 100644 --- a/qapi/block-export.json +++ b/qapi/block-export.json @@ -6,6 +6,7 @@ ## { 'include': 'sockets.json' } +{ 'include': 'block-core.json' } ## # @NbdServerOptions: @@ -89,6 +90,7 @@ # @device, so the NBD client can use NBD_OPT_SET_META_CONTEXT with # the metadata context name "qemu:dirty-bitmap:BITMAP" to inspect # each bitmap. +# Since 7.1 bitmap may be specified by node/name pair. # # @allocation-depth: Also export the allocation depth map for @device, so # the NBD client can use NBD_OPT_SET_META_CONTEXT with @@ -99,7 +101,8 @@ ## { 'struct': 'BlockExportOptionsNbd', 'base': 'BlockExportOptionsNbdBase', - 'data': { '*bitmaps': ['str'], '*allocation-depth': 'bool' } } + 'data': { '*bitmaps': ['BlockDirtyBitmapOrStr'], + '*allocation-depth': 'bool' } } ## # @BlockExportOptionsVhostUserBlk: diff --git a/qemu-nbd.c b/qemu-nbd.c index 397ffa64d7..db63980df1 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -567,7 +567,7 @@ int main(int argc, char **argv) QDict *options = NULL; const char *export_name = NULL; /* defaults to "" later for server mode */ const char *export_description = NULL; - strList *bitmaps = NULL; + BlockDirtyBitmapOrStrList *bitmaps = NULL; bool alloc_depth = false; const char *tlscredsid = NULL; const char *tlshostname = NULL; @@ -687,7 +687,14 @@ int main(int argc, char **argv) alloc_depth = true; break; case 'B': - QAPI_LIST_PREPEND(bitmaps, g_strdup(optarg)); + { + BlockDirtyBitmapOrStr *el = g_new(BlockDirtyBitmapOrStr, 1); + *el = (BlockDirtyBitmapOrStr) { + .type = QTYPE_QSTRING, + .u.local = g_strdup(optarg), + }; + QAPI_LIST_PREPEND(bitmaps, el); + } break; case 'k': sockpath = optarg;