* [Qemu-devel] [PATCH v7 1/3] iotests: fix 097 when run with qcow
2017-03-30 22:36 [Qemu-devel] [PATCH for-2.9 v7 0/3] Fix wipe of unaligned qcow2 image [was add blkdebug tests] Eric Blake
@ 2017-03-30 22:36 ` Eric Blake
2017-03-30 22:36 ` [Qemu-devel] [PATCH v7 2/3] iotests: Improve image-clear tests on non-aligned image Eric Blake
2017-03-30 22:36 ` [Qemu-devel] [PATCH v7 3/3] qcow2: Discard unaligned tail when wiping image Eric Blake
2 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2017-03-30 22:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz, Daniel P. Berrange
From: "Daniel P. Berrange" <berrange@redhat.com>
The previous commit:
commit a3e1505daec31ef56f0489f8c8fff1b8e4ca92bd
Author: Eric Blake <eblake@redhat.com>
Date: Mon Dec 5 09:49:34 2016 -0600
qcow2: Don't strand clusters near 2G intervals during commit
extended the 097 test case so that it did two passes, once
with an internal snapshot, once without.
qcow (v1) does not support internal snapshots, so this change
broke test 097 when run against qcow.
This splits 097 in two, creating a new 176 that tests the
internal snapshot codepath, effectively putting 097 back
to its content before the above commit.
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Message-Id: <20170221115512.21918-8-berrange@redhat.com>
[eblake: test collisions: s/173/176/g]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/qemu-iotests/097 | 10 +---
tests/qemu-iotests/097.out | 125 ++------------------------------------------
tests/qemu-iotests/176 | 126 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/176.out | 119 ++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
5 files changed, 251 insertions(+), 130 deletions(-)
create mode 100755 tests/qemu-iotests/176
create mode 100644 tests/qemu-iotests/176.out
diff --git a/tests/qemu-iotests/097 b/tests/qemu-iotests/097
index 4c33e80..1d28aff 100755
--- a/tests/qemu-iotests/097
+++ b/tests/qemu-iotests/097
@@ -56,26 +56,19 @@ _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. 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.$j ==="
+echo "=== Test pass $i ==="
echo
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 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
$QEMU_IO -c 'write -P 2 0x7ffe0000 128k' "$TEST_IMG.itmd" | _filter_qemu_io
@@ -121,7 +114,6 @@ $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 8106cc9..81fc225 100644
--- a/tests/qemu-iotests/097.out
+++ b/tests/qemu-iotests/097.out
@@ -1,6 +1,6 @@
QA output created by 097
-=== Test pass 0.0 ===
+=== Test pass 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
@@ -29,66 +29,7 @@ Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
-=== Test pass 0.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 0x20000 TEST_DIR/t.IMGFMT.itmd
-
-=== Test pass 1.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 1.1 ===
+=== Test pass 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
@@ -118,7 +59,7 @@ Offset Length File
0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
-=== Test pass 2.0 ===
+=== Test pass 2 ===
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
@@ -148,65 +89,7 @@ Offset Length File
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 ===
+=== Test pass 3 ===
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
diff --git a/tests/qemu-iotests/176 b/tests/qemu-iotests/176
new file mode 100755
index 0000000..9a70a87
--- /dev/null
+++ b/tests/qemu-iotests/176
@@ -0,0 +1,126 @@
+#!/bin/bash
+#
+# Commit changes into backing chains and empty the top image if the
+# backing image is not explicitly specified.
+#
+# Variant of 097, which includes snapshots to test different codepath
+# in qcow2
+#
+# Copyright (C) 2014 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+ _rm_test_img "$TEST_IMG.itmd"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+# Any format supporting backing files and bdrv_make_empty
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+
+# Four 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)
+# (in this case, the top image will implicitly stay unchanged)
+# 2: Two-layer backing chain, commit to upper backing file (implicitly with -d)
+# (in this case, the top image will explicitly stay unchanged)
+# 3: Two-layer backing chain, commit to lower backing file
+# (in this case, the top image will implicitly stay unchanged)
+#
+# 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. Check near the 2G mark, as qcow2
+# has been buggy at that boundary in the past.
+for i in 0 1 2 3; do
+
+echo
+echo "=== Test pass $i ==="
+echo
+
+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
+$QEMU_IMG snapshot -c snap "$TEST_IMG"
+
+$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
+ # -b "$TEST_IMG.itmd" should be the default (that is, committing to the
+ # first backing file in the chain)
+ $QEMU_IMG commit "$TEST_IMG"
+ elif [ $i == 1 ]; then
+ # explicitly specify the commit target (this should imply -d)
+ $QEMU_IMG commit -b "$TEST_IMG.itmd" "$TEST_IMG"
+ else
+ # do not explicitly specify the commit target, but use -d to leave the
+ # top image unchanged
+ $QEMU_IMG commit -d "$TEST_IMG"
+ fi
+
+ # Bottom should be unchanged
+ $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 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)
+else
+ $QEMU_IMG commit -b "$TEST_IMG.base" "$TEST_IMG"
+
+ # Bottom should contain all changes
+ $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
+
+$QEMU_IMG map "$TEST_IMG.base" | _filter_qemu_img_map
+$QEMU_IMG map "$TEST_IMG.itmd" | _filter_qemu_img_map
+$QEMU_IMG map "$TEST_IMG" | _filter_qemu_img_map
+
+done
+
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/176.out b/tests/qemu-iotests/176.out
new file mode 100644
index 0000000..fc63653
--- /dev/null
+++ b/tests/qemu-iotests/176.out
@@ -0,0 +1,119 @@
+QA output created by 176
+
+=== Test pass 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 0x20000 TEST_DIR/t.IMGFMT.itmd
+
+=== Test pass 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 2 ===
+
+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 ===
+
+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
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1f4bf03..43142dd 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -168,3 +168,4 @@
173 rw auto
174 auto
175 auto quick
+176 rw auto backing
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH v7 2/3] iotests: Improve image-clear tests on non-aligned image
2017-03-30 22:36 [Qemu-devel] [PATCH for-2.9 v7 0/3] Fix wipe of unaligned qcow2 image [was add blkdebug tests] Eric Blake
2017-03-30 22:36 ` [Qemu-devel] [PATCH v7 1/3] iotests: fix 097 when run with qcow Eric Blake
@ 2017-03-30 22:36 ` Eric Blake
2017-03-31 12:40 ` Max Reitz
2017-03-30 22:36 ` [Qemu-devel] [PATCH v7 3/3] qcow2: Discard unaligned tail when wiping image Eric Blake
2 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2017-03-30 22:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz
Tweak 097 and 176 to operate on an image that is not cluster-aligned,
to give further coverage of clearing out an entire image.
Tested for qcow (97) and qcow2 (97 and 176).
The fact that there is a subtle difference between the expected
outputs of 97 and 176 on pass 0 is evidence that we still don't
always clear out the full image in qcow2. Commit a3e1505 fixed
the case of clusters stranded at the 2G mark, but did not fix
stranding an unaligned tail. That will be the next patch, kept
separate to make it easier to see the impact.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v7: also write data in the unaligned cluster, and document bug it exposes
v6: new patch
---
tests/qemu-iotests/097 | 17 +++++++++-----
tests/qemu-iotests/097.out | 55 ++++++++++++++++++++++++++++++++++++----------
tests/qemu-iotests/176 | 17 +++++++++-----
tests/qemu-iotests/176.out | 55 ++++++++++++++++++++++++++++++++++++----------
4 files changed, 108 insertions(+), 36 deletions(-)
diff --git a/tests/qemu-iotests/097 b/tests/qemu-iotests/097
index 1d28aff..e22670c 100755
--- a/tests/qemu-iotests/097
+++ b/tests/qemu-iotests/097
@@ -66,13 +66,15 @@ echo
echo "=== Test pass $i ==="
echo
-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
+len=$((2100 * 1024 * 1024 + 512)) # larger than 2G, and not cluster aligned
+TEST_IMG="$TEST_IMG.base" _make_test_img $len
+TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" $len
+_make_test_img -b "$TEST_IMG.itmd" $len
-$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
+$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
+$QEMU_IO -c "write -P 4 $(($len - 512)) 512" "$TEST_IMG" | _filter_qemu_io
if [ $i -lt 3 ]; then
if [ $i == 0 ]; then
@@ -90,11 +92,13 @@ if [ $i -lt 3 ]; then
# Bottom should be unchanged
$QEMU_IO -c 'read -P 1 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
+ $QEMU_IO -c "read -P 0 $((len - 512)) 512" "$TEST_IMG.base" | _filter_qemu_io
# Intermediate should contain changes from top
$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
+ $QEMU_IO -c "read -P 4 $((len - 512)) 512" "$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)
@@ -105,6 +109,7 @@ else
$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
+ $QEMU_IO -c "read -P 4 $((len - 512)) 512" "$TEST_IMG.base" | _filter_qemu_io
# Both top and intermediate should be unchanged
fi
diff --git a/tests/qemu-iotests/097.out b/tests/qemu-iotests/097.out
index 81fc225..f6705a1 100644
--- a/tests/qemu-iotests/097.out
+++ b/tests/qemu-iotests/097.out
@@ -2,104 +2,130 @@ QA output created by 097
=== Test pass 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
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 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)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, 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 512/512 bytes at offset 2202009600
+512 bytes, 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)
+read 512/512 bytes at offset 2202009600
+512 bytes, 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
+0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
=== Test pass 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
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 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)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, 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 512/512 bytes at offset 2202009600
+512 bytes, 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)
+read 512/512 bytes at offset 2202009600
+512 bytes, 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
+0x83400000 0x200 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
+0x83400000 0x200 TEST_DIR/t.IMGFMT
=== Test pass 2 ===
-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
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 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)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, 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 512/512 bytes at offset 2202009600
+512 bytes, 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)
+read 512/512 bytes at offset 2202009600
+512 bytes, 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
+0x83400000 0x200 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
+0x83400000 0x200 TEST_DIR/t.IMGFMT
=== Test pass 3 ===
-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
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 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)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, 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)
@@ -107,13 +133,18 @@ 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)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
+0x83400000 0x200 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+0x83400000 0x200 TEST_DIR/t.IMGFMT
*** done
diff --git a/tests/qemu-iotests/176 b/tests/qemu-iotests/176
index 9a70a87..950b287 100755
--- a/tests/qemu-iotests/176
+++ b/tests/qemu-iotests/176
@@ -69,14 +69,16 @@ echo
echo "=== Test pass $i ==="
echo
-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
+len=$((2100 * 1024 * 1024 + 512)) # larger than 2G, and not cluster aligned
+TEST_IMG="$TEST_IMG.base" _make_test_img $len
+TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" $len
+_make_test_img -b "$TEST_IMG.itmd" $len
$QEMU_IMG snapshot -c snap "$TEST_IMG"
-$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
+$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
+$QEMU_IO -c "write -P 4 $(($len - 512)) 512" "$TEST_IMG" | _filter_qemu_io
if [ $i -lt 3 ]; then
if [ $i == 0 ]; then
@@ -94,11 +96,13 @@ if [ $i -lt 3 ]; then
# Bottom should be unchanged
$QEMU_IO -c 'read -P 1 0x7ffd0000 192k' "$TEST_IMG.base" | _filter_qemu_io
+ $QEMU_IO -c "read -P 0 $((len - 512)) 512" "$TEST_IMG.base" | _filter_qemu_io
# Intermediate should contain changes from top
$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
+ $QEMU_IO -c "read -P 4 $((len - 512)) 512" "$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)
@@ -109,6 +113,7 @@ else
$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
+ $QEMU_IO -c "read -P 4 $((len - 512)) 512" "$TEST_IMG.base" | _filter_qemu_io
# Both top and intermediate should be unchanged
fi
diff --git a/tests/qemu-iotests/176.out b/tests/qemu-iotests/176.out
index fc63653..990a41c 100644
--- a/tests/qemu-iotests/176.out
+++ b/tests/qemu-iotests/176.out
@@ -2,104 +2,130 @@ QA output created by 176
=== Test pass 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
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 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)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, 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 512/512 bytes at offset 2202009600
+512 bytes, 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)
+read 512/512 bytes at offset 2202009600
+512 bytes, 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
+0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT
=== Test pass 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
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 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)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, 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 512/512 bytes at offset 2202009600
+512 bytes, 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)
+read 512/512 bytes at offset 2202009600
+512 bytes, 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
+0x83400000 0x200 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
+0x83400000 0x200 TEST_DIR/t.IMGFMT
=== Test pass 2 ===
-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
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 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)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, 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 512/512 bytes at offset 2202009600
+512 bytes, 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)
+read 512/512 bytes at offset 2202009600
+512 bytes, 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
+0x83400000 0x200 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
+0x83400000 0x200 TEST_DIR/t.IMGFMT
=== Test pass 3 ===
-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
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=2202010112
+Formatting 'TEST_DIR/t.IMGFMT.itmd', fmt=IMGFMT size=2202010112 backing_file=TEST_DIR/t.IMGFMT.base
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2202010112 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)
+wrote 512/512 bytes at offset 2202009600
+512 bytes, 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)
@@ -107,13 +133,18 @@ 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)
+read 512/512 bytes at offset 2202009600
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
Offset Length File
0x7ffd0000 0x30000 TEST_DIR/t.IMGFMT.base
+0x83400000 0x200 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
+0x83400000 0x200 TEST_DIR/t.IMGFMT.base
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x10000 TEST_DIR/t.IMGFMT.itmd
0x7fff0000 0x10000 TEST_DIR/t.IMGFMT
+0x83400000 0x200 TEST_DIR/t.IMGFMT
*** done
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v7 2/3] iotests: Improve image-clear tests on non-aligned image
2017-03-30 22:36 ` [Qemu-devel] [PATCH v7 2/3] iotests: Improve image-clear tests on non-aligned image Eric Blake
@ 2017-03-31 12:40 ` Max Reitz
0 siblings, 0 replies; 9+ messages in thread
From: Max Reitz @ 2017-03-31 12:40 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: kwolf, qemu-block
[-- Attachment #1: Type: text/plain, Size: 1633 bytes --]
On 31.03.2017 00:36, Eric Blake wrote:
> Tweak 097 and 176 to operate on an image that is not cluster-aligned,
> to give further coverage of clearing out an entire image.
>
> Tested for qcow (97) and qcow2 (97 and 176).
>
> The fact that there is a subtle difference between the expected
> outputs of 97 and 176 on pass 0 is evidence that we still don't
> always clear out the full image in qcow2. Commit a3e1505 fixed
> the case of clusters stranded at the 2G mark, but did not fix
> stranding an unaligned tail. That will be the next patch, kept
> separate to make it easier to see the impact.
I'm not sure I like this very much. The reference output should always
be what we actually expect it to be. Therefore, usually the issue is
fixed first and the test is added only afterwards.
If I want to see the impact I can just do what I always do in those
cases: Go to the base commit, compile qemu, step forward to commits to
the test, run the test, see it fail, compile qemu again, run the test,
see it pass.
So would it be possible to switch this and patch 3?
(Apart from that, this patch looks good.)
Max
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v7: also write data in the unaligned cluster, and document bug it exposes
> v6: new patch
> ---
> tests/qemu-iotests/097 | 17 +++++++++-----
> tests/qemu-iotests/097.out | 55 ++++++++++++++++++++++++++++++++++++----------
> tests/qemu-iotests/176 | 17 +++++++++-----
> tests/qemu-iotests/176.out | 55 ++++++++++++++++++++++++++++++++++++----------
> 4 files changed, 108 insertions(+), 36 deletions(-)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v7 3/3] qcow2: Discard unaligned tail when wiping image
2017-03-30 22:36 [Qemu-devel] [PATCH for-2.9 v7 0/3] Fix wipe of unaligned qcow2 image [was add blkdebug tests] Eric Blake
2017-03-30 22:36 ` [Qemu-devel] [PATCH v7 1/3] iotests: fix 097 when run with qcow Eric Blake
2017-03-30 22:36 ` [Qemu-devel] [PATCH v7 2/3] iotests: Improve image-clear tests on non-aligned image Eric Blake
@ 2017-03-30 22:36 ` Eric Blake
2017-03-31 12:51 ` Max Reitz
2 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2017-03-30 22:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, qemu-block, mreitz
The previous commit pointed out a subtle difference between the
fast and slow path of qcow2_make_empty(), where we failed to discard
the final (partial) cluster of an unaligned image.
The problem stems from the fact that qcow2_discard_clusters() was
silently ignoring sub-cluster head and tail on unaligned requests.
A quick audit of all callers shows that qcow2_snapshot_create() has
always passed a cluster-aligned request since the call was added
in commit 1ebf561; qcow2_co_pdiscard() has passed a cluster-aligned
request since commit ecdbead taught the block layer about preferred
discard alignment; and qcow2_make_empty() was fixed to pass an
aligned start (but not necessarily end) in commit a3e1505.
Asserting that the start is always aligned also points out that we
now have a dead check: rounding the end offset down can never result
in a value less than the aligned start offset (the check was rendered
dead with commit ecdbead). Meanwhile, we do not want to round the
end cluster down in the one case of the end offset matching the
(unaligned) file size - that final partial cluster should still be
discarded.
With those fixes in place, we can adjust the testsuite so that
qemu-iotests 97 and 176 are once again identical for qcow2, showing
that fast and slow paths are back in sync.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
v7: new patch
---
block/qcow2-cluster.c | 10 ++++------
tests/qemu-iotests/176.out | 2 +-
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 78c11d4..100398c 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1519,12 +1519,10 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
end_offset = offset + (nb_sectors << BDRV_SECTOR_BITS);
- /* Round start up and end down */
- offset = align_offset(offset, s->cluster_size);
- end_offset = start_of_cluster(s, end_offset);
-
- if (offset > end_offset) {
- return 0;
+ /* The caller must cluster-align start; round end down except at EOF */
+ assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
+ if (end_offset != bs->total_sectors * BDRV_SECTOR_SIZE) {
+ end_offset = start_of_cluster(s, end_offset);
}
nb_clusters = size_to_clusters(s, end_offset - offset);
diff --git a/tests/qemu-iotests/176.out b/tests/qemu-iotests/176.out
index 990a41c..6271fa7 100644
--- a/tests/qemu-iotests/176.out
+++ b/tests/qemu-iotests/176.out
@@ -35,7 +35,7 @@ Offset Length File
Offset Length File
0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
-0x83400000 0x200 TEST_DIR/t.IMGFMT
+0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
=== Test pass 1 ===
--
2.9.3
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v7 3/3] qcow2: Discard unaligned tail when wiping image
2017-03-30 22:36 ` [Qemu-devel] [PATCH v7 3/3] qcow2: Discard unaligned tail when wiping image Eric Blake
@ 2017-03-31 12:51 ` Max Reitz
2017-03-31 13:56 ` Eric Blake
0 siblings, 1 reply; 9+ messages in thread
From: Max Reitz @ 2017-03-31 12:51 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: kwolf, qemu-block
[-- Attachment #1: Type: text/plain, Size: 4010 bytes --]
On 31.03.2017 00:36, Eric Blake wrote:
> The previous commit pointed out a subtle difference between the
> fast and slow path of qcow2_make_empty(), where we failed to discard
> the final (partial) cluster of an unaligned image.
>
> The problem stems from the fact that qcow2_discard_clusters() was
> silently ignoring sub-cluster head and tail on unaligned requests.
> A quick audit of all callers shows that qcow2_snapshot_create() has
> always passed a cluster-aligned request since the call was added
> in commit 1ebf561; qcow2_co_pdiscard() has passed a cluster-aligned
> request since commit ecdbead taught the block layer about preferred
> discard alignment; and qcow2_make_empty() was fixed to pass an
> aligned start (but not necessarily end) in commit a3e1505.
>
> Asserting that the start is always aligned also points out that we
> now have a dead check: rounding the end offset down can never result
> in a value less than the aligned start offset (the check was rendered
> dead with commit ecdbead). Meanwhile, we do not want to round the
> end cluster down in the one case of the end offset matching the
> (unaligned) file size - that final partial cluster should still be
> discarded.
>
> With those fixes in place, we can adjust the testsuite so that
> qemu-iotests 97 and 176 are once again identical for qcow2, showing
> that fast and slow paths are back in sync.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v7: new patch
> ---
> block/qcow2-cluster.c | 10 ++++------
> tests/qemu-iotests/176.out | 2 +-
> 2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 78c11d4..100398c 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1519,12 +1519,10 @@ int qcow2_discard_clusters(BlockDriverState *bs, uint64_t offset,
>
> end_offset = offset + (nb_sectors << BDRV_SECTOR_BITS);
>
> - /* Round start up and end down */
> - offset = align_offset(offset, s->cluster_size);
> - end_offset = start_of_cluster(s, end_offset);
> -
> - if (offset > end_offset) {
> - return 0;
> + /* The caller must cluster-align start; round end down except at EOF */
> + assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
> + if (end_offset != bs->total_sectors * BDRV_SECTOR_SIZE) {
> + end_offset = start_of_cluster(s, end_offset);
> }
This change looks good and works for qcow2_make_empty(), but
qcow2_co_pdiscard() will still drop these requests:
$ ./qemu-img create -f qcow2 foo.qcow2 512
Formatting 'foo.qcow2', fmt=qcow2 size=512 encryption=off
cluster_size=65536 lazy_refcounts=off refcount_bits=16
$ ./qemu-io -c 'write 0 512' foo.qcow2
wrote 512/512 bytes at offset 0
512 bytes, 1 ops; 0.0062 sec (80.167 KiB/sec and 160.3335 ops/sec)
$ ./qemu-img map foo.qcow2
Offset Length Mapped to File
0 0x200 0x50000 foo.qcow2
$ ./qemu-io -c 'discard 0 512' foo.qcow2
discard 512/512 bytes at offset 0
512 bytes, 1 ops; 0.0000 sec (34.877 MiB/sec and 71428.5714 ops/sec)
$ ./qemu-img map foo.qcow2
Offset Length Mapped to File
0 0x200 0x50000 foo.qcow2
We don't necessarily have to fix it for 2.9, so:
Reviewed-by: Max Reitz <mreitz@redhat.com>
>
> nb_clusters = size_to_clusters(s, end_offset - offset);
> diff --git a/tests/qemu-iotests/176.out b/tests/qemu-iotests/176.out
> index 990a41c..6271fa7 100644
> --- a/tests/qemu-iotests/176.out
> +++ b/tests/qemu-iotests/176.out
> @@ -35,7 +35,7 @@ Offset Length File
> Offset Length File
> 0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
> 0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
> -0x83400000 0x200 TEST_DIR/t.IMGFMT
> +0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
>
> === Test pass 1 ===
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v7 3/3] qcow2: Discard unaligned tail when wiping image
2017-03-31 12:51 ` Max Reitz
@ 2017-03-31 13:56 ` Eric Blake
2017-03-31 14:01 ` Max Reitz
0 siblings, 1 reply; 9+ messages in thread
From: Eric Blake @ 2017-03-31 13:56 UTC (permalink / raw)
To: Max Reitz, qemu-devel; +Cc: kwolf, qemu-block
[-- Attachment #1: Type: text/plain, Size: 2593 bytes --]
On 03/31/2017 07:51 AM, Max Reitz wrote:
> On 31.03.2017 00:36, Eric Blake wrote:
>> The previous commit pointed out a subtle difference between the
>> fast and slow path of qcow2_make_empty(), where we failed to discard
>> the final (partial) cluster of an unaligned image.
>>
>> + /* The caller must cluster-align start; round end down except at EOF */
>> + assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
>> + if (end_offset != bs->total_sectors * BDRV_SECTOR_SIZE) {
>> + end_offset = start_of_cluster(s, end_offset);
>> }
>
> This change looks good and works for qcow2_make_empty(), but
> qcow2_co_pdiscard() will still drop these requests:
Yes, but qcow2_co_pdiscard() is advisory, and leaving the last partial
cluster undiscarded (consistent for what we do for all other partial
cluster requests) is different than our documented notion that
qcow2_make_empty() gets rid of all clusters no matter what.
> We don't necessarily have to fix it for 2.9, so:
Agreed that enhancing pdiscard to special-case a partial cluster at EOF
is not a bug fix, and therefore 2.10 material if we even want it.
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>
>
>>
>> nb_clusters = size_to_clusters(s, end_offset - offset);
>> diff --git a/tests/qemu-iotests/176.out b/tests/qemu-iotests/176.out
>> index 990a41c..6271fa7 100644
>> --- a/tests/qemu-iotests/176.out
>> +++ b/tests/qemu-iotests/176.out
>> @@ -35,7 +35,7 @@ Offset Length File
>> Offset Length File
>> 0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
>> 0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
>> -0x83400000 0x200 TEST_DIR/t.IMGFMT
>> +0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
As to your comment about swapping patches 2 and 3, if I did that, then I
would not have this change to 176.out during the bug fix, and would
instead squash this line into the single patch touching the testsuite to
add the enhancement. How important is the swapped order? Do I need to
respin for that, or is it something a maintainer could tweak, or is the
series fine as-is? For what it's worth, the policy of single test after
the patch is somewhat opposite of Markus' approach of test first showing
the buggy behavior, then patch that includes the testsuite fix to match
the patch. But I can live with either order, so I won't respin without
an explicit request to do so.
--
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] 9+ messages in thread* Re: [Qemu-devel] [PATCH v7 3/3] qcow2: Discard unaligned tail when wiping image
2017-03-31 13:56 ` Eric Blake
@ 2017-03-31 14:01 ` Max Reitz
2017-03-31 14:11 ` Eric Blake
0 siblings, 1 reply; 9+ messages in thread
From: Max Reitz @ 2017-03-31 14:01 UTC (permalink / raw)
To: Eric Blake, qemu-devel; +Cc: kwolf, qemu-block
[-- Attachment #1: Type: text/plain, Size: 3075 bytes --]
On 31.03.2017 15:56, Eric Blake wrote:
> On 03/31/2017 07:51 AM, Max Reitz wrote:
>> On 31.03.2017 00:36, Eric Blake wrote:
>>> The previous commit pointed out a subtle difference between the
>>> fast and slow path of qcow2_make_empty(), where we failed to discard
>>> the final (partial) cluster of an unaligned image.
>>>
>
>>> + /* The caller must cluster-align start; round end down except at EOF */
>>> + assert(QEMU_IS_ALIGNED(offset, s->cluster_size));
>>> + if (end_offset != bs->total_sectors * BDRV_SECTOR_SIZE) {
>>> + end_offset = start_of_cluster(s, end_offset);
>>> }
>>
>> This change looks good and works for qcow2_make_empty(), but
>> qcow2_co_pdiscard() will still drop these requests:
>
> Yes, but qcow2_co_pdiscard() is advisory, and leaving the last partial
> cluster undiscarded (consistent for what we do for all other partial
> cluster requests) is different than our documented notion that
> qcow2_make_empty() gets rid of all clusters no matter what.
>
>> We don't necessarily have to fix it for 2.9, so:
>
> Agreed that enhancing pdiscard to special-case a partial cluster at EOF
> is not a bug fix, and therefore 2.10 material if we even want it.
Why would we not want it? :-)
>
>>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>>
>>>
>>> nb_clusters = size_to_clusters(s, end_offset - offset);
>>> diff --git a/tests/qemu-iotests/176.out b/tests/qemu-iotests/176.out
>>> index 990a41c..6271fa7 100644
>>> --- a/tests/qemu-iotests/176.out
>>> +++ b/tests/qemu-iotests/176.out
>>> @@ -35,7 +35,7 @@ Offset Length File
>>> Offset Length File
>>> 0x7ffd0000 0x10000 TEST_DIR/t.IMGFMT.base
>>> 0x7ffe0000 0x20000 TEST_DIR/t.IMGFMT.itmd
>>> -0x83400000 0x200 TEST_DIR/t.IMGFMT
>>> +0x83400000 0x200 TEST_DIR/t.IMGFMT.itmd
>
> As to your comment about swapping patches 2 and 3, if I did that, then I
> would not have this change to 176.out during the bug fix, and would
> instead squash this line into the single patch touching the testsuite to
> add the enhancement.
Right.
> How important is the swapped order?
As you can probably guess, technically not important. But I having
reference outputs that are not actually a reference kind of defeats the
purpose in my opinion.
> Do I need to
> respin for that, or is it something a maintainer could tweak, or is the
> series fine as-is?
Of course I can fix the code, but changing the commit messages is a bit
more involved...
> For what it's worth, the policy of single test after
> the patch is somewhat opposite of Markus' approach of test first showing
> the buggy behavior, then patch that includes the testsuite fix to match
> the patch. But I can live with either order, so I won't respin without
> an explicit request to do so.
Well, then consider this an explicit request. :-)
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Qemu-devel] [PATCH v7 3/3] qcow2: Discard unaligned tail when wiping image
2017-03-31 14:01 ` Max Reitz
@ 2017-03-31 14:11 ` Eric Blake
0 siblings, 0 replies; 9+ messages in thread
From: Eric Blake @ 2017-03-31 14:11 UTC (permalink / raw)
To: Max Reitz, qemu-devel; +Cc: kwolf, qemu-block
[-- Attachment #1: Type: text/plain, Size: 862 bytes --]
On 03/31/2017 09:01 AM, Max Reitz wrote:
>> Do I need to
>> respin for that, or is it something a maintainer could tweak, or is the
>> series fine as-is?
>
> Of course I can fix the code, but changing the commit messages is a bit
> more involved...
>
>> For what it's worth, the policy of single test after
>> the patch is somewhat opposite of Markus' approach of test first showing
>> the buggy behavior, then patch that includes the testsuite fix to match
>> the patch. But I can live with either order, so I won't respin without
>> an explicit request to do so.
>
> Well, then consider this an explicit request. :-)
Yes sir! v8 coming right up.
--
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] 9+ messages in thread