From nobody Mon Feb 9 22:02:54 2026 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 1547236235241765.692636657919; Fri, 11 Jan 2019 11:50:35 -0800 (PST) Received: from localhost ([127.0.0.1]:36701 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gi2p8-0008Ay-3R for importer@patchew.org; Fri, 11 Jan 2019 14:50:34 -0500 Received: from eggs.gnu.org ([209.51.188.92]:40162) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gi2mN-0006Q1-DE for qemu-devel@nongnu.org; Fri, 11 Jan 2019 14:47:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gi2mM-0004yN-9D for qemu-devel@nongnu.org; Fri, 11 Jan 2019 14:47:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52872) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gi2mE-0004nV-RL; Fri, 11 Jan 2019 14:47:36 -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 3F16DC0740E9; Fri, 11 Jan 2019 19:47:29 +0000 (UTC) Received: from blue.redhat.com (ovpn-116-216.phx2.redhat.com [10.3.116.216]) by smtp.corp.redhat.com (Postfix) with ESMTP id 94201605AE; Fri, 11 Jan 2019 19:47:28 +0000 (UTC) From: Eric Blake To: qemu-devel@nongnu.org Date: Fri, 11 Jan 2019 13:47:15 -0600 Message-Id: <20190111194720.15671-4-eblake@redhat.com> In-Reply-To: <20190111194720.15671-1-eblake@redhat.com> References: <20190111194720.15671-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.31]); Fri, 11 Jan 2019 19:47:29 +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] [PATCH v3 3/8] nbd: Only require disabled bitmap for read-only exports 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 , vsementsov@virtuozzo.com, jsnow@redhat.com, qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" Our initial implementation of x-nbd-server-add-bitmap put in a restriction because of incremental backups: in that usage, we are exporting one qcow2 file (the temporary overlay target of a blockdev-backup sync:none job) and a dirty bitmap owned by a second qcow2 file (the source of the blockdev-backup, which is the backing file of the temporary). While both qcow2 files are still writable (the target in order to capture copy-on-write of old contents, and the source in order to track live guest writes in the meantime), the NBD client expects to see constant data, including the dirty bitmap. An enabled bitmap in the source would be modified by guest writes, which is at odds with the NBD export being a read-only constant view, hence the initial code choice of enforcing a disabled bitmap (the intent is that the exposed bitmap was disabled in the same transaction that started the blockdev-backup job, although we don't want to track enough state to actually enforce that). However, consider the case of a bitmap contained in a read-only node (including when the bitmap is found in a backing layer of the active image). Because the node can't be modified, the bitmap won't change due to writes, regardless of whether it is still enabled. Forbidding the export unless the bitmap is disabled is awkward, paritcularly since we can't change the bitmap to be disabled (because the node is read-only). Alternatively, consider the case of live storage migration, where management directs the destination to create a writable NBD server, then performs a drive-mirror from the source to the mirror, prior to doing the rest of the live migration. Since storage migration can be time-consuming, it may be wise to let the destination include a dirty bitmap to track which portions it has already received, where even if the migration is interrupted and restarted, the source can query the destination block status in order to minimize re-sending data that has not changed in the meantime on a second attempt. Such code has not been written, but it makes sense that letting an active dirty bitmap be exposed and changing alongside writes may prove useful in the future. Solve both issues by gating the restriction against a disabled bitmap to only happen when the caller has requested a read-only export, and where the BDS that owns the bitmap (whether or not it is the BDS handed to nbd_export_new() or from its backing chain) is still writable. Update iotest 223 to show the looser behavior by leaving a bitmap enabled the whole run; note that we have to tear down and re-export a node when handling an error. Signed-off-by: Eric Blake Reviewed-by: Vladimir Sementsov-Ogievskiy --- v3: split out unrelated test changes, improve commit message [Vladimir] v2: new patch --- nbd/server.c | 7 +++++-- tests/qemu-iotests/223 | 10 +++++++--- tests/qemu-iotests/223.out | 3 ++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 7af0ddffb20..98327088cb4 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -2456,8 +2456,11 @@ void nbd_export_bitmap(NBDExport *exp, const char *b= itmap, return; } - if (bdrv_dirty_bitmap_enabled(bm)) { - error_setg(errp, "Bitmap '%s' is enabled", bitmap); + if ((exp->nbdflags & NBD_FLAG_READ_ONLY) && bdrv_is_writable(bs) && + bdrv_dirty_bitmap_enabled(bm)) { + error_setg(errp, + "Enabled bitmap '%s' incompatible with readonly export", + bitmap); return; } diff --git a/tests/qemu-iotests/223 b/tests/qemu-iotests/223 index a4016091b21..f200e313c06 100755 --- a/tests/qemu-iotests/223 +++ b/tests/qemu-iotests/223 @@ -61,6 +61,8 @@ echo "=3D=3D=3D Create partially sparse image, then add d= irty bitmaps =3D=3D=3D" echo # Two bitmaps, to contrast granularity issues +# Also note that b will be disabled, while b2 is left enabled, to +# check for read-only interactions _make_test_img -o cluster_size=3D4k 4M $QEMU_IO -c 'w -P 0x11 1M 2M' "$TEST_IMG" | _filter_qemu_io run_qemu <