| Message ID | 20200331174455.31792-5-dplotnikov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | qcow2: Implement zstd cluster compression method | expand |
On 3/31/20 12:44 PM, Denis Plotnikov wrote: > The test checks fulfilling qcow2 requiriements for the compression > type feature and zstd compression type operability. > > Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > tests/qemu-iotests/287 | 128 +++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/287.out | 43 +++++++++++++ > tests/qemu-iotests/group | 1 + > 3 files changed, 172 insertions(+) > create mode 100755 tests/qemu-iotests/287 > create mode 100644 tests/qemu-iotests/287.out > > + > +# Check if we can run this test. > + > +IMGOPTS='compression_type=zstd' _make_test_img 64M | grep "Invalid parameter 'zstd'" 2>&1 1>/dev/null > + > +ZSTD_SUPPORTED=$? > + > +if (($ZSTD_SUPPORTED==0)); then This is a bash script, so (()) works; but the bash manual documents that (()) is not idiomatic. Also, what you have is rather verbose... > + _notrun "ZSTD is disabled" > +fi ...I might have written: if IMGOPTS='compression_type=zstd' _make_test_img 64M | grep "Invalid parameter 'zstd'"; then _notrun "ZSTD is disabled" fi > +# Test: an image can't be openned if compression type is zlib and opened > +# incompatible feature compression type is set > +echo > +echo "=== Testing zlib with incompatible bit set ===" > +echo > + > +IMGOPTS='compression_type=zlib' _make_test_img 64M > +$PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 3 > +# to make sure the bit was actually set > +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features > +$QEMU_IMG info "$TEST_IMG" 2>1 1>/dev/null > +if (($?==0)); then > + echo "Error: The image openned successfully. The image must not be openned" twice more > +fi > + > +# Test: an image can't be openned if compression type is NOT zlib and and again. Multiple spots in the file, I'll quit pointing them out. > +# incompatible feature compression type is UNSET > +echo > +echo "=== Testing zstd with incompatible bit unset ===" Why the double space? > +# Test: using zstd compression, write to and read from an image > +echo > +echo "=== Testing reading and writing with zstd ===" > +echo > + > +IMGOPTS='compression_type=zstd' _make_test_img 64M > +$QEMU_IO -c "write -c -P 0xAC 65536 64k " "$TEST_IMG" | _filter_qemu_io That's only one cluster. Wouldn't it be better to write more than one, to prove that we handle back-to-back compressed clusters resulting from back-to-back inputs? > +$QEMU_IO -c "read -P 0xAC 65536 65536 " "$TEST_IMG" | _filter_qemu_io And if you do compress more than one cluster, you may also want to use different patterns over those various clusters. > +$QEMU_IO -c "read -v 131070 8 " "$TEST_IMG" | _filter_qemu_io > +$QEMU_IO -c "read -v 65534 8" "$TEST_IMG" | _filter_qemu_io > + > +# success, all done > +echo "*** done" Is it also worth trying to generate an image with (pseudo-)random contents, and trying qemu-img convert to prove that uncompressable clusters are handled sanely? Ideally, there would be a way to use a fixed PRNG and seed that produces a deterministic sequence that cannot be compressed, but even if we can't meet the ideal, having a test that non-deterministically is likely to generate an uncompressable cluster in most runs is better than nothing (such as 1M of data copied from /dev/urandom, then qemu-img convert on that data).
On 3/31/20 1:43 PM, Eric Blake wrote: > Is it also worth trying to generate an image with (pseudo-)random > contents, and trying qemu-img convert to prove that uncompressable > clusters are handled sanely? Ideally, there would be a way to use a > fixed PRNG and seed that produces a deterministic sequence that cannot > be compressed, but even if we can't meet the ideal, having a test that > non-deterministically is likely to generate an uncompressable cluster in > most runs is better than nothing (such as 1M of data copied from > /dev/urandom, then qemu-img convert on that data). For an example, nbdkit -U - random 1M seed=1234 --run 'qemu-img convert $nbd file' produces 'file' containing deterministic pseudo-random data. And 'zstd file' proves that the data was non-compressible in bulk: $ zstd file file :100.00% (1048576 => 1048613 bytes, file.zst) In fact, inspecting od output, I can see that the first few bytes of the original file are replayed verbatim after a 12-byte header in the .zst file. But we don't have any other uses of nbdkit in our iotests, so I'm not sure if making this the first test to depend on that is ideal.
On 31.03.2020 21:43, Eric Blake wrote: > On 3/31/20 12:44 PM, Denis Plotnikov wrote: >> The test checks fulfilling qcow2 requiriements for the compression >> type feature and zstd compression type operability. >> >> Signed-off-by: Denis Plotnikov <dplotnikov@virtuozzo.com> >> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> tests/qemu-iotests/287 | 128 +++++++++++++++++++++++++++++++++++++ >> tests/qemu-iotests/287.out | 43 +++++++++++++ >> tests/qemu-iotests/group | 1 + >> 3 files changed, 172 insertions(+) >> create mode 100755 tests/qemu-iotests/287 >> create mode 100644 tests/qemu-iotests/287.out >> > >> + >> +# Check if we can run this test. >> + >> +IMGOPTS='compression_type=zstd' _make_test_img 64M | grep "Invalid >> parameter 'zstd'" 2>&1 1>/dev/null >> + >> +ZSTD_SUPPORTED=$? >> + >> +if (($ZSTD_SUPPORTED==0)); then > > This is a bash script, so (()) works; but the bash manual documents > that (()) is not idiomatic. Also, what you have is rather verbose... > >> + _notrun "ZSTD is disabled" >> +fi > > ...I might have written: > > if IMGOPTS='compression_type=zstd' _make_test_img 64M | > grep "Invalid parameter 'zstd'"; then > _notrun "ZSTD is disabled" > fi yes, this on is shorter > > >> +# Test: an image can't be openned if compression type is zlib and > > opened > >> +# incompatible feature compression type is set >> +echo >> +echo "=== Testing zlib with incompatible bit set ===" >> +echo >> + >> +IMGOPTS='compression_type=zlib' _make_test_img 64M >> +$PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 3 >> +# to make sure the bit was actually set >> +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features >> +$QEMU_IMG info "$TEST_IMG" 2>1 1>/dev/null >> +if (($?==0)); then >> + echo "Error: The image openned successfully. The image must not >> be openned" > > twice more > >> +fi >> + >> +# Test: an image can't be openned if compression type is NOT zlib and > > and again. Multiple spots in the file, I'll quit pointing them out. > >> +# incompatible feature compression type is UNSET >> +echo >> +echo "=== Testing zstd with incompatible bit unset ===" > > Why the double space? > >> +# Test: using zstd compression, write to and read from an image >> +echo >> +echo "=== Testing reading and writing with zstd ===" >> +echo >> + >> +IMGOPTS='compression_type=zstd' _make_test_img 64M >> +$QEMU_IO -c "write -c -P 0xAC 65536 64k " "$TEST_IMG" | _filter_qemu_io > > That's only one cluster. Wouldn't it be better to write more than > one, to prove that we handle back-to-back compressed clusters > resulting from back-to-back inputs? I'll add a case for back-to-back compressed clusters > > >> +$QEMU_IO -c "read -P 0xAC 65536 65536 " "$TEST_IMG" | _filter_qemu_io > > And if you do compress more than one cluster, you may also want to use > different patterns over those various clusters. > >> +$QEMU_IO -c "read -v 131070 8 " "$TEST_IMG" | _filter_qemu_io >> +$QEMU_IO -c "read -v 65534 8" "$TEST_IMG" | _filter_qemu_io >> + >> +# success, all done >> +echo "*** done" > > Is it also worth trying to generate an image with (pseudo-)random > contents, and trying qemu-img convert to prove that uncompressable > clusters are handled sanely? Ideally, there would be a way to use a > fixed PRNG and seed that produces a deterministic sequence that cannot > be compressed, but even if we can't meet the ideal, having a test that > non-deterministically is likely to generate an uncompressable cluster > in most runs is better than nothing (such as 1M of data copied from > /dev/urandom, then qemu-img convert on that data). And, I'll try to add a case for incompressible data. Thanks
diff --git a/tests/qemu-iotests/287 b/tests/qemu-iotests/287 new file mode 100755 index 0000000000..49d15b3d43 --- /dev/null +++ b/tests/qemu-iotests/287 @@ -0,0 +1,128 @@ +#!/usr/bin/env bash +# +# Test case for an image using zstd compression +# +# Copyright (c) 2020 Virtuozzo International GmbH +# +# 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=dplotnikov@virtuozzo.com + +seq="$(basename $0)" +echo "QA output created by $seq" + +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# standard environment +. ./common.rc +. ./common.filter + +# This tests qocw2-specific low-level functionality +_supported_fmt qcow2 +_supported_proto file +_supported_os Linux + +# for all the cases +CLUSTER_SIZE=65536 + +# Check if we can run this test. + +IMGOPTS='compression_type=zstd' _make_test_img 64M | grep "Invalid parameter 'zstd'" 2>&1 1>/dev/null + +ZSTD_SUPPORTED=$? + +if (($ZSTD_SUPPORTED==0)); then + _notrun "ZSTD is disabled" +fi + +# Test: when compression is zlib the incompatible bit is unset +echo +echo "=== Testing compression type incompatible bit setting for zlib ===" +echo + +IMGOPTS='compression_type=zlib' _make_test_img 64M +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features + +# Test: when compression differs from zlib the incompatible bit is set +echo +echo "=== Testing compression type incompatible bit setting for zstd ===" +echo + +IMGOPTS='compression_type=zstd' _make_test_img 64M +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features + +# Test: an image can't be openned if compression type is zlib and +# incompatible feature compression type is set +echo +echo "=== Testing zlib with incompatible bit set ===" +echo + +IMGOPTS='compression_type=zlib' _make_test_img 64M +$PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 3 +# to make sure the bit was actually set +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$QEMU_IMG info "$TEST_IMG" 2>1 1>/dev/null +if (($?==0)); then + echo "Error: The image openned successfully. The image must not be openned" +fi + +# Test: an image can't be openned if compression type is NOT zlib and +# incompatible feature compression type is UNSET +echo +echo "=== Testing zstd with incompatible bit unset ===" +echo + +IMGOPTS='compression_type=zstd' _make_test_img 64M +$PYTHON qcow2.py "$TEST_IMG" set-header incompatible_features 0 +# to make sure the bit was actually unset +$PYTHON qcow2.py "$TEST_IMG" dump-header | grep incompatible_features +$QEMU_IMG info "$TEST_IMG" 2>1 1>/dev/null +if (($?==0)); then + echo "Error: The image openned successfully. The image must not be openned" +fi +# Test: check compression type values +echo +echo "=== Testing compression type values ===" +echo +# zlib=0 +IMGOPTS='compression_type=zlib' _make_test_img 64M +od -j104 -N1 -An -vtu1 "$TEST_IMG" + +# zstd=1 +IMGOPTS='compression_type=zstd' _make_test_img 64M +od -j104 -N1 -An -vtu1 "$TEST_IMG" + +# Test: using zstd compression, write to and read from an image +echo +echo "=== Testing reading and writing with zstd ===" +echo + +IMGOPTS='compression_type=zstd' _make_test_img 64M +$QEMU_IO -c "write -c -P 0xAC 65536 64k " "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -P 0xAC 65536 65536 " "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -v 131070 8 " "$TEST_IMG" | _filter_qemu_io +$QEMU_IO -c "read -v 65534 8" "$TEST_IMG" | _filter_qemu_io + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/287.out b/tests/qemu-iotests/287.out new file mode 100644 index 0000000000..8e51c3078d --- /dev/null +++ b/tests/qemu-iotests/287.out @@ -0,0 +1,43 @@ +QA output created by 287 + +=== Testing compression type incompatible bit setting for zlib === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +incompatible_features [] + +=== Testing compression type incompatible bit setting for zstd === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +incompatible_features [3] + +=== Testing zlib with incompatible bit set === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +incompatible_features [3] + +=== Testing zstd with incompatible bit unset === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +incompatible_features [] + +=== Testing compression type values === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 + 0 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 + 1 + +=== Testing reading and writing with zstd === + +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +wrote 65536/65536 bytes at offset 65536 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 65536/65536 bytes at offset 65536 +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +0001fffe: ac ac 00 00 00 00 00 00 ........ +read 8/8 bytes at offset 131070 +8 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +0000fffe: 00 00 ac ac ac ac ac ac ........ +read 8/8 bytes at offset 65534 +8 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +*** done diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group index 79c6dfc85d..dacbcfc12d 100644 --- a/tests/qemu-iotests/group +++ b/tests/qemu-iotests/group @@ -294,5 +294,6 @@ 283 auto quick 284 rw 286 rw quick +287 auto quick 288 quick 289 rw quick