From nobody Sun Oct 5 21:12:42 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1547484850445795.7340248152165; Mon, 14 Jan 2019 08:54:10 -0800 (PST) Received: from localhost ([127.0.0.1]:44853 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gj5V2-0000km-OM for importer@patchew.org; Mon, 14 Jan 2019 11:54:08 -0500 Received: from eggs.gnu.org ([209.51.188.92]:58247) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gj55X-0004pA-QY for qemu-devel@nongnu.org; Mon, 14 Jan 2019 11:27:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gj55V-000622-Sf for qemu-devel@nongnu.org; Mon, 14 Jan 2019 11:27:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46914) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gj55O-0005l1-QY; Mon, 14 Jan 2019 11:27:39 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A24E6C3A78; Mon, 14 Jan 2019 16:27:22 +0000 (UTC) Received: from blue.redhat.com (ovpn-117-16.phx2.redhat.com [10.3.117.16]) by smtp.corp.redhat.com (Postfix) with ESMTP id CC7C85C6D4; Mon, 14 Jan 2019 16:27:17 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Mon, 14 Jan 2019 10:26:02 -0600 Message-Id: <20190114162605.5330-18-eblake@redhat.com> In-Reply-To: <20190114162605.5330-1-eblake@redhat.com> References: <20190114162605.5330-1-eblake@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Mon, 14 Jan 2019 16:27:22 +0000 (UTC) Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 17/20] nbd: Allow bitmap export during QMP nbd-server-add X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Vladimir Sementsov-Ogievskiy , "open list:Block layer core" , Markus Armbruster , "Dr. David Alan Gilbert" , Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" With the experimental x-nbd-server-add-bitmap command, there was a window of time where an NBD 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 the exposed dirty bitmap (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, with automatic failure to export if the bitmap is not available. The experimental command included an optional 'bitmap-export-name' field for remapping the name exposed over NBD to be different from the bitmap name stored on disk. However, my libvirt demo code for implementing differential backups on top of persistent bitmaps did not need to take advantage of that feature (it is instead possible to create a new temporary bitmap with the desired name, use block-dirty-bitmap-merge to merge one or more persistent bitmaps into the temporary, then associate the temporary with the NBD export, if control is needed over the exported bitmap name). Hence, I'm not copying that part of the experiment over to the stable addition. For more details on the libvirt demo, see https://www.redhat.com/archives/libvir-list/2018-October/msg01254.html, https://kvmforum2018.sched.com/event/FzuB/facilitating-incremental-backup-e= ric-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, with less work to clean up after errors. Later patches will add further cleanups now that this interface is declared stable via a single QMP command, including removing the race window. Update test 223 to use the new interface. Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy Message-Id: <20190111194720.15671-6-eblake@redhat.com> --- qapi/block.json | 7 ++++++- blockdev-nbd.c | 12 +++++++++++- hmp.c | 5 +++-- tests/qemu-iotests/223 | 19 ++++++++----------- tests/qemu-iotests/223.out | 5 +---- 5 files changed, 29 insertions(+), 19 deletions(-) diff --git a/qapi/block.json b/qapi/block.json index 11f01f28efe..3d70420f763 100644 --- a/qapi/block.json +++ b/qapi/block.json @@ -246,6 +246,10 @@ # # @writable: Whether clients should be able to write to the device via the # NBD connection (default false). + +# @bitmap: Also export the dirty bitmap reachable from @device, so the +# NBD client can use NBD_OPT_SET_META_CONTEXT with +# "qemu:dirty-bitmap:NAME" to inspect the bitmap. (since 4.0) # # Returns: error if the server is not running, or export with the same name # already exists. @@ -253,7 +257,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' } } ## # @NbdServerRemoveMode: diff --git a/blockdev-nbd.c b/blockdev-nbd.c index 582ffded77f..ec8cf0ab8c3 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -140,7 +140,8 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr, } void qmp_nbd_server_add(const char *device, bool has_name, const char *nam= e, - bool has_writable, bool writable, Error **errp) + bool has_writable, bool writable, + bool has_bitmap, const char *bitmap, Error **errp) { BlockDriverState *bs =3D NULL; BlockBackend *on_eject_blk; @@ -185,6 +186,15 @@ void qmp_nbd_server_add(const char *device, bool has_n= ame, const char *name, * our only way of accessing it is through nbd_export_find(), so we ca= n drop * the strong reference that is @exp. */ nbd_export_put(exp); + + if (has_bitmap) { + Error *err =3D NULL; + nbd_export_bitmap(exp, bitmap, 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..8da5fd8760a 100644 --- a/hmp.c +++ b/hmp.c @@ -2326,7 +2326,7 @@ 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, &local_err); if (local_err !=3D NULL) { qmp_nbd_server_stop(NULL); @@ -2347,7 +2347,8 @@ void hmp_nbd_server_add(Monitor *mon, const QDict *qd= ict) bool writable =3D qdict_get_try_bool(qdict, "writable", false); Error *local_err =3D NULL; - qmp_nbd_server_add(device, !!name, name, true, writable, &local_err); + qmp_nbd_server_add(device, !!name, name, true, writable, + false, NULL, &local_err); hmp_handle_error(mon, &local_err); } diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223 index f200e313c06..0bcc98a8677 100755 --- a/tests/qemu-iotests/223 +++ b/tests/qemu-iotests/223 @@ -126,23 +126,20 @@ _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-s= tart", "arguments":{"addr":{"type":"unix", "data":{"path":"'"$TEST_DIR/nbd"1'"}}}}' "error" # Attempt second serv= er _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add", - "arguments":{"device":"n"}}' "return" + "arguments":{"device":"n", "bitmap":"b"}}' "return" _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add", "arguments":{"device":"nosuch"}}' "error" # Attempt to export missing no= de _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add", "arguments":{"device":"n"}}' "error" # Attempt to export same name twice -_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-nbd-server-add-bitmap", - "arguments":{"name":"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"}}' "error" # Enabled vs. read-on= ly -_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-remove", - "arguments":{"name":"n2"}}' "return" + "arguments":{"device":"n", "name":"n2", + "bitmap":"b2"}}' "error" # enabled vs. read-only _send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add", - "arguments":{"device":"n", "name":"n2", "writable":true}}' "return" -_send_qemu_cmd $QEMU_HANDLE '{"execute":"x-nbd-server-add-bitmap", - "arguments":{"name":"n2", "bitmap":"b2"}}' "return" + "arguments":{"device":"n", "name":"n2", + "bitmap":"b3"}}' "error" # Missing bitmap +_send_qemu_cmd $QEMU_HANDLE '{"execute":"nbd-server-add", + "arguments":{"device":"n", "name":"n2", "writable":true, + "bitmap":"b2"}}' "return" echo echo "=3D=3D=3D Contrast normal status to large granularity dirty-bitmap = =3D=3D=3D" diff --git a/tests/qemu-iotests/223.out b/tests/qemu-iotests/223.out index 7d147291d48..ebd3935c974 100644 --- a/tests/qemu-iotests/223.out +++ b/tests/qemu-iotests/223.out @@ -33,11 +33,8 @@ wrote 2097152/2097152 bytes at offset 2097152 {"return": {}} {"error": {"class": "GenericError", "desc": "Cannot find device=3Dnosuch n= or node_name=3Dnosuch"}} {"error": {"class": "GenericError", "desc": "NBD server already has export= named 'n'"}} -{"return": {}} -{"return": {}} {"error": {"class": "GenericError", "desc": "Enabled bitmap 'b2' incompati= ble with readonly export"}} -{"return": {}} -{"return": {}} +{"error": {"class": "GenericError", "desc": "Bitmap 'b3' is not found"}} {"return": {}} =3D=3D=3D Contrast normal status to large granularity dirty-bitmap =3D=3D= =3D --=20 2.20.1