Hi Alberto, hi Kevin,
this patch caused the iotest 024 to fail when being run with -qed:
024 fail [17:35:06] [17:35:08] 2.2s (last: 2.0s) output
mismatch (see .../tests/qemu-iotests/scratch/qed-file-024/024.out.bad)
--- .../qemu/tests/qemu-iotests/024.out
+++ .../qemu-build/tests/qemu-iotests/scratch/qed-file-024/024.out.bad
@@ -267,6 +267,7 @@
read 1048576/1048576 bytes at offset 3145728
1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
-0 0x400000 TEST_DIR/subdir/t.IMGFMT
+0 0x200000 TEST_DIR/subdir/t.IMGFMT
+0x200000 0x200000 TEST_DIR/subdir/t.IMGFMT
Could you please have a look?
Thanks,
Thomas
PS: Thank you, Kevin, for merging the "Add more image formats to the
thorough testing" patch! As you can see, it already helped to spot the first
regression :-)
On 11/11/2025 22.32, Kevin Wolf wrote:
From: Alberto Garcia <[email protected]>
During a rebase operation data is copied from the backing chain into
the target image using a loop, and each iteration looks for a
contiguous region of allocated data of at most IO_BUF_SIZE (2 MB).
Once that region is found, and in order to avoid partial writes, its
boundaries are extended so they are aligned to the (sub)clusters of
the target image (see commit 12df580b).
This operation can however result in a region that exceeds the maximum
allowed IO_BUF_SIZE, crashing qemu-img.
This can be easily reproduced when the source image has a smaller
cluster size than the target image:
base <- int <- active
$ qemu-img create -f qcow2 base.qcow2 4M
$ qemu-img create -f qcow2 -F qcow2 -b base.qcow2 -o cluster_size=1M int.qcow2
$ qemu-img create -f qcow2 -F qcow2 -b int.qcow2 -o cluster_size=2M
active.qcow2
$ qemu-io -c "write -P 0xff 1M 2M" int.qcow2
$ qemu-img rebase -F qcow2 -b base.qcow2 active.qcow2
qemu-img: qemu-img.c:4102: img_rebase: Assertion `written + pnum <=
IO_BUF_SIZE' failed.
Aborted
Cc: qemu-stable <[email protected]>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3174
Fixes: 12df580b3b7f ("qemu-img: rebase: avoid unnecessary COW operations")
Signed-off-by: Alberto Garcia <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Kevin Wolf <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
---
qemu-img.c | 2 +-
tests/qemu-iotests/024 | 46 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/024.out | 26 +++++++++++++++++++++
3 files changed, 73 insertions(+), 1 deletion(-)
diff --git a/qemu-img.c b/qemu-img.c
index 7a32d2d16c..c42dd4e995 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4081,7 +4081,7 @@ static int img_rebase(const img_cmd_t *ccmd, int argc,
char **argv)
n += offset - QEMU_ALIGN_DOWN(offset, write_align);
offset = QEMU_ALIGN_DOWN(offset, write_align);
n += QEMU_ALIGN_UP(offset + n, write_align) - (offset + n);
- n = MIN(n, size - offset);
+ n = MIN(n, MIN(size - offset, IO_BUF_SIZE));
assert(!bdrv_is_allocated(unfiltered_bs, offset, n, &n_alloc) &&
n_alloc == n);
diff --git a/tests/qemu-iotests/024 b/tests/qemu-iotests/024
index b29c76e161..021169b4a1 100755
--- a/tests/qemu-iotests/024
+++ b/tests/qemu-iotests/024
@@ -315,6 +315,52 @@ echo
$QEMU_IMG map "$OVERLAY" | _filter_qemu_img_map
+# Check that the region to copy to the overlay during a rebase
+# operation does not exceed the I/O buffer size.
+#
+# backing_new <-- backing_old <-- overlay
+#
+# Backing (new): -- -- -- -- <-- Empty image, size 4MB
+# Backing (old):|--|ff|ff|--| <-- 4 clusters, 1MB each
+# Overlay: |-- --|-- --| <-- 2 clusters, 2MB each
+#
+# The data at [1MB, 3MB) must be copied from the old backing image to
+# the overlay. However the rebase code will extend that region to the
+# overlay's (sub)cluster boundaries to avoid CoW (see commit 12df580b).
+# This test checks that IO_BUF_SIZE (2 MB) is taken into account.
+
+echo
+echo "=== Test that the region to copy does not exceed 2MB (IO_BUF_SIZE) ==="
+echo
+
+echo "Creating backing chain"
+echo
+
+TEST_IMG=$BASE_NEW _make_test_img 4M
+TEST_IMG=$BASE_OLD CLUSTER_SIZE=1M _make_test_img -b "$BASE_NEW" -F $IMGFMT
+TEST_IMG=$OVERLAY CLUSTER_SIZE=2M _make_test_img -b "$BASE_OLD" -F $IMGFMT
+
+echo
+echo "Writing data to region [1MB, 3MB)"
+echo
+
+$QEMU_IO "$BASE_OLD" -c "write -P 0xff 1M 2M" | _filter_qemu_io
+
+echo
+echo "Rebasing"
+echo
+
+$QEMU_IMG rebase -b "$BASE_NEW" -F $IMGFMT "$OVERLAY"
+
+echo "Verifying the data"
+echo
+
+$QEMU_IO "$OVERLAY" -c "read -P 0x00 0 1M" | _filter_qemu_io
+$QEMU_IO "$OVERLAY" -c "read -P 0xff 1M 2M" | _filter_qemu_io
+$QEMU_IO "$OVERLAY" -c "read -P 0x00 3M 1M" | _filter_qemu_io
+
+$QEMU_IMG map "$OVERLAY" | _filter_qemu_img_map
+
echo
# success, all done
diff --git a/tests/qemu-iotests/024.out b/tests/qemu-iotests/024.out
index 3d1e31927a..1b7522ba71 100644
--- a/tests/qemu-iotests/024.out
+++ b/tests/qemu-iotests/024.out
@@ -243,4 +243,30 @@ Offset Length File
0 0x20000 TEST_DIR/subdir/t.IMGFMT
0x40000 0x20000 TEST_DIR/subdir/t.IMGFMT
+=== Test that the region to copy does not exceed 2MB (IO_BUF_SIZE) ===
+
+Creating backing chain
+
+Formatting 'TEST_DIR/subdir/t.IMGFMT.base_new', fmt=IMGFMT size=4194304
+Formatting 'TEST_DIR/subdir/t.IMGFMT.base_old', fmt=IMGFMT size=4194304
backing_file=TEST_DIR/subdir/t.IMGFMT.base_new backing_fmt=IMGFMT
+Formatting 'TEST_DIR/subdir/t.IMGFMT', fmt=IMGFMT size=4194304
backing_file=TEST_DIR/subdir/t.IMGFMT.base_old backing_fmt=IMGFMT
+
+Writing data to region [1MB, 3MB)
+
+wrote 2097152/2097152 bytes at offset 1048576
+2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+Rebasing
+
+Verifying the data
+
+read 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 2097152/2097152 bytes at offset 1048576
+2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1048576/1048576 bytes at offset 3145728
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset Length File
+0 0x400000 TEST_DIR/subdir/t.IMGFMT
+
*** done
