With the experimental x-nbd-server-add-bitmap command, there was a window of time where a client could see the export but not the associated dirty bitmap, which can cause a client that planned on using the dirty bitmap to be forced to treat the entire image as dirty as a safety fallback. Furthermore, if the QMP client successfully exports a disk but then fails to add the bitmap, it has to take on the burden of removing the export. Since we don't allow changing a dirty bitmap once it is exported (whether to a different bitmap, or removing advertisement of the bitmap), it is nicer to make the bitmap tied to the export at the time the export is created, and automatically failing to export if the bitmap is not available.
Since there is working libvirt demo code that uses both the bitmap export and the ability to specify an alternative name (rather than exposing the private-use bitmap that libvirt created to merge in several persistent bitmaps when doing a differential backup), the two new parameters do not need to be marked experimental. See https://www.redhat.com/archives/libvir-list/2018-October/msg01254.html, https://kvmforum2018.sched.com/event/FzuB/facilitating-incremental-backup-eric-blake-red-hat This patch focuses on the user interface, and reduces (but does not completely eliminate) the window where an NBD client can see the export but not the dirty bitmap. Later patches will add further cleanups now that this interface is available. Update test 223 to use the new interface. Signed-off-by: Eric Blake <ebl...@redhat.com> --- qapi/block.json | 12 +++++++++++- blockdev-nbd.c | 27 ++++++++++++++++++++++++++- hmp.c | 6 ++++-- tests/qemu-iotests/223 | 11 ++++------- tests/qemu-iotests/223.out | 2 -- 5 files changed, 45 insertions(+), 13 deletions(-) diff --git a/qapi/block.json b/qapi/block.json index 11f01f28efe..4b336303d21 100644 --- a/qapi/block.json +++ b/qapi/block.json @@ -246,6 +246,15 @@ # # @writable: Whether clients should be able to write to the device via the # NBD connection (default false). + +# @bitmap: Dirty bitmap to associate with the selected export. The export +# must be read-only, and the given bitmap is locked until the +# export is removed. (since 4.0) +# +# @bitmap-export-name: How the bitmap will be seen by nbd clients +# (default @bitmap). The NBD client must use +# NBD_OPT_SET_META_CONTEXT with "qemu:dirty-bitmap:NAME" +# to access the exposed bitmap. (since 4.0) # # Returns: error if the server is not running, or export with the same name # already exists. @@ -253,7 +262,8 @@ # Since: 1.3.0 ## { 'command': 'nbd-server-add', - 'data': {'device': 'str', '*name': 'str', '*writable': 'bool'} } + 'data': {'device': 'str', '*name': 'str', '*writable': 'bool', + '*bitmap': 'str', '*bitmap-export-name': 'str' } } ## # @NbdServerRemoveMode: diff --git a/blockdev-nbd.c b/blockdev-nbd.c index f5edbc27d88..cae9e802d48 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -140,7 +140,10 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr, } void qmp_nbd_server_add(const char *device, bool has_name, const char *name, - bool has_writable, bool writable, Error **errp) + bool has_writable, bool writable, + bool has_bitmap, const char *bitmap, + bool has_bitmap_export_name, + const char *bitmap_export_name, Error **errp) { BlockDriverState *bs = NULL; BlockBackend *on_eject_blk; @@ -160,6 +163,17 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name, return; } + if (has_bitmap_export_name && !has_bitmap) { + error_setg(errp, "Choosing bitmap export name '%s' requires a bitmap", + bitmap_export_name); + return; + } + if (has_bitmap && writable) { + error_setg(errp, "Cannot export bitmap '%s' on writable export", + bitmap); + return; + } + on_eject_blk = blk_by_name(device); bs = bdrv_lookup_bs(device, device, errp); @@ -185,6 +199,17 @@ void qmp_nbd_server_add(const char *device, bool has_name, const char *name, * our only way of accessing it is through nbd_export_find(), so we can drop * the strong reference that is @exp. */ nbd_export_put(exp); + + if (has_bitmap) { + Error *err = NULL; + nbd_export_bitmap(exp, bitmap, + has_bitmap_export_name ? bitmap_export_name : bitmap, + &err); + if (err) { + error_propagate(errp, err); + nbd_export_remove(exp, NBD_SERVER_REMOVE_MODE_HARD, NULL); + } + } } void qmp_nbd_server_remove(const char *name, diff --git a/hmp.c b/hmp.c index 80aa5ab504b..484a0000de1 100644 --- a/hmp.c +++ b/hmp.c @@ -2326,7 +2326,8 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict) } qmp_nbd_server_add(info->value->device, false, NULL, - true, writable, &local_err); + true, writable, false, NULL, false, NULL, + &local_err); if (local_err != NULL) { qmp_nbd_server_stop(NULL); @@ -2347,7 +2348,8 @@ void hmp_nbd_server_add(Monitor *mon, const QDict *qdict) bool writable = qdict_get_try_bool(qdict, "writable", false); Error *local_err = NULL; - qmp_nbd_server_add(device, !!name, name, true, writable, &local_err); + qmp_nbd_server_add(device, !!name, name, true, writable, + false, NULL, false, NULL, &local_err); hmp_handle_error(mon, &local_err); } diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223 index 5513dc62159..7a92c3018e2 100755 --- a/tests/qemu-iotests/223 +++ b/tests/qemu-iotests/223 @@ -120,13 +120,10 @@ _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-start", "arguments":{"addr":{"type":"unix", "data":{"path":"'"$TEST_DIR/nbd"'"}}}}' "return" _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add", - "arguments":{"device":"n"}}' "return" -_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-nbd-server-add-bitmap", - "arguments":{"name":"n", "bitmap":"b"}}' "return" + "arguments":{"device":"n", "bitmap":"b"}}' "return" _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add", - "arguments":{"device":"n", "name":"n2"}}' "return" -_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-nbd-server-add-bitmap", - "arguments":{"name":"n2", "bitmap":"b2"}}' "return" + "arguments":{"device":"n", "name":"n2", "bitmap":"b2", + "bitmap-export-name":"b3"}}' "return" echo echo "=== Contrast normal status to large granularity dirty-bitmap ===" @@ -147,7 +144,7 @@ echo IMG="driver=nbd,export=n2,server.type=unix,server.path=$TEST_DIR/nbd" $QEMU_IMG map --output=json --image-opts \ - "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b2" | _filter_qemu_img_map + "$IMG,x-dirty-bitmap=qemu:dirty-bitmap:b3" | _filter_qemu_img_map echo echo "=== End NBD server ===" diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out index 99ca172fbb8..0e467981bb8 100644 --- a/tests/qemu-iotests/223.out +++ b/tests/qemu-iotests/223.out @@ -31,8 +31,6 @@ wrote 2097152/2097152 bytes at offset 2097152 {"return": {}} {"return": {}} {"return": {}} -{"return": {}} -{"return": {}} === Contrast normal status to large granularity dirty-bitmap === -- 2.20.1
