| Message ID | 20161202192223.15153-6-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 02.12.2016 20:22, Eric Blake wrote: > Use blkdebug's new geometry constraints to emulate setups that > have caused recent regression fixes: write zeroes asserting > when running through a loopback block device with max-transfer > smaller than cluster size, and discard rounding away portions > of requests not aligned to preferred boundaries. Also, add > coverage that the block layer is honoring max transfer limits. > > For now, a single iotest performs all actions, with the idea > that we can add future blkdebug constraint test cases in the > same file; but it can be split into multiple iotests if we find > reason to run one portion of the test in more setups than what > are possible in the other. > > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > v3: make comments tied more to test at hand, rather than the > particular hardware that led to the earlier patches being tested > v2: new patch > --- > tests/qemu-iotests/173 | 115 +++++++++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/173.out | 49 +++++++++++++++++++ > tests/qemu-iotests/group | 1 + > 3 files changed, 165 insertions(+) > create mode 100755 tests/qemu-iotests/173 > create mode 100644 tests/qemu-iotests/173.out Reviewed-by: Max Reitz <mreitz@redhat.com>
Am 02.12.2016 um 20:22 hat Eric Blake geschrieben: > Use blkdebug's new geometry constraints to emulate setups that > have caused recent regression fixes: write zeroes asserting > when running through a loopback block device with max-transfer > smaller than cluster size, and discard rounding away portions > of requests not aligned to preferred boundaries. Also, add > coverage that the block layer is honoring max transfer limits. > > For now, a single iotest performs all actions, with the idea > that we can add future blkdebug constraint test cases in the > same file; but it can be split into multiple iotests if we find > reason to run one portion of the test in more setups than what > are possible in the other. > > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > v3: make comments tied more to test at hand, rather than the > particular hardware that led to the earlier patches being tested > v2: new patch > --- > tests/qemu-iotests/173 | 115 +++++++++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/173.out | 49 +++++++++++++++++++ > tests/qemu-iotests/group | 1 + > 3 files changed, 165 insertions(+) > create mode 100755 tests/qemu-iotests/173 > create mode 100644 tests/qemu-iotests/173.out > > diff --git a/tests/qemu-iotests/173 b/tests/qemu-iotests/173 > new file mode 100755 > index 0000000..fd421fc > --- /dev/null > +++ b/tests/qemu-iotests/173 > @@ -0,0 +1,115 @@ > +#!/bin/bash > +# > +# Test corner cases with unusual block geometries > +# > +# Copyright (C) 2016 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=eblake@redhat.com > + > +seq=`basename $0` > +echo "QA output created by $seq" > + > +here=`pwd` > +status=1 # failure is the default! > + > +_cleanup() > +{ > + _cleanup_test_img > +} > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +# get standard environment, filters and checks > +. ./common.rc > +. ./common.filter > + > +_supported_fmt qcow2 > +_supported_proto file > + > +CLUSTER_SIZE=1M > +size=128M > +options=driver=blkdebug,image.driver=qcow2 > + > +echo > +echo "== setting up files ==" > + > +_make_test_img $size > +$QEMU_IO -c "write -P 11 0 $size" "$TEST_IMG" | _filter_qemu_io > +mv "$TEST_IMG" "$TEST_IMG.base" I know that you declared "_supported_proto file", but if you don't use mv after creating the image, it might actually work with other protocols. Most other tests use something like this: TEST_IMG="$TEST_IMG.base" _make_test_img $size And for the qemu-io invocation you can just use the right filename. > +_make_test_img -b "$TEST_IMG.base" > +$QEMU_IO -c "write -P 22 0 $size" "$TEST_IMG" | _filter_qemu_io > + > +# Limited to 64k max-transfer > +echo > +echo "== constrained alignment and max-transfer ==" > +limits=align=4k,max-transfer=64k > +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \ > + -c "write -P 33 1000 128k" -c "read -P 33 1000 128k" | _filter_qemu_io > + > +echo > +echo "== write zero with constrained max-transfer ==" > +limits=align=512,max-transfer=64k,opt-write-zero=$CLUSTER_SIZE > +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \ > + -c "write -z 8003584 2093056" | _filter_qemu_io > + > +# non-power-of-2 write-zero/discard alignments > +echo > +echo "== non-power-of-2 write zeroes ==" "non-power-of-2 write zeroes _limits_". The request offset/size is a power of two. > +limits=align=512,opt-write-zero=15M,max-write-zero=15M,opt-discard=15M,max-discard=15M > +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \ > + -c "write -z 32M 32M" | _filter_qemu_io > + > +echo > +echo "== non-power-of-2 discard ==" Here limits, too. > +limits=align=512,opt-write-zero=15M,max-write-zero=15M,opt-discard=15M,max-discard=15M > +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \ > + -c "discard 80000001 30M" | _filter_qemu_io > + > +echo > +echo "== verify image content ==" > + > +function verify_io() > +{ > + if ($QEMU_IMG info -f "$IMGFMT" "$TEST_IMG" | > + grep "compat: 0.10" > /dev/null); then > + # For v2 images, discarded clusters are read from the backing file > + discarded=11 > + else > + # Discarded clusters are zeroed for v3 or later > + discarded=0 > + fi > + > + echo read -P 22 0 1000 > + echo read -P 33 1000 128k > + echo read -P 22 132072 7871512 > + echo read -P 0 8003584 2093056 > + echo read -P 22 10096640 23457792 > + echo read -P 0 32M 32M > + echo read -P 22 64M 13M > + echo read -P $discarded 77M 29M Hm, why is this exactly 77M? The original request starts at 80000001, 77M is 80740352. We have a discard limit of 15M, but that is only used for splitting the request (and wouldn't match 77M anyway). We still pass the partial requests at the head and the tail to the driver, and what it enforces is align, i.e. 512. The next 512 byte boundary from 80000001 would be 80000512. I'm almost sure that the patch is correct and I'm just missing a piece, but what is it? > + echo read -P 22 106M 22M > +} > + > +verify_io | $QEMU_IO "$TEST_IMG" | _filter_qemu_io > + > +_check_test_img > + > +# success, all done > +echo "*** done" > +status=0 Kevin
On 12/07/2016 10:16 AM, Kevin Wolf wrote: > Am 02.12.2016 um 20:22 hat Eric Blake geschrieben: >> Use blkdebug's new geometry constraints to emulate setups that >> have caused recent regression fixes: write zeroes asserting >> when running through a loopback block device with max-transfer >> smaller than cluster size, and discard rounding away portions >> of requests not aligned to preferred boundaries. Also, add >> coverage that the block layer is honoring max transfer limits. >> >> For now, a single iotest performs all actions, with the idea >> that we can add future blkdebug constraint test cases in the >> same file; but it can be split into multiple iotests if we find >> reason to run one portion of the test in more setups than what >> are possible in the other. >> >> + >> +_supported_fmt qcow2 >> +_supported_proto file >> + >> +CLUSTER_SIZE=1M >> +size=128M >> +options=driver=blkdebug,image.driver=qcow2 >> + >> +echo >> +echo "== setting up files ==" >> + >> +_make_test_img $size >> +$QEMU_IO -c "write -P 11 0 $size" "$TEST_IMG" | _filter_qemu_io >> +mv "$TEST_IMG" "$TEST_IMG.base" > > I know that you declared "_supported_proto file", but if you don't use > mv after creating the image, it might actually work with other > protocols. > > Most other tests use something like this: > > TEST_IMG="$TEST_IMG.base" _make_test_img $size > > And for the qemu-io invocation you can just use the right filename. Thanks. I obviously copied-and-pasted from an earlier test, rather than a later one, so I'll make the tweaks in v4. >> +# non-power-of-2 write-zero/discard alignments >> +echo >> +echo "== non-power-of-2 write zeroes ==" > > "non-power-of-2 write zeroes _limits_". The request offset/size is a > power of two. Indeed. > >> +limits=align=512,opt-write-zero=15M,max-write-zero=15M,opt-discard=15M,max-discard=15M >> +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \ >> + -c "write -z 32M 32M" | _filter_qemu_io >> + >> +echo >> +echo "== non-power-of-2 discard ==" > > Here limits, too. > >> +limits=align=512,opt-write-zero=15M,max-write-zero=15M,opt-discard=15M,max-discard=15M >> +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \ >> + -c "discard 80000001 30M" | _filter_qemu_io >> + >> +echo >> +echo "== verify image content ==" >> + >> +function verify_io() >> +{ >> + if ($QEMU_IMG info -f "$IMGFMT" "$TEST_IMG" | >> + grep "compat: 0.10" > /dev/null); then >> + # For v2 images, discarded clusters are read from the backing file >> + discarded=11 >> + else >> + # Discarded clusters are zeroed for v3 or later >> + discarded=0 >> + fi >> + >> + echo read -P 22 0 1000 >> + echo read -P 33 1000 128k >> + echo read -P 22 132072 7871512 >> + echo read -P 0 8003584 2093056 >> + echo read -P 22 10096640 23457792 >> + echo read -P 0 32M 32M >> + echo read -P 22 64M 13M >> + echo read -P $discarded 77M 29M > > Hm, why is this exactly 77M? > > The original request starts at 80000001, 77M is 80740352. We have a > discard limit of 15M, but that is only used for splitting the request > (and wouldn't match 77M anyway). We still pass the partial requests at > the head and the tail to the driver, and what it enforces is align, i.e. > 512. The next 512 byte boundary from 80000001 would be 80000512. > > I'm almost sure that the patch is correct and I'm just missing a piece, > but what is it? We are using a qcow2 image with 1M clusters. Discarding visible through qcow2 is therefore at 1M boundaries, regardless of whether we can discard at finer granularity elsewhere. Stepping through the request, we have: qemu-io: discard 30M at 80000001, passed to blkdebug blkdebug: discard 511 bytes at 80000001, -ENOTSUP (smaller than blkdebug's 512 align) blkdebug: discard 14371328 bytes at 80000512, passed to qcow2 qcow2: discard 739840 bytes at 80000512, -ENOTSUP (smaller than qcow2's 1M align) qcow2: discard 13M bytes at 77M, succeeds blkdebug: discard 15M bytes at 90M, passed to qcow2 qcow2: discard 15M bytes at 90M, succeeds blkdebug: discard 1356800 bytes at 105M, passed to qcow2 qcow2: discard 1M at 105M, succeeds qcow2: discard 308224 bytes at 106M, -ENOTSUP (smaller than qcow2's 1M align) blkdebug: discard 1 byte at 111457280, -ENOTSUP (smaller than blkdebug's 512 align)
On 12/07/2016 10:34 AM, Eric Blake wrote: > On 12/07/2016 10:16 AM, Kevin Wolf wrote: >> Am 02.12.2016 um 20:22 hat Eric Blake geschrieben: >>> Use blkdebug's new geometry constraints to emulate setups that >>> have caused recent regression fixes: write zeroes asserting >>> when running through a loopback block device with max-transfer >>> smaller than cluster size, and discard rounding away portions >>> of requests not aligned to preferred boundaries. Also, add >>> coverage that the block layer is honoring max transfer limits. >>> >>> + >>> +_supported_fmt qcow2 >>> +_supported_proto file >>> + >>> +CLUSTER_SIZE=1M >>> +size=128M >>> +options=driver=blkdebug,image.driver=qcow2 >>> + >>> +echo >>> +echo "== setting up files ==" >>> + >>> +_make_test_img $size >>> +$QEMU_IO -c "write -P 11 0 $size" "$TEST_IMG" | _filter_qemu_io >>> +mv "$TEST_IMG" "$TEST_IMG.base" >> >> I know that you declared "_supported_proto file", but if you don't use >> mv after creating the image, it might actually work with other >> protocols. >> >> Most other tests use something like this: >> >> TEST_IMG="$TEST_IMG.base" _make_test_img $size >> >> And for the qemu-io invocation you can just use the right filename. > > Thanks. I obviously copied-and-pasted from an earlier test, rather than > a later one, so I'll make the tweaks in v4. Even with the tweaks, I still wasn't able to get things like: ./check -qcow2 -nfs 173 to work. I'm probably missing something trivial, but if someone wants to improve the test as a follow-up, they can be my guest. I'll go ahead and post v4 with the cleaner creation of the backing file, but leave the _supported_proto file line for now.
diff --git a/tests/qemu-iotests/173 b/tests/qemu-iotests/173 new file mode 100755 index 0000000..fd421fc --- /dev/null +++ b/tests/qemu-iotests/173 @@ -0,0 +1,115 @@ +#!/bin/bash +# +# Test corner cases with unusual block geometries +# +# Copyright (C) 2016 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=eblake@redhat.com + +seq=`basename $0` +echo "QA output created by $seq" + +here=`pwd` +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +_supported_fmt qcow2 +_supported_proto file + +CLUSTER_SIZE=1M +size=128M +options=driver=blkdebug,image.driver=qcow2 + +echo +echo "== setting up files ==" + +_make_test_img $size +$QEMU_IO -c "write -P 11 0 $size" "$TEST_IMG" | _filter_qemu_io +mv "$TEST_IMG" "$TEST_IMG.base" +_make_test_img -b "$TEST_IMG.base" +$QEMU_IO -c "write -P 22 0 $size" "$TEST_IMG" | _filter_qemu_io + +# Limited to 64k max-transfer +echo +echo "== constrained alignment and max-transfer ==" +limits=align=4k,max-transfer=64k +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \ + -c "write -P 33 1000 128k" -c "read -P 33 1000 128k" | _filter_qemu_io + +echo +echo "== write zero with constrained max-transfer ==" +limits=align=512,max-transfer=64k,opt-write-zero=$CLUSTER_SIZE +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \ + -c "write -z 8003584 2093056" | _filter_qemu_io + +# non-power-of-2 write-zero/discard alignments +echo +echo "== non-power-of-2 write zeroes ==" + +limits=align=512,opt-write-zero=15M,max-write-zero=15M,opt-discard=15M,max-discard=15M +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \ + -c "write -z 32M 32M" | _filter_qemu_io + +echo +echo "== non-power-of-2 discard ==" + +limits=align=512,opt-write-zero=15M,max-write-zero=15M,opt-discard=15M,max-discard=15M +$QEMU_IO -c "open -o $options,$limits blkdebug::$TEST_IMG" \ + -c "discard 80000001 30M" | _filter_qemu_io + +echo +echo "== verify image content ==" + +function verify_io() +{ + if ($QEMU_IMG info -f "$IMGFMT" "$TEST_IMG" | + grep "compat: 0.10" > /dev/null); then + # For v2 images, discarded clusters are read from the backing file + discarded=11 + else + # Discarded clusters are zeroed for v3 or later + discarded=0 + fi + + echo read -P 22 0 1000 + echo read -P 33 1000 128k + echo read -P 22 132072 7871512 + echo read -P 0 8003584 2093056 + echo read -P 22 10096640 23457792 + echo read -P 0 32M 32M + echo read -P 22 64M 13M + echo read -P $discarded 77M 29M + echo read -P 22 106M 22M +} + +verify_io | $QEMU_IO "$TEST_IMG" | _filter_qemu_io + +_check_test_img + +# success, all done +echo "*** done" +status=0 diff --git a/tests/qemu-iotests/173.out b/tests/qemu-iotests/173.out new file mode 100644 index 0000000..3665c3d --- /dev/null +++ b/tests/qemu-iotests/173.out @@ -0,0 +1,49 @@ +QA output created by 173 + +== setting up files == +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 +wrote 134217728/134217728 bytes at offset 0 +128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file=TEST_DIR/t.IMGFMT.base +wrote 134217728/134217728 bytes at offset 0 +128 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +== constrained alignment and max-transfer == +wrote 131072/131072 bytes at offset 1000 +128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 131072/131072 bytes at offset 1000 +128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +== write zero with constrained max-transfer == +wrote 2093056/2093056 bytes at offset 8003584 +1.996 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +== non-power-of-2 write zeroes == +wrote 33554432/33554432 bytes at offset 33554432 +32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +== non-power-of-2 discard == +discard 31457280/31457280 bytes at offset 80000001 +30 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +== verify image content == +read 1000/1000 bytes at offset 0 +1000 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 131072/131072 bytes at offset 1000 +128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 7871512/7871512 bytes at offset 132072 +7.507 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 2093056/2093056 bytes at offset 8003584 +1.996 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 23457792/23457792 bytes at offset 10096640 +22.371 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 33554432/33554432 bytes at offset 33554432 +32 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 13631488/13631488 bytes at offset 67108864 +13 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 30408704/30408704 bytes at offset 80740352 +29 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 23068672/23068672 bytes at offset 111149056 +22 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +No errors were found on the image. +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 866c1a0..0453e6c 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -165,3 +165,4 @@ 170 rw auto quick 171 rw auto quick 172 auto +173 rw auto quick
Use blkdebug's new geometry constraints to emulate setups that have caused recent regression fixes: write zeroes asserting when running through a loopback block device with max-transfer smaller than cluster size, and discard rounding away portions of requests not aligned to preferred boundaries. Also, add coverage that the block layer is honoring max transfer limits. For now, a single iotest performs all actions, with the idea that we can add future blkdebug constraint test cases in the same file; but it can be split into multiple iotests if we find reason to run one portion of the test in more setups than what are possible in the other. Signed-off-by: Eric Blake <eblake@redhat.com> --- v3: make comments tied more to test at hand, rather than the particular hardware that led to the earlier patches being tested v2: new patch --- tests/qemu-iotests/173 | 115 +++++++++++++++++++++++++++++++++++++++++++++ tests/qemu-iotests/173.out | 49 +++++++++++++++++++ tests/qemu-iotests/group | 1 + 3 files changed, 165 insertions(+) create mode 100755 tests/qemu-iotests/173 create mode 100644 tests/qemu-iotests/173.out