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.eb...@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