From nobody Mon Nov 25 09:26:55 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1716294099651229.02570130402592; Tue, 21 May 2024 05:21:39 -0700 (PDT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1s9OUA-0000ES-Jj; Tue, 21 May 2024 08:20:58 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1s9OTp-0008DJ-AU; Tue, 21 May 2024 08:20:34 -0400 Received: from proxmox-new.maurer-it.com ([94.136.29.106]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1s9OTl-0007y0-AN; Tue, 21 May 2024 08:20:33 -0400 Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 2595E431AC; Tue, 21 May 2024 14:20:22 +0200 (CEST) From: Fiona Ebner To: qemu-devel@nongnu.org Cc: qemu-block@nongnu.org, armbru@redhat.com, eblake@redhat.com, hreitz@redhat.com, kwolf@redhat.com, vsementsov@yandex-team.ru, jsnow@redhat.com, f.gruenbichler@proxmox.com, t.lamprecht@proxmox.com, mahaocong@didichuxing.com, xiechanglong.d@gmail.com, wencongyang2@huawei.com Subject: [PATCH v4 5/5] blockdev: mirror: check for target's cluster size when using bitmap Date: Tue, 21 May 2024 14:20:14 +0200 Message-Id: <20240521122014.333221-6-f.ebner@proxmox.com> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240521122014.333221-1-f.ebner@proxmox.com> References: <20240521122014.333221-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.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; Received-SPF: pass client-ip=94.136.29.106; envelope-from=f.ebner@proxmox.com; helo=proxmox-new.maurer-it.com X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: qemu-devel-bounces+importer=patchew.org@nongnu.org X-ZM-MESSAGEID: 1716294099879100005 Content-Type: text/plain; charset="utf-8" When using mirror with a bitmap and the target does not do COW and is is a diff image, i.e. one that should only contain the delta and was not synced to previously, a too large cluster size for the target can be problematic. In particular, when the mirror sends data to the target aligned to the jobs granularity, but not aligned to the larger target image's cluster size, the target's cluster would be allocated but only be filled partially. When rebasing such a diff image later, the corresponding cluster of the base image would get "masked" and the part of the cluster not in the diff image is not accessible anymore. Unfortunately, it is not always possible to check for the target image's cluster size, e.g. when it's NBD. Because the limitation is already documented in the QAPI description for the @bitmap parameter and it's only required for special diff image use-case, simply skip the check then. Signed-off-by: Fiona Ebner --- blockdev.c | 57 ++++++++++++++++++++++ tests/qemu-iotests/tests/mirror-bitmap | 6 +++ tests/qemu-iotests/tests/mirror-bitmap.out | 7 +++ 3 files changed, 70 insertions(+) diff --git a/blockdev.c b/blockdev.c index 4f72a72dc7..468974108e 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2769,6 +2769,59 @@ void qmp_blockdev_backup(BlockdevBackup *backup, Err= or **errp) blockdev_do_action(&action, errp); } =20 +static int blockdev_mirror_check_bitmap_granularity(BlockDriverState *targ= et, + BdrvDirtyBitmap *bitma= p, + Error **errp) +{ + int ret; + BlockDriverInfo bdi; + uint32_t bitmap_granularity; + + GLOBAL_STATE_CODE(); + GRAPH_RDLOCK_GUARD_MAINLOOP(); + + if (bdrv_backing_chain_next(target)) { + /* + * No need to worry about creating clusters with partial data when= the + * target does COW. + */ + return 0; + } + + /* + * If there is no backing file on the target, we cannot rely on COW if= our + * backup cluster size is smaller than the target cluster size. Even f= or + * targets with a backing file, try to avoid COW if possible. + */ + ret =3D bdrv_get_info(target, &bdi); + if (ret =3D=3D -ENOTSUP) { + /* + * Ignore if unable to get the info, e.g. when target is NBD. It's= only + * relevant for syncing to a diff image and the documentation alre= ady + * states that the target's cluster size needs to small enough the= n. + */ + return 0; + } else if (ret < 0) { + error_setg_errno(errp, -ret, + "Couldn't determine the cluster size of the target image, " + "which has no backing file"); + return ret; + } + + bitmap_granularity =3D bdrv_dirty_bitmap_granularity(bitmap); + if (bitmap_granularity < bdi.cluster_size || + bitmap_granularity % bdi.cluster_size !=3D 0) { + error_setg(errp, "Bitmap granularity %u is not a multiple of the " + "target image's cluster size %u and the target image ha= s " + "no backing file", + bitmap_granularity, bdi.cluster_size); + return -EINVAL; + } + + return 0; +} + + /* Parameter check and block job starting for drive mirroring. * Caller should hold @device and @target's aio context (must be the same). **/ @@ -2863,6 +2916,10 @@ static void blockdev_mirror_common(const char *job_i= d, BlockDriverState *bs, return; } =20 + if (blockdev_mirror_check_bitmap_granularity(target, bitmap, errp)= ) { + return; + } + if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_DEFAULT, errp)) { return; } diff --git a/tests/qemu-iotests/tests/mirror-bitmap b/tests/qemu-iotests/te= sts/mirror-bitmap index 37bbe0f241..e8cd482a19 100755 --- a/tests/qemu-iotests/tests/mirror-bitmap +++ b/tests/qemu-iotests/tests/mirror-bitmap @@ -584,6 +584,12 @@ def test_mirror_api(): bitmap=3Dbitmap) log('') =20 + log("-- Test bitmap with too small granularity to non-COW target -= -\n") + vm.qmp_log("block-dirty-bitmap-add", node=3Ddrive0.node, + name=3D"bitmap-small", granularity=3DGRANULARITY) + blockdev_mirror(drive0.vm, drive0.node, "mirror_target", "full", + job_id=3D'api_job', bitmap=3D"bitmap-small") + log('') =20 def main(): for bsync_mode in ("never", "on-success", "always"): diff --git a/tests/qemu-iotests/tests/mirror-bitmap.out b/tests/qemu-iotest= s/tests/mirror-bitmap.out index 5c8acc1d69..af605f3803 100644 --- a/tests/qemu-iotests/tests/mirror-bitmap.out +++ b/tests/qemu-iotests/tests/mirror-bitmap.out @@ -3189,3 +3189,10 @@ qemu_img compare "TEST_DIR/PID-img" "TEST_DIR/PID-fm= irror3" =3D=3D> Identical, OK! {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", "copy-mo= de": "write-blocking", "device": "drive0", "filter-node-name": "mirror-top"= , "job-id": "api_job", "sync": "none", "target": "mirror_target"}} {"error": {"class": "GenericError", "desc": "Sync mode 'none' not supporte= d with bitmap."}} =20 +-- Test bitmap with too small granularity to non-COW target -- + +{"execute": "block-dirty-bitmap-add", "arguments": {"granularity": 65536, = "name": "bitmap-small", "node": "drive0"}} +{"return": {}} +{"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap-small", "co= py-mode": "write-blocking", "device": "drive0", "filter-node-name": "mirror= -top", "job-id": "api_job", "sync": "full", "target": "mirror_target"}} +{"error": {"class": "GenericError", "desc": "Bitmap granularity 65536 is n= ot a multiple of the target image's cluster size 131072 and the target imag= e has no backing file"}} + --=20 2.39.2