* [Qemu-devel] [PATCH for-2.8 v2] qcow2: Don't strand clusters near 2G intervals during commit
@ 2016-12-05 15:49 Eric Blake
2016-12-05 19:52 ` John Snow
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Eric Blake @ 2016-12-05 15:49 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, jsnow, qemu-block, stefanha, Max Reitz
The qcow2_make_empty() function is reached during 'qemu-img commit',
in order to clear out ALL clusters of an image. However, if the
image cannot use the fast code path (true if the image is format
0.10, or if the image contains a snapshot), the cluster size is
larger than 512, and the image is larger than 2G in size, then our
choice of sector_step causes problems. Since it is not cluster
aligned, but qcow2_discard_clusters() silently ignores an unaligned
head or tail, we are leaving clusters allocated.
Enhance the testsuite to expose the flaw, and patch the problem by
ensuring our step size is aligned.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v2: perform rounding correctly
---
block/qcow2.c | 3 +-
tests/qemu-iotests/097 | 41 +++++---
tests/qemu-iotests/097.out | 249 +++++++++++++++++++++++++++++++++------------
3 files changed, 210 insertions(+), 83 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index ed9e0f3..96fb8a8 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2808,7 +2808,8 @@ static int qcow2_make_empty(BlockDriverState *bs)
{
BDRVQcow2State *s = bs->opaque;
uint64_t start_sector;
- int sector_step = INT_MAX / BDRV_SECTOR_SIZE;
+ int sector_step = (QEMU_ALIGN_DOWN(INT_MAX, s->cluster_size) /
+ BDRV_SECTOR_SIZE);
int l1_clusters, ret = 0;
l1_clusters = DIV_ROUND_UP(s->l1_size, s->cluster_size / sizeof(uint64_t));
diff --git a/tests/qemu-iotests/097 b/tests/qemu-iotests/097
index 01d8dd0..4c33e80 100755
--- a/tests/qemu-iotests/097
+++ b/tests/qemu-iotests/097
@@ -46,7 +46,7 @@ _supported_proto file
_supported_os Linux
-# Four passes:
+# Four main passes:
# 0: Two-layer backing chain, commit to upper backing file (implicitly)
# (in this case, the top image will be emptied)
# 1: Two-layer backing chain, commit to upper backing file (explicitly)
@@ -56,22 +56,30 @@ _supported_os Linux
# 3: Two-layer backing chain, commit to lower backing file
# (in this case, the top image will implicitly stay unchanged)
#
+# Each pass is run twice, since qcow2 has different code paths for cleaning
+# an image depending on whether it has a snapshot.
+#
# 020 already tests committing, so this only tests whether image chains are
# working properly and that all images above the base are emptied; therefore,
-# no complicated patterns are necessary
+# no complicated patterns are necessary. Check near the 2G mark, as qcow2
+# has been buggy at that boundary in the past.
for i in 0 1 2 3; do
+for j in 0 1; do
echo
-echo "=== Test pass $i ==="
+echo "=== Test pass $i.$j ==="
echo
-TEST_IMG="$TEST_IMG.base" _make_test_img 64M
-TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" 64M
-_make_test_img -b "$TEST_IMG.itmd" 64M
+TEST_IMG="$TEST_IMG.base" _make_test_img 2100M
+TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" 2100M
+_make_test_img -b "$TEST_IMG.itmd" 2100M
+if [ $j -eq 0 ]; then
+ $QEMU_IMG snapshot -c snap "$TEST_IMG"
+fi
-$QEMU_IO -c 'write -P 1 0 192k' "$TEST_IMG.base" | _filter_qemu_io
-$QEMU_IO -c 'write -P 2 64k 128k' "$TEST_IMG.itmd" | _filter_qemu_io
-$QEMU_IO -c 'write -P 3 128k 64k' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'write -P 1 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c 'write -P 2 0x7ffe0000 128k' "$TEST_IMG.itmd" | _filter_qemu_io
+$QEMU_IO -c 'write -P 3 0x7fff0000 64k' "$TEST_IMG" | _filter_qemu_io
if [ $i -lt 3 ]; then
if [ $i == 0 ]; then
@@ -88,12 +96,12 @@ if [ $i -lt 3 ]; then
fi
# Bottom should be unchanged
- $QEMU_IO -c 'read -P 1 0 192k' "$TEST_IMG.base" | _filter_qemu_io
+ $QEMU_IO -c 'read -P 1 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
# Intermediate should contain changes from top
- $QEMU_IO -c 'read -P 1 0 64k' "$TEST_IMG.itmd" | _filter_qemu_io
- $QEMU_IO -c 'read -P 2 64k 64k' "$TEST_IMG.itmd" | _filter_qemu_io
- $QEMU_IO -c 'read -P 3 128k 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+ $QEMU_IO -c 'read -P 1 0x7ffd0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+ $QEMU_IO -c 'read -P 2 0x7ffe0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+ $QEMU_IO -c 'read -P 3 0x7fff0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
# And in pass 0, the top image should be empty, whereas in both other passes
# it should be unchanged (which is both checked by qemu-img map)
@@ -101,9 +109,9 @@ else
$QEMU_IMG commit -b "$TEST_IMG.base" "$TEST_IMG"
# Bottom should contain all changes
- $QEMU_IO -c 'read -P 1 0 64k' "$TEST_IMG.base" | _filter_qemu_io
- $QEMU_IO -c 'read -P 2 64k 64k' "$TEST_IMG.base" | _filter_qemu_io
- $QEMU_IO -c 'read -P 3 128k 64k' "$TEST_IMG.base" | _filter_qemu_io
+ $QEMU_IO -c 'read -P 1 0x7ffd0000 64k' "$TEST_IMG.base" | _filter_qemu_io
+ $QEMU_IO -c 'read -P 2 0x7ffe0000 64k' "$TEST_IMG.base" | _filter_qemu_io
+ $QEMU_IO -c 'read -P 3 0x7fff0000 64k' "$TEST_IMG.base" | _filter_qemu_io
# Both top and intermediate should be unchanged
fi
@@ -113,6 +121,7 @@ $QEMU_IMG map "$TEST_IMG.itmd" | _filter_qemu_img_map
$QEMU_IMG map "$TEST_IMG" | _filter_qemu_img_map
done
+done
# success, all done
diff --git a/tests/qemu-iotests/097.out b/tests/qemu-iotests/097.out
index 48abd2e..8106cc9 100644
--- a/tests/qemu-iotests/097.out
+++ b/tests/qemu-iotests/097.out
@@ -1,119 +1,236 @@
QA output created by 097
-=== Test pass 0 ===
+=== Test pass 0.0 ===
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.itmd
-wrote 196608/196608 bytes at offset 0
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+wrote 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 131072/131072 bytes at offset 65536
+wrote 131072/131072 bytes at offset 2147352576
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset 131072
+wrote 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Image committed.
-read 196608/196608 bytes at offset 0
+read 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 0
+read 65536/65536 bytes at offset 2147287040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 65536
+read 65536/65536 bytes at offset 2147352576
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 131072
+read 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
-0 0x30000 TEST_DIR/t.IMGFMT.base
+0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
Offset Length File
-0 0x10000 TEST_DIR/t.IMGFMT.base
-0x10000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
Offset Length File
-0 0x10000 TEST_DIR/t.IMGFMT.base
-0x10000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
-=== Test pass 1 ===
+=== Test pass 0.1 ===
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.itmd
-wrote 196608/196608 bytes at offset 0
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+wrote 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 131072/131072 bytes at offset 65536
+wrote 131072/131072 bytes at offset 2147352576
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset 131072
+wrote 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Image committed.
-read 196608/196608 bytes at offset 0
+read 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 0
+read 65536/65536 bytes at offset 2147287040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 65536
+read 65536/65536 bytes at offset 2147352576
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 131072
+read 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
-0 0x30000 TEST_DIR/t.IMGFMT.base
+0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
Offset Length File
-0 0x10000 TEST_DIR/t.IMGFMT.base
-0x10000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
Offset Length File
-0 0x10000 TEST_DIR/t.IMGFMT.base
-0x10000 0x10000 TEST_DIR/t.IMGFMT.itmd
-0x20000 0x10000 TEST_DIR/t.IMGFMT
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
-=== Test pass 2 ===
+=== Test pass 1.0 ===
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.itmd
-wrote 196608/196608 bytes at offset 0
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+wrote 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 131072/131072 bytes at offset 65536
+wrote 131072/131072 bytes at offset 2147352576
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset 131072
+wrote 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Image committed.
-read 196608/196608 bytes at offset 0
+read 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 0
+read 65536/65536 bytes at offset 2147287040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 65536
+read 65536/65536 bytes at offset 2147352576
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 131072
+read 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
-0 0x30000 TEST_DIR/t.IMGFMT.base
+0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
Offset Length File
-0 0x10000 TEST_DIR/t.IMGFMT.base
-0x10000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
Offset Length File
-0 0x10000 TEST_DIR/t.IMGFMT.base
-0x10000 0x10000 TEST_DIR/t.IMGFMT.itmd
-0x20000 0x10000 TEST_DIR/t.IMGFMT
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
+0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
-=== Test pass 3 ===
+=== Test pass 1.1 ===
-Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
-Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base
-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.itmd
-wrote 196608/196608 bytes at offset 0
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+wrote 196608/196608 bytes at offset 2147287040
192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 131072/131072 bytes at offset 65536
+wrote 131072/131072 bytes at offset 2147352576
128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-wrote 65536/65536 bytes at offset 131072
+wrote 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Image committed.
-read 65536/65536 bytes at offset 0
+read 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147287040
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147352576
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset Length File
+0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
+0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+
+=== Test pass 2.0 ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+wrote 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 2147352576
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Image committed.
+read 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147287040
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147352576
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset Length File
+0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
+0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+
+=== Test pass 2.1 ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+wrote 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 2147352576
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Image committed.
+read 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147287040
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147352576
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset Length File
+0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
+0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+
+=== Test pass 3.0 ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+wrote 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 2147352576
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Image committed.
+read 65536/65536 bytes at offset 2147287040
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147352576
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset Length File
+0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+Offset Length File
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
+0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+
+=== Test pass 3.1 ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
+wrote 196608/196608 bytes at offset 2147287040
+192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 2147352576
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 2147418112
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Image committed.
+read 65536/65536 bytes at offset 2147287040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 65536
+read 65536/65536 bytes at offset 2147352576
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-read 65536/65536 bytes at offset 131072
+read 65536/65536 bytes at offset 2147418112
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
-0 0x30000 TEST_DIR/t.IMGFMT.base
+0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
Offset Length File
-0 0x10000 TEST_DIR/t.IMGFMT.base
-0x10000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
Offset Length File
-0 0x10000 TEST_DIR/t.IMGFMT.base
-0x10000 0x10000 TEST_DIR/t.IMGFMT.itmd
-0x20000 0x10000 TEST_DIR/t.IMGFMT
+0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
+0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
+0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
*** done
--
2.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.8 v2] qcow2: Don't strand clusters near 2G intervals during commit
2016-12-05 15:49 [Qemu-devel] [PATCH for-2.8 v2] qcow2: Don't strand clusters near 2G intervals during commit Eric Blake
@ 2016-12-05 19:52 ` John Snow
2016-12-06 14:42 ` Eric Blake
2016-12-06 9:47 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-12-06 14:41 ` [Qemu-devel] " Kevin Wolf
2 siblings, 1 reply; 5+ messages in thread
From: John Snow @ 2016-12-05 19:52 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: kwolf, stefanha, qemu-block, Max Reitz
On 12/05/2016 10:49 AM, Eric Blake wrote:
> The qcow2_make_empty() function is reached during 'qemu-img commit',
> in order to clear out ALL clusters of an image. However, if the
> image cannot use the fast code path (true if the image is format
> 0.10, or if the image contains a snapshot), the cluster size is
> larger than 512, and the image is larger than 2G in size, then our
> choice of sector_step causes problems. Since it is not cluster
> aligned, but qcow2_discard_clusters() silently ignores an unaligned
> head or tail, we are leaving clusters allocated.
>
> Enhance the testsuite to expose the flaw, and patch the problem by
> ensuring our step size is aligned.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v2: perform rounding correctly
> ---
> block/qcow2.c | 3 +-
> tests/qemu-iotests/097 | 41 +++++---
> tests/qemu-iotests/097.out | 249 +++++++++++++++++++++++++++++++++------------
> 3 files changed, 210 insertions(+), 83 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index ed9e0f3..96fb8a8 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2808,7 +2808,8 @@ static int qcow2_make_empty(BlockDriverState *bs)
> {
> BDRVQcow2State *s = bs->opaque;
> uint64_t start_sector;
> - int sector_step = INT_MAX / BDRV_SECTOR_SIZE;
> + int sector_step = (QEMU_ALIGN_DOWN(INT_MAX, s->cluster_size) /
> + BDRV_SECTOR_SIZE);
> int l1_clusters, ret = 0;
>
> l1_clusters = DIV_ROUND_UP(s->l1_size, s->cluster_size / sizeof(uint64_t));
> diff --git a/tests/qemu-iotests/097 b/tests/qemu-iotests/097
> index 01d8dd0..4c33e80 100755
> --- a/tests/qemu-iotests/097
> +++ b/tests/qemu-iotests/097
> @@ -46,7 +46,7 @@ _supported_proto file
> _supported_os Linux
>
>
> -# Four passes:
> +# Four main passes:
> # 0: Two-layer backing chain, commit to upper backing file (implicitly)
> # (in this case, the top image will be emptied)
> # 1: Two-layer backing chain, commit to upper backing file (explicitly)
> @@ -56,22 +56,30 @@ _supported_os Linux
> # 3: Two-layer backing chain, commit to lower backing file
> # (in this case, the top image will implicitly stay unchanged)
> #
> +# Each pass is run twice, since qcow2 has different code paths for cleaning
> +# an image depending on whether it has a snapshot.
> +#
> # 020 already tests committing, so this only tests whether image chains are
> # working properly and that all images above the base are emptied; therefore,
> -# no complicated patterns are necessary
> +# no complicated patterns are necessary. Check near the 2G mark, as qcow2
> +# has been buggy at that boundary in the past.
> for i in 0 1 2 3; do
> +for j in 0 1; do
>
> echo
> -echo "=== Test pass $i ==="
> +echo "=== Test pass $i.$j ==="
> echo
>
> -TEST_IMG="$TEST_IMG.base" _make_test_img 64M
> -TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" 64M
> -_make_test_img -b "$TEST_IMG.itmd" 64M
> +TEST_IMG="$TEST_IMG.base" _make_test_img 2100M
> +TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" 2100M
> +_make_test_img -b "$TEST_IMG.itmd" 2100M
> +if [ $j -eq 0 ]; then
> + $QEMU_IMG snapshot -c snap "$TEST_IMG"
> +fi
>
> -$QEMU_IO -c 'write -P 1 0 192k' "$TEST_IMG.base" | _filter_qemu_io
> -$QEMU_IO -c 'write -P 2 64k 128k' "$TEST_IMG.itmd" | _filter_qemu_io
> -$QEMU_IO -c 'write -P 3 128k 64k' "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c 'write -P 1 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
> +$QEMU_IO -c 'write -P 2 0x7ffe0000 128k' "$TEST_IMG.itmd" | _filter_qemu_io
> +$QEMU_IO -c 'write -P 3 0x7fff0000 64k' "$TEST_IMG" | _filter_qemu_io
>
> if [ $i -lt 3 ]; then
> if [ $i == 0 ]; then
> @@ -88,12 +96,12 @@ if [ $i -lt 3 ]; then
> fi
>
> # Bottom should be unchanged
> - $QEMU_IO -c 'read -P 1 0 192k' "$TEST_IMG.base" | _filter_qemu_io
> + $QEMU_IO -c 'read -P 1 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
>
> # Intermediate should contain changes from top
> - $QEMU_IO -c 'read -P 1 0 64k' "$TEST_IMG.itmd" | _filter_qemu_io
> - $QEMU_IO -c 'read -P 2 64k 64k' "$TEST_IMG.itmd" | _filter_qemu_io
> - $QEMU_IO -c 'read -P 3 128k 64k' "$TEST_IMG.itmd" | _filter_qemu_io
> + $QEMU_IO -c 'read -P 1 0x7ffd0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
> + $QEMU_IO -c 'read -P 2 0x7ffe0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
> + $QEMU_IO -c 'read -P 3 0x7fff0000 64k' "$TEST_IMG.itmd" | _filter_qemu_io
>
> # And in pass 0, the top image should be empty, whereas in both other passes
> # it should be unchanged (which is both checked by qemu-img map)
> @@ -101,9 +109,9 @@ else
> $QEMU_IMG commit -b "$TEST_IMG.base" "$TEST_IMG"
>
> # Bottom should contain all changes
> - $QEMU_IO -c 'read -P 1 0 64k' "$TEST_IMG.base" | _filter_qemu_io
> - $QEMU_IO -c 'read -P 2 64k 64k' "$TEST_IMG.base" | _filter_qemu_io
> - $QEMU_IO -c 'read -P 3 128k 64k' "$TEST_IMG.base" | _filter_qemu_io
> + $QEMU_IO -c 'read -P 1 0x7ffd0000 64k' "$TEST_IMG.base" | _filter_qemu_io
> + $QEMU_IO -c 'read -P 2 0x7ffe0000 64k' "$TEST_IMG.base" | _filter_qemu_io
> + $QEMU_IO -c 'read -P 3 0x7fff0000 64k' "$TEST_IMG.base" | _filter_qemu_io
>
> # Both top and intermediate should be unchanged
> fi
> @@ -113,6 +121,7 @@ $QEMU_IMG map "$TEST_IMG.itmd" | _filter_qemu_img_map
> $QEMU_IMG map "$TEST_IMG" | _filter_qemu_img_map
>
> done
> +done
>
>
> # success, all done
> diff --git a/tests/qemu-iotests/097.out b/tests/qemu-iotests/097.out
> index 48abd2e..8106cc9 100644
> --- a/tests/qemu-iotests/097.out
> +++ b/tests/qemu-iotests/097.out
> @@ -1,119 +1,236 @@
> QA output created by 097
>
> -=== Test pass 0 ===
> +=== Test pass 0.0 ===
>
> -Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
> -Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.itmd
> -wrote 196608/196608 bytes at offset 0
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
> +Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
> +wrote 196608/196608 bytes at offset 2147287040
> 192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 131072/131072 bytes at offset 65536
> +wrote 131072/131072 bytes at offset 2147352576
> 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 65536/65536 bytes at offset 131072
> +wrote 65536/65536 bytes at offset 2147418112
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> Image committed.
> -read 196608/196608 bytes at offset 0
> +read 196608/196608 bytes at offset 2147287040
> 192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -read 65536/65536 bytes at offset 0
> +read 65536/65536 bytes at offset 2147287040
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -read 65536/65536 bytes at offset 65536
> +read 65536/65536 bytes at offset 2147352576
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -read 65536/65536 bytes at offset 131072
> +read 65536/65536 bytes at offset 2147418112
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> Offset Length File
> -0 0x30000 TEST_DIR/t.IMGFMT.base
> +0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
> Offset Length File
> -0 0x10000 TEST_DIR/t.IMGFMT.base
> -0x10000 0x20000 TEST_DIR/t.IMGFMT.itmd
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
> Offset Length File
> -0 0x10000 TEST_DIR/t.IMGFMT.base
> -0x10000 0x20000 TEST_DIR/t.IMGFMT.itmd
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
>
> -=== Test pass 1 ===
> +=== Test pass 0.1 ===
>
> -Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
> -Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.itmd
> -wrote 196608/196608 bytes at offset 0
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
> +Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
> +wrote 196608/196608 bytes at offset 2147287040
> 192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 131072/131072 bytes at offset 65536
> +wrote 131072/131072 bytes at offset 2147352576
> 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 65536/65536 bytes at offset 131072
> +wrote 65536/65536 bytes at offset 2147418112
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> Image committed.
> -read 196608/196608 bytes at offset 0
> +read 196608/196608 bytes at offset 2147287040
> 192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -read 65536/65536 bytes at offset 0
> +read 65536/65536 bytes at offset 2147287040
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -read 65536/65536 bytes at offset 65536
> +read 65536/65536 bytes at offset 2147352576
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -read 65536/65536 bytes at offset 131072
> +read 65536/65536 bytes at offset 2147418112
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> Offset Length File
> -0 0x30000 TEST_DIR/t.IMGFMT.base
> +0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
> Offset Length File
> -0 0x10000 TEST_DIR/t.IMGFMT.base
> -0x10000 0x20000 TEST_DIR/t.IMGFMT.itmd
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
> Offset Length File
> -0 0x10000 TEST_DIR/t.IMGFMT.base
> -0x10000 0x10000 TEST_DIR/t.IMGFMT.itmd
> -0x20000 0x10000 TEST_DIR/t.IMGFMT
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
>
> -=== Test pass 2 ===
> +=== Test pass 1.0 ===
>
> -Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
> -Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.itmd
> -wrote 196608/196608 bytes at offset 0
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
> +Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
> +wrote 196608/196608 bytes at offset 2147287040
> 192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 131072/131072 bytes at offset 65536
> +wrote 131072/131072 bytes at offset 2147352576
> 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 65536/65536 bytes at offset 131072
> +wrote 65536/65536 bytes at offset 2147418112
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> Image committed.
> -read 196608/196608 bytes at offset 0
> +read 196608/196608 bytes at offset 2147287040
> 192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -read 65536/65536 bytes at offset 0
> +read 65536/65536 bytes at offset 2147287040
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -read 65536/65536 bytes at offset 65536
> +read 65536/65536 bytes at offset 2147352576
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -read 65536/65536 bytes at offset 131072
> +read 65536/65536 bytes at offset 2147418112
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> Offset Length File
> -0 0x30000 TEST_DIR/t.IMGFMT.base
> +0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
> Offset Length File
> -0 0x10000 TEST_DIR/t.IMGFMT.base
> -0x10000 0x20000 TEST_DIR/t.IMGFMT.itmd
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
> Offset Length File
> -0 0x10000 TEST_DIR/t.IMGFMT.base
> -0x10000 0x10000 TEST_DIR/t.IMGFMT.itmd
> -0x20000 0x10000 TEST_DIR/t.IMGFMT
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
> +0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
>
> -=== Test pass 3 ===
> +=== Test pass 1.1 ===
>
> -Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864
> -Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base
> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.itmd
> -wrote 196608/196608 bytes at offset 0
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
> +Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
> +wrote 196608/196608 bytes at offset 2147287040
> 192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 131072/131072 bytes at offset 65536
> +wrote 131072/131072 bytes at offset 2147352576
> 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -wrote 65536/65536 bytes at offset 131072
> +wrote 65536/65536 bytes at offset 2147418112
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> Image committed.
> -read 65536/65536 bytes at offset 0
> +read 196608/196608 bytes at offset 2147287040
> +192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 65536/65536 bytes at offset 2147287040
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 65536/65536 bytes at offset 2147352576
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 65536/65536 bytes at offset 2147418112
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Offset Length File
> +0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
> +Offset Length File
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
> +Offset Length File
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
> +0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
> +
> +=== Test pass 2.0 ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
> +Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
> +wrote 196608/196608 bytes at offset 2147287040
> +192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +wrote 131072/131072 bytes at offset 2147352576
> +128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +wrote 65536/65536 bytes at offset 2147418112
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Image committed.
> +read 196608/196608 bytes at offset 2147287040
> +192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 65536/65536 bytes at offset 2147287040
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 65536/65536 bytes at offset 2147352576
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 65536/65536 bytes at offset 2147418112
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Offset Length File
> +0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
> +Offset Length File
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
> +Offset Length File
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
> +0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
> +
> +=== Test pass 2.1 ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
> +Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
> +wrote 196608/196608 bytes at offset 2147287040
> +192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +wrote 131072/131072 bytes at offset 2147352576
> +128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +wrote 65536/65536 bytes at offset 2147418112
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Image committed.
> +read 196608/196608 bytes at offset 2147287040
> +192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 65536/65536 bytes at offset 2147287040
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 65536/65536 bytes at offset 2147352576
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 65536/65536 bytes at offset 2147418112
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Offset Length File
> +0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
> +Offset Length File
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
> +Offset Length File
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
> +0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
> +
> +=== Test pass 3.0 ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
> +Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
> +wrote 196608/196608 bytes at offset 2147287040
> +192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +wrote 131072/131072 bytes at offset 2147352576
> +128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +wrote 65536/65536 bytes at offset 2147418112
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Image committed.
> +read 65536/65536 bytes at offset 2147287040
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 65536/65536 bytes at offset 2147352576
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 65536/65536 bytes at offset 2147418112
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Offset Length File
> +0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
> +Offset Length File
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
> +Offset Length File
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
> +0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
> +
> +=== Test pass 3.1 ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202009600
> +Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.base
> +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202009600 backing_file=TEST_DIR/t.IMGFMT.itmd
> +wrote 196608/196608 bytes at offset 2147287040
> +192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +wrote 131072/131072 bytes at offset 2147352576
> +128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +wrote 65536/65536 bytes at offset 2147418112
> +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +Image committed.
> +read 65536/65536 bytes at offset 2147287040
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -read 65536/65536 bytes at offset 65536
> +read 65536/65536 bytes at offset 2147352576
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -read 65536/65536 bytes at offset 131072
> +read 65536/65536 bytes at offset 2147418112
> 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> Offset Length File
> -0 0x30000 TEST_DIR/t.IMGFMT.base
> +0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
> Offset Length File
> -0 0x10000 TEST_DIR/t.IMGFMT.base
> -0x10000 0x20000 TEST_DIR/t.IMGFMT.itmd
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
> Offset Length File
> -0 0x10000 TEST_DIR/t.IMGFMT.base
> -0x10000 0x10000 TEST_DIR/t.IMGFMT.itmd
> -0x20000 0x10000 TEST_DIR/t.IMGFMT
> +0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> +0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
> +0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
> *** done
>
I skimmed the test changes, but it looks good.
Reviewed-by: John Snow <jsnow@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.8 v2] qcow2: Don't strand clusters near 2G intervals during commit
2016-12-05 15:49 [Qemu-devel] [PATCH for-2.8 v2] qcow2: Don't strand clusters near 2G intervals during commit Eric Blake
2016-12-05 19:52 ` John Snow
@ 2016-12-06 9:47 ` Stefan Hajnoczi
2016-12-06 14:41 ` [Qemu-devel] " Kevin Wolf
2 siblings, 0 replies; 5+ messages in thread
From: Stefan Hajnoczi @ 2016-12-06 9:47 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, kwolf, stefanha, qemu-block, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 1092 bytes --]
On Mon, Dec 05, 2016 at 09:49:34AM -0600, Eric Blake wrote:
> The qcow2_make_empty() function is reached during 'qemu-img commit',
> in order to clear out ALL clusters of an image. However, if the
> image cannot use the fast code path (true if the image is format
> 0.10, or if the image contains a snapshot), the cluster size is
> larger than 512, and the image is larger than 2G in size, then our
> choice of sector_step causes problems. Since it is not cluster
> aligned, but qcow2_discard_clusters() silently ignores an unaligned
> head or tail, we are leaving clusters allocated.
>
> Enhance the testsuite to expose the flaw, and patch the problem by
> ensuring our step size is aligned.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v2: perform rounding correctly
> ---
> block/qcow2.c | 3 +-
> tests/qemu-iotests/097 | 41 +++++---
> tests/qemu-iotests/097.out | 249 +++++++++++++++++++++++++++++++++------------
> 3 files changed, 210 insertions(+), 83 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.8 v2] qcow2: Don't strand clusters near 2G intervals during commit
2016-12-05 15:49 [Qemu-devel] [PATCH for-2.8 v2] qcow2: Don't strand clusters near 2G intervals during commit Eric Blake
2016-12-05 19:52 ` John Snow
2016-12-06 9:47 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
@ 2016-12-06 14:41 ` Kevin Wolf
2 siblings, 0 replies; 5+ messages in thread
From: Kevin Wolf @ 2016-12-06 14:41 UTC (permalink / raw)
To: Eric Blake; +Cc: qemu-devel, jsnow, qemu-block, stefanha, Max Reitz
Am 05.12.2016 um 16:49 hat Eric Blake geschrieben:
> The qcow2_make_empty() function is reached during 'qemu-img commit',
> in order to clear out ALL clusters of an image. However, if the
> image cannot use the fast code path (true if the image is format
> 0.10, or if the image contains a snapshot), the cluster size is
> larger than 512, and the image is larger than 2G in size, then our
> choice of sector_step causes problems. Since it is not cluster
> aligned, but qcow2_discard_clusters() silently ignores an unaligned
> head or tail, we are leaving clusters allocated.
>
> Enhance the testsuite to expose the flaw, and patch the problem by
> ensuring our step size is aligned.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
Thanks, applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.8 v2] qcow2: Don't strand clusters near 2G intervals during commit
2016-12-05 19:52 ` John Snow
@ 2016-12-06 14:42 ` Eric Blake
0 siblings, 0 replies; 5+ messages in thread
From: Eric Blake @ 2016-12-06 14:42 UTC (permalink / raw)
To: John Snow, qemu-devel; +Cc: kwolf, stefanha, qemu-block, Max Reitz
[-- Attachment #1: Type: text/plain, Size: 2865 bytes --]
On 12/05/2016 01:52 PM, John Snow wrote:
>
>
> On 12/05/2016 10:49 AM, Eric Blake wrote:
>> The qcow2_make_empty() function is reached during 'qemu-img commit',
>> in order to clear out ALL clusters of an image. However, if the
>> image cannot use the fast code path (true if the image is format
>> 0.10, or if the image contains a snapshot), the cluster size is
>> larger than 512, and the image is larger than 2G in size, then our
>> choice of sector_step causes problems. Since it is not cluster
>> aligned, but qcow2_discard_clusters() silently ignores an unaligned
>> head or tail, we are leaving clusters allocated.
>>
>> Enhance the testsuite to expose the flaw, and patch the problem by
>> ensuring our step size is aligned.
>> +++ b/tests/qemu-iotests/097
>> @@ -46,7 +46,7 @@ _supported_proto file
>> _supported_os Linux
>>
>>
>> -# Four passes:
>> +# Four main passes:
>> # 0: Two-layer backing chain, commit to upper backing file (implicitly)
>> # (in this case, the top image will be emptied)
>> # 1: Two-layer backing chain, commit to upper backing file (explicitly)
>> @@ -56,22 +56,30 @@ _supported_os Linux
>> # 3: Two-layer backing chain, commit to lower backing file
>> # (in this case, the top image will implicitly stay unchanged)
>> #
>> +# Each pass is run twice, since qcow2 has different code paths for cleaning
>> +# an image depending on whether it has a snapshot.
>> +#
>> # 020 already tests committing, so this only tests whether image chains are
>> # working properly and that all images above the base are emptied; therefore,
>> -# no complicated patterns are necessary
>> +# no complicated patterns are necessary. Check near the 2G mark, as qcow2
>> +# has been buggy at that boundary in the past.
>> for i in 0 1 2 3; do
>> +for j in 0 1; do
>>
>> echo
>> -echo "=== Test pass $i ==="
>> +echo "=== Test pass $i.$j ==="
>
> I skimmed the test changes, but it looks good.
By the way, it was test 0.0 that fails without the change to qcow2.c
(the changes added the x.0 passes; the pre-existing x.1 passes all
succeed because they use the fast path, and passes 1-3.y succeed because
they aren't clearing all clusters).
Maybe I could have mentioned that in the commit message, and/or split
this into two patches to make the test changes have an easier diff (one
to shift the portion of the test image being written from 0-0x30000 out
to 0x7ffd0000-0x80000000, the other to double the number of passes). But
at this point, it's probably not worth a respin if we're going to get it
in 2.8; but I'm okay if the maintainer wants to add the above paragraph
into the commit message.
>
> Reviewed-by: John Snow <jsnow@redhat.com>
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-12-06 14:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-05 15:49 [Qemu-devel] [PATCH for-2.8 v2] qcow2: Don't strand clusters near 2G intervals during commit Eric Blake
2016-12-05 19:52 ` John Snow
2016-12-06 14:42 ` Eric Blake
2016-12-06 9:47 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-12-06 14:41 ` [Qemu-devel] " Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).