[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v14 4/4] iotests: 287: add qcow2 compression type test

From: Eric Blake
Subject: Re: [PATCH v14 4/4] iotests: 287: add qcow2 compression type test
Date: Tue, 31 Mar 2020 13:43:57 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0

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 <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
  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
+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"

...I might have written:

if IMGOPTS='compression_type=zstd' _make_test_img 64M |
    grep "Invalid parameter 'zstd'"; then
    _notrun "ZSTD is disabled"

+# Test: an image can't be openned if compression type is zlib and


+#       incompatible feature compression type is set
+echo "=== Testing zlib with incompatible bit set  ==="
+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

+# 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 "=== Testing zstd with incompatible bit unset  ==="

Why the double space?

+# Test: using zstd compression, write to and read from an image
+echo "=== Testing reading and writing with zstd ==="
+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).

Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

reply via email to

[Prev in Thread] Current Thread [Next in Thread]