[PATCH v2 4/4] blockdev: mirror: check for target's cluster size when using bitmap

[Fiona Ebner]

When using mirror with a bitmap and the target 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 <f.ebner@proxmox.com>
---
 blockdev.c                                    | 19 +++++++++++++++++++
 tests/qemu-iotests/tests/bitmap-sync-mirror   |  6 ++++++
 .../qemu-iotests/tests/bitmap-sync-mirror.out |  7 +++++++
 3 files changed, 32 insertions(+)

diff --git a/blockdev.c b/blockdev.c
index c76eb97a4c..968d44cd3b 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2847,6 +2847,9 @@ static void blockdev_mirror_common(const char *job_id, 
BlockDriverState *bs,
     }
 
     if (bitmap_name) {
+        BlockDriverInfo bdi;
+        uint32_t bitmap_granularity;
+
         if (sync != MIRROR_SYNC_MODE_FULL) {
             error_setg(errp, "Sync mode '%s' not supported with bitmap.",
                        MirrorSyncMode_str(sync));
@@ -2863,6 +2866,22 @@ static void blockdev_mirror_common(const char *job_id, 
BlockDriverState *bs,
             return;
         }
 
+        bitmap_granularity = bdrv_dirty_bitmap_granularity(bitmap);
+        /*
+         * 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 already
+         * states that the target's cluster size needs to small enough then.
+         */
+        if (bdrv_get_info(target, &bdi) >= 0) {
+            if (bitmap_granularity < bdi.cluster_size ||
+                bitmap_granularity % bdi.cluster_size != 0) {
+                error_setg(errp, "Bitmap granularity %u is not a multiple of "
+                           "the target image's cluster size %u",
+                           bitmap_granularity, bdi.cluster_size);
+                return;
+            }
+        }
+
         if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_ALLOW_RO, errp)) {
             return;
         }
diff --git a/tests/qemu-iotests/tests/bitmap-sync-mirror 
b/tests/qemu-iotests/tests/bitmap-sync-mirror
index 898f1f4ba4..cbd5cc99cc 100755
--- a/tests/qemu-iotests/tests/bitmap-sync-mirror
+++ b/tests/qemu-iotests/tests/bitmap-sync-mirror
@@ -552,6 +552,12 @@ def test_mirror_api():
                                 bitmap=bitmap)
                 log('')
 
+        log("-- Test bitmap with too small granularity --\n".format(sync_mode))
+        vm.qmp_log("block-dirty-bitmap-add", node=drive0.node,
+                   name="bitmap-small", granularity=(GRANULARITY // 2))
+        blockdev_mirror(drive0.vm, drive0.node, "mirror_target", "full",
+                        job_id='api_job', bitmap="bitmap-small")
+        log('')
 
 def main():
     for bsync_mode in ("never", "on-success", "always"):
diff --git a/tests/qemu-iotests/tests/bitmap-sync-mirror.out 
b/tests/qemu-iotests/tests/bitmap-sync-mirror.out
index c05b4788c6..d40ea7d689 100644
--- a/tests/qemu-iotests/tests/bitmap-sync-mirror.out
+++ b/tests/qemu-iotests/tests/bitmap-sync-mirror.out
@@ -2937,3 +2937,10 @@ qemu_img compare "TEST_DIR/PID-img" 
"TEST_DIR/PID-fmirror3" ==> Identical, OK!
 {"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap0", "device": 
"drive0", "filter-node-name": "mirror-top", "job-id": "api_job", "sync": 
"none", "target": "mirror_target"}}
 {"error": {"class": "GenericError", "desc": "Sync mode 'none' not supported 
with bitmap."}}
 
+-- Test bitmap with too small granularity --
+
+{"execute": "block-dirty-bitmap-add", "arguments": {"granularity": 32768, 
"name": "bitmap-small", "node": "drive0"}}
+{"return": {}}
+{"execute": "blockdev-mirror", "arguments": {"bitmap": "bitmap-small", 
"device": "drive0", "filter-node-name": "mirror-top", "job-id": "api_job", 
"sync": "full", "target": "mirror_target"}}
+{"error": {"class": "GenericError", "desc": "Bitmap granularity 32768 is not a 
multiple of the target image's cluster size 65536"}}
+
-- 
2.39.2