[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 20/21] iotests: Disable data_file where it cannot be used
|
From: |
Maxim Levitsky |
|
Subject: |
Re: [PATCH v2 20/21] iotests: Disable data_file where it cannot be used |
|
Date: |
Wed, 06 Nov 2019 17:52:20 +0200 |
On Tue, 2019-10-15 at 16:27 +0200, Max Reitz wrote:
> Signed-off-by: Max Reitz <address@hidden>
> ---
> tests/qemu-iotests/007 | 5 +++--
> tests/qemu-iotests/014 | 2 ++
> tests/qemu-iotests/015 | 5 +++--
> tests/qemu-iotests/026 | 5 ++++-
> tests/qemu-iotests/029 | 5 +++--
> tests/qemu-iotests/031 | 6 +++---
> tests/qemu-iotests/036 | 5 +++--
> tests/qemu-iotests/039 | 3 +++
> tests/qemu-iotests/046 | 2 ++
> tests/qemu-iotests/048 | 2 ++
> tests/qemu-iotests/051 | 5 +++--
> tests/qemu-iotests/058 | 5 +++--
> tests/qemu-iotests/060 | 6 ++++--
> tests/qemu-iotests/061 | 6 ++++--
> tests/qemu-iotests/062 | 2 +-
> tests/qemu-iotests/066 | 2 +-
> tests/qemu-iotests/067 | 6 ++++--
> tests/qemu-iotests/068 | 5 +++--
> tests/qemu-iotests/071 | 3 +++
> tests/qemu-iotests/073 | 2 ++
> tests/qemu-iotests/074 | 2 ++
> tests/qemu-iotests/080 | 5 +++--
> tests/qemu-iotests/090 | 2 ++
> tests/qemu-iotests/098 | 6 ++++--
> tests/qemu-iotests/099 | 3 ++-
> tests/qemu-iotests/103 | 5 +++--
> tests/qemu-iotests/108 | 6 ++++--
> tests/qemu-iotests/112 | 5 +++--
> tests/qemu-iotests/114 | 2 ++
> tests/qemu-iotests/121 | 3 +++
> tests/qemu-iotests/138 | 2 ++
> tests/qemu-iotests/156 | 2 ++
> tests/qemu-iotests/176 | 7 +++++--
> tests/qemu-iotests/191 | 2 ++
> tests/qemu-iotests/201 | 6 +++---
> tests/qemu-iotests/214 | 3 ++-
> tests/qemu-iotests/217 | 3 ++-
> tests/qemu-iotests/220 | 5 +++--
> tests/qemu-iotests/243 | 6 ++++--
> tests/qemu-iotests/244 | 5 +++--
> tests/qemu-iotests/250 | 2 ++
> tests/qemu-iotests/267 | 5 +++--
> 42 files changed, 117 insertions(+), 52 deletions(-)
>
> diff --git a/tests/qemu-iotests/007 b/tests/qemu-iotests/007
> index 7d3544b479..160683adf8 100755
> --- a/tests/qemu-iotests/007
> +++ b/tests/qemu-iotests/007
> @@ -41,8 +41,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto generic
> # refcount_bits must be at least 4 so we can create ten internal snapshots
> -# (1 bit supports none, 2 bits support two, 4 bits support 14)
> -_unsupported_imgopts 'refcount_bits=\(1\|2\)[^0-9]'
> +# (1 bit supports none, 2 bits support two, 4 bits support 14);
> +# snapshot are generally impossible with external data files
> +_unsupported_imgopts 'refcount_bits=\(1\|2\)[^0-9]' data_file
ACK
>
> echo
> echo "creating image"
> diff --git a/tests/qemu-iotests/014 b/tests/qemu-iotests/014
> index 2f728a1956..e1221c0fff 100755
> --- a/tests/qemu-iotests/014
> +++ b/tests/qemu-iotests/014
> @@ -43,6 +43,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> +# Compression and snapshots do not work with external data files
> +_unsupported_imgopts data_file
ACK
>
> TEST_OFFSETS="0 4294967296"
> TEST_OPS="writev read write readv"
> diff --git a/tests/qemu-iotests/015 b/tests/qemu-iotests/015
> index eec5387f3d..4d8effd0ae 100755
> --- a/tests/qemu-iotests/015
> +++ b/tests/qemu-iotests/015
> @@ -40,8 +40,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> # actually any format that supports snapshots
> _supported_fmt qcow2
> _supported_proto generic
> -# Internal snapshots are (currently) impossible with refcount_bits=1
> -_unsupported_imgopts 'refcount_bits=1[^0-9]'
> +# Internal snapshots are (currently) impossible with refcount_bits=1,
> +# and generally impossible with external data files
ACK
> +_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
>
> echo
> echo "creating image"
> diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026
> index 3430029ed6..a4aa74764f 100755
> --- a/tests/qemu-iotests/026
> +++ b/tests/qemu-iotests/026
> @@ -49,7 +49,10 @@ _supported_cache_modes writethrough none
> # 32 and 64 bits do not work either, however, due to different leaked cluster
> # count on error.
> # Thus, the only remaining option is refcount_bits=16.
> -_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
> +#
> +# As for data_file, none of the refcount tests can work for it.
ACK
> +_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' \
> + data_file
>
> echo "Errors while writing 128 kB"
> echo
> diff --git a/tests/qemu-iotests/029 b/tests/qemu-iotests/029
> index 9254ede5e5..2161a4b87a 100755
> --- a/tests/qemu-iotests/029
> +++ b/tests/qemu-iotests/029
> @@ -42,8 +42,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto generic
> _unsupported_proto vxhs
> -# Internal snapshots are (currently) impossible with refcount_bits=1
> -_unsupported_imgopts 'refcount_bits=1[^0-9]'
> +# Internal snapshots are (currently) impossible with refcount_bits=1,
> +# and generally impossible with external data files
ACK
> +_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
>
> offset_size=24
> offset_l1_size=36
> diff --git a/tests/qemu-iotests/031 b/tests/qemu-iotests/031
> index c44fcf91bb..646ecd593f 100755
> --- a/tests/qemu-iotests/031
> +++ b/tests/qemu-iotests/031
> @@ -40,9 +40,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> # This tests qcow2-specific low-level functionality
> _supported_fmt qcow2
> _supported_proto file
> -# We want to test compat=0.10, which does not support refcount widths
> -# other than 16
> -_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
> +# We want to test compat=0.10, which does not support external data
> +# files or refcount widths other than 16
> +_unsupported_imgopts data_file 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
This is maybe another reason to split this test for compat=0.10 and for
compat=1.1
But still can be done later of course.
>
> CLUSTER_SIZE=65536
>
> diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036
> index bbaf0ef45b..512598421c 100755
> --- a/tests/qemu-iotests/036
> +++ b/tests/qemu-iotests/036
> @@ -43,8 +43,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> # This tests qcow2-specific low-level functionality
> _supported_fmt qcow2
> _supported_proto file
> -# Only qcow2v3 and later supports feature bits
> -_unsupported_imgopts 'compat=0.10'
> +# Only qcow2v3 and later supports feature bits;
> +# qcow2.py does not support external data files
Minor nitpick, maybe tag this with TODO or so. No need to do now.
> +_unsupported_imgopts 'compat=0.10' data_file
>
> echo
> echo === Image with unknown incompatible feature bit ===
> diff --git a/tests/qemu-iotests/039 b/tests/qemu-iotests/039
> index 99563bf126..ddce48ab47 100755
> --- a/tests/qemu-iotests/039
> +++ b/tests/qemu-iotests/039
> @@ -44,6 +44,9 @@ _supported_proto file
> _supported_os Linux
> _default_cache_mode writethrough
> _supported_cache_modes writethrough
> +# Some of these test cases expect no external data file so that all
> +# clusters are part of the qcow2 image and refcounted
> +_unsupported_imgopts data_file
ACK
>
> size=128M
>
> diff --git a/tests/qemu-iotests/046 b/tests/qemu-iotests/046
> index 4e03ead7b1..a066eec605 100755
> --- a/tests/qemu-iotests/046
> +++ b/tests/qemu-iotests/046
> @@ -38,6 +38,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>
> _supported_fmt qcow2
> _supported_proto file
> +# data_file does not support compressed clusters
> +_unsupported_imgopts data_file
This is a very nice test, which doesn't seem to use compressed
clusters that much. I think it should be split as well.
No need to do this now of course, but maybe mark with TODO to
avoid loosing that info.
>
> CLUSTER_SIZE=64k
> size=128M
> diff --git a/tests/qemu-iotests/048 b/tests/qemu-iotests/048
> index a8feb76184..2af6b74b41 100755
> --- a/tests/qemu-iotests/048
> +++ b/tests/qemu-iotests/048
> @@ -49,6 +49,8 @@ _compare()
> _supported_fmt raw qcow2 qed luks
> _supported_proto file
> _supported_os Linux
> +# Using 'cp' is incompatible with external data files
> +_unsupported_imgopts data_file
You could compare the external files instead in theory *I think*.
Another item on some TODO list I guess.
>
> # Remove once all tests are fixed to use TEST_IMG_FILE
> # correctly and common.rc sets it unconditionally
> diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
> index 9cd1d60d45..0053bad46a 100755
> --- a/tests/qemu-iotests/051
> +++ b/tests/qemu-iotests/051
> @@ -39,8 +39,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto file
> # A compat=0.10 image is created in this test which does not support anything
> -# other than refcount_bits=16
Here also the compat=0.10 image is only a small part of the test,
although it seems to get used later in the rest of the test,
so the test I think should be split so that rest of the test could run in all
configurations.
> -_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
> +# other than refcount_bits=16;
> +# it also will not support an external data file
> +_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' data_file
>
> do_run_qemu()
> {
> diff --git a/tests/qemu-iotests/058 b/tests/qemu-iotests/058
> index ed01115fa3..d5304bb404 100755
> --- a/tests/qemu-iotests/058
> +++ b/tests/qemu-iotests/058
> @@ -56,8 +56,9 @@ _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> _require_command QEMU_NBD
> -# Internal snapshots are (currently) impossible with refcount_bits=1
> -_unsupported_imgopts 'refcount_bits=1[^0-9]'
> +# Internal snapshots are (currently) impossible with refcount_bits=1,
> +# and generally impossible with external data files
ACK
> +_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
>
> nbd_snapshot_img="nbd:unix:$nbd_unix_socket"
>
> diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
> index 92243c2edd..8ad0d7a904 100755
> --- a/tests/qemu-iotests/060
> +++ b/tests/qemu-iotests/060
> @@ -48,8 +48,10 @@ _filter_io_error()
> _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> -# These tests only work for compat=1.1 images with refcount_bits=16
> -_unsupported_imgopts 'compat=0.10' 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
> +# These tests only work for compat=1.1 images without an external
> +# data file with refcount_bits=16
Yea, with all hardcoded offsets, that isn't going to work.
ACK.
> +_unsupported_imgopts 'compat=0.10' data_file \
> + 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
>
> rt_offset=65536 # 0x10000 (XXX: just an assumption)
> rb_offset=131072 # 0x20000 (XXX: just an assumption)
> diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061
> index b4076d8e8b..4e218798d8 100755
> --- a/tests/qemu-iotests/061
> +++ b/tests/qemu-iotests/061
> @@ -42,8 +42,10 @@ _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> # Conversion between different compat versions can only really work
> -# with refcount_bits=16
> -_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
> +# with refcount_bits=16;
> +# we have explicit tests for data_file here, but the whole test does
> +# not work with it
> +_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' data_file
>
> echo
> echo "=== Testing version downgrade with zero expansion ==="
> diff --git a/tests/qemu-iotests/062 b/tests/qemu-iotests/062
> index ac0d2a9a3b..68e52a6402 100755
> --- a/tests/qemu-iotests/062
> +++ b/tests/qemu-iotests/062
> @@ -41,7 +41,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto generic
> # We need zero clusters and snapshots
> -_unsupported_imgopts 'compat=0.10' 'refcount_bits=1[^0-9]'
> +_unsupported_imgopts 'compat=0.10' 'refcount_bits=1[^0-9]' data_file
>
> IMG_SIZE=64M
>
Maybe split that test as well in the long run.
ACK.
> diff --git a/tests/qemu-iotests/066 b/tests/qemu-iotests/066
> index 00eb80d89e..0fff3e3a52 100755
> --- a/tests/qemu-iotests/066
> +++ b/tests/qemu-iotests/066
> @@ -40,7 +40,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto generic
> # We need zero clusters and snapshots
> -_unsupported_imgopts 'compat=0.10' 'refcount_bits=1[^0-9]'
> +_unsupported_imgopts 'compat=0.10' 'refcount_bits=1[^0-9]' data_file
Yet again, one small test case forcing the whole test to be skipped.
This should be split as well eventually.
>
> # Intentionally create an unaligned image
> IMG_SIZE=$((64 * 1024 * 1024 + 512))
> diff --git a/tests/qemu-iotests/067 b/tests/qemu-iotests/067
> index 926c79b37c..3bc6e719eb 100755
> --- a/tests/qemu-iotests/067
> +++ b/tests/qemu-iotests/067
> @@ -32,8 +32,10 @@ status=1 # failure is the default!
>
> _supported_fmt qcow2
> _supported_proto file
> -# Because anything other than 16 would change the output of query-block
> -_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
> +# Because anything other than 16 would change the output of query-block,
> +# and external data files would change the output of
> +# query-named-block-ndoes
> +_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' data_file
OK. There probably is a way to filter that, but I don't know if this is worth
it.
>
> do_run_qemu()
> {
> diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
> index 65650fca9a..c8748f5b02 100755
> --- a/tests/qemu-iotests/068
> +++ b/tests/qemu-iotests/068
> @@ -39,8 +39,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> # This tests qocw2-specific low-level functionality
> _supported_fmt qcow2
> _supported_proto generic
> -# Internal snapshots are (currently) impossible with refcount_bits=1
> -_unsupported_imgopts 'compat=0.10' 'refcount_bits=1[^0-9]'
> +# Internal snapshots are (currently) impossible with refcount_bits=1,
> +# and generally impossible with external data files
> +_unsupported_imgopts 'compat=0.10' 'refcount_bits=1[^0-9]' data_file
ACK
>
> IMG_SIZE=128K
>
> diff --git a/tests/qemu-iotests/071 b/tests/qemu-iotests/071
> index 4e31943244..88faebcc1d 100755
> --- a/tests/qemu-iotests/071
> +++ b/tests/qemu-iotests/071
> @@ -39,6 +39,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto file
> _require_drivers blkdebug blkverify
> +# blkdebug can only inject errors on bs->file, not on the data_file,
> +# so thie test does not work with external data files
s/thie/this
ACK. I guess that it would be too much trouble to extend it to
inject errors on the data file as well.
> +_unsupported_imgopts data_file
>
> do_run_qemu()
> {
> diff --git a/tests/qemu-iotests/073 b/tests/qemu-iotests/073
> index e684b1b780..903dc9c9ab 100755
> --- a/tests/qemu-iotests/073
> +++ b/tests/qemu-iotests/073
> @@ -39,6 +39,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto generic
> _unsupported_proto vxhs
> +# External data files do not support compressed clusters
> +_unsupported_imgopts data_file
This test should be split as well eventually.
>
> CLUSTER_SIZE=64k
> size=128M
> diff --git a/tests/qemu-iotests/074 b/tests/qemu-iotests/074
> index 62be89a0d9..db03edf0b0 100755
> --- a/tests/qemu-iotests/074
> +++ b/tests/qemu-iotests/074
> @@ -50,6 +50,8 @@ _compare()
> _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> +# blkdebug can only inject errors on bs->file
> +_unsupported_imgopts data_file
ACK, as above.
>
> # Setup test basic parameters
> TEST_IMG2=$TEST_IMG.2
> diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
> index b1ecafb41e..a3d13c414e 100755
> --- a/tests/qemu-iotests/080
> +++ b/tests/qemu-iotests/080
> @@ -40,9 +40,10 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> -# - Internal snapshots are (currently) impossible with refcount_bits=1
> +# - Internal snapshots are (currently) impossible with refcount_bits=1,
> +# and generally impossible with external data files
> # - This is generally a test for compat=1.1 images
> -_unsupported_imgopts 'refcount_bits=1[^0-9]' 'compat=0.10'
> +_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file 'compat=0.10'
I would more say that the test is too hardcoded for more that exact
settings it expects. It is all right in this case IMHO.
ACK.
>
> header_size=104
>
> diff --git a/tests/qemu-iotests/090 b/tests/qemu-iotests/090
> index 9f8cfbb80f..1246e4f910 100755
> --- a/tests/qemu-iotests/090
> +++ b/tests/qemu-iotests/090
> @@ -38,6 +38,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>
> _supported_fmt qcow2
> _supported_proto file nfs
> +# External data files do not support compressed clusters
> +_unsupported_imgopts data_file
ACK
>
> IMG_SIZE=128K
>
> diff --git a/tests/qemu-iotests/098 b/tests/qemu-iotests/098
> index 700068b328..1e29d96b3d 100755
> --- a/tests/qemu-iotests/098
> +++ b/tests/qemu-iotests/098
> @@ -40,8 +40,10 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>
> _supported_fmt qcow2
> _supported_proto file
> -# The code path we want to test here only works for compat=1.1 images
> -_unsupported_imgopts 'compat=0.10'
> +# The code path we want to test here only works for compat=1.1 images;
> +# blkdebug can only inject errors on bs->file, so external data files
> +# do not work with this test
> +_unsupported_imgopts 'compat=0.10' data_file
ACK, but this is already 3rd test we loose. Maybe add to a TODO to extend
blkdebug
to access data file as well.
>
> for event in l1_update empty_image_prepare reftable_update refblock_alloc; do
>
> diff --git a/tests/qemu-iotests/099 b/tests/qemu-iotests/099
> index b383c11e6a..65e8e92572 100755
> --- a/tests/qemu-iotests/099
> +++ b/tests/qemu-iotests/099
> @@ -46,8 +46,9 @@ _supported_fmt qcow qcow2 qed vdi vhdx vmdk vpc
> _supported_proto file
> _supported_os Linux
> _require_drivers blkdebug blkverify
> +# data_file would change the json:{} filenames
True but maybe still worth it to support the case?
> _unsupported_imgopts "subformat=monolithicFlat"
> "subformat=twoGbMaxExtentFlat" \
> - "subformat=twoGbMaxExtentSparse"
> + "subformat=twoGbMaxExtentSparse" data_file
>
> do_run_qemu()
> {
> diff --git a/tests/qemu-iotests/103 b/tests/qemu-iotests/103
> index 554b9de054..8c1ebe0443 100755
> --- a/tests/qemu-iotests/103
> +++ b/tests/qemu-iotests/103
> @@ -38,8 +38,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>
> _supported_fmt qcow2
> _supported_proto file nfs
> -# Internal snapshots are (currently) impossible with refcount_bits=1
> -_unsupported_imgopts 'refcount_bits=1[^0-9]'
> +# Internal snapshots are (currently) impossible with refcount_bits=1,
> +# and generally impossible with external data files
> +_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
ACK.
The test also only needs the snapshot in a part of it, so maybe split as well
later.
>
> IMG_SIZE=64K
>
> diff --git a/tests/qemu-iotests/108 b/tests/qemu-iotests/108
> index b0a6ae597b..6bbeb4f996 100755
> --- a/tests/qemu-iotests/108
> +++ b/tests/qemu-iotests/108
> @@ -41,8 +41,10 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> -# This test directly modifies a refblock so it relies on refcount_bits being
> 16
> -_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
> +# This test directly modifies a refblock so it relies on refcount_bits being
> 16;
> +# and the low-level modification it performs are not tuned for external data
> +# files
ACK
> +_unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)' data_file
>
> echo
> echo '=== Repairing an image without any refcount table ==='
> diff --git a/tests/qemu-iotests/112 b/tests/qemu-iotests/112
> index 6850225939..20ff5c224a 100755
> --- a/tests/qemu-iotests/112
> +++ b/tests/qemu-iotests/112
> @@ -40,8 +40,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto file
> # This test will set refcount_bits on its own which would conflict with the
> -# manual setting; compat will be overridden as well
> -_unsupported_imgopts refcount_bits 'compat=0.10'
> +# manual setting; compat will be overridden as well;
> +# and external data files do not work well with our refcount testing
ACK.
> +_unsupported_imgopts refcount_bits 'compat=0.10' data_file
>
> print_refcount_bits()
> {
> diff --git a/tests/qemu-iotests/114 b/tests/qemu-iotests/114
> index f90a744fc0..26104fff6c 100755
> --- a/tests/qemu-iotests/114
> +++ b/tests/qemu-iotests/114
> @@ -39,6 +39,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto generic
> _unsupported_proto vxhs
> +# qcow2.py does not work too well with external data files
ACK, but should be fixed later.
> +_unsupported_imgopts data_file
>
>
> TEST_IMG="$TEST_IMG.base" _make_test_img 64M
> diff --git a/tests/qemu-iotests/121 b/tests/qemu-iotests/121
> index 10db813d94..90ea0db737 100755
> --- a/tests/qemu-iotests/121
> +++ b/tests/qemu-iotests/121
> @@ -39,6 +39,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> +# Refcount structures are used much differently with external data
> +# files
ACK.
> +_unsupported_imgopts data_file
>
> echo
> echo '=== New refcount structures may not conflict with existing structures
> ==='
> diff --git a/tests/qemu-iotests/138 b/tests/qemu-iotests/138
> index 66ae9d5e78..7b0bc62a74 100755
> --- a/tests/qemu-iotests/138
> +++ b/tests/qemu-iotests/138
> @@ -40,6 +40,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> +# These refcount calculations do not work with external data files
> +_unsupported_imgopts data_file
Thats why I don't like the hardcoded tests that much.
ACK.
>
> echo
> echo '=== Check on an image with a multiple of 2^32 clusters ==='
> diff --git a/tests/qemu-iotests/156 b/tests/qemu-iotests/156
> index 3f27db71f2..5559df63a5 100755
> --- a/tests/qemu-iotests/156
> +++ b/tests/qemu-iotests/156
> @@ -51,6 +51,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2 qed
> _supported_proto generic
> _unsupported_proto vxhs
> +# Copying files around with cp does not work with external data files
> +_unsupported_imgopts data_file
Another place to fix later I guess.
ACK.
>
> # Create source disk
> TEST_IMG="$TEST_IMG.backing" _make_test_img 1M
> diff --git a/tests/qemu-iotests/176 b/tests/qemu-iotests/176
> index 50df4c00fa..117c8b6954 100755
> --- a/tests/qemu-iotests/176
> +++ b/tests/qemu-iotests/176
> @@ -47,8 +47,11 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> -# Persistent dirty bitmaps require compat=1.1
> -_unsupported_imgopts 'compat=0.10'
> +# Persistent dirty bitmaps require compat=1.1;
> +# Internal snapshots forbid using an external data file
> +# (they work with refcount_bits=1 here, though, because there actually
> +# is no data when creating the snapshot)
ACK
> +_unsupported_imgopts 'compat=0.10' data_file
>
> run_qemu()
> {
> diff --git a/tests/qemu-iotests/191 b/tests/qemu-iotests/191
> index 23ab0ce899..b05db68141 100755
> --- a/tests/qemu-iotests/191
> +++ b/tests/qemu-iotests/191
> @@ -43,6 +43,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>
> _supported_fmt qcow2
> _supported_proto file
> +# An external data file would change the query-named-block-nodes output
> +_unsupported_imgopts data_file
ACK.
>
> size=64M
>
> diff --git a/tests/qemu-iotests/201 b/tests/qemu-iotests/201
> index 7abf740fe4..3a458f18a0 100755
> --- a/tests/qemu-iotests/201
> +++ b/tests/qemu-iotests/201
> @@ -43,9 +43,9 @@ _supported_fmt qcow2
> _supported_proto generic
> _supported_os Linux
>
> -# Internal snapshots are (currently) impossible with refcount_bits=1
> -# This was taken from test 080
> -_unsupported_imgopts 'refcount_bits=1[^0-9]'
> +# Internal snapshots are (currently) impossible with refcount_bits=1,
> +# and generally impossible with external data files
> +_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
ACK
>
> size=64M
> _make_test_img $size
> diff --git a/tests/qemu-iotests/214 b/tests/qemu-iotests/214
> index 21ec8a2ad8..0f2e61280a 100755
> --- a/tests/qemu-iotests/214
> +++ b/tests/qemu-iotests/214
> @@ -39,7 +39,8 @@ _supported_proto file
>
> # Repairing the corrupted image requires qemu-img check to store a
> # refcount up to 3, which requires at least two refcount bits.
> -_unsupported_imgopts 'refcount_bits=1[^0-9]'
> +# External data files do not support compressed clusters.
> +_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
ACK
>
>
> echo
> diff --git a/tests/qemu-iotests/217 b/tests/qemu-iotests/217
> index 58a78a6098..d89116ccad 100755
> --- a/tests/qemu-iotests/217
> +++ b/tests/qemu-iotests/217
> @@ -40,7 +40,8 @@ _supported_proto file
>
> # This test needs clusters with at least a refcount of 2 so that
> # OFLAG_COPIED is not set. refcount_bits=1 is therefore unsupported.
> -_unsupported_imgopts 'refcount_bits=1[^0-9]'
> +# (As are external data files.)
> +_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
>
> echo
> echo '=== Simulating an I/O error during snapshot deletion ==='
ACK
> diff --git a/tests/qemu-iotests/220 b/tests/qemu-iotests/220
> index 3769f372cb..a88c59152b 100755
> --- a/tests/qemu-iotests/220
> +++ b/tests/qemu-iotests/220
> @@ -37,8 +37,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> -# To use a different refcount width but 16 bits we need compat=1.1
> -_unsupported_imgopts 'compat=0.10'
> +# To use a different refcount width but 16 bits we need compat=1.1,
> +# and external data files do not support compressed clusters.
> +_unsupported_imgopts 'compat=0.10' data_file
>
> echo "== Creating huge file =="
>
ACK.
> diff --git a/tests/qemu-iotests/243 b/tests/qemu-iotests/243
> index 3dc3b6a711..a61852f6d9 100755
> --- a/tests/qemu-iotests/243
> +++ b/tests/qemu-iotests/243
> @@ -40,8 +40,10 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> -# External data files do not work with compat=0.10
> -_unsupported_imgopts 'compat=0.10'
> +# External data files do not work with compat=0.10, and because there
> +# is an explicit case for external data files here, we cannot allow
> +# the user to specify whether to use one
> +_unsupported_imgopts 'compat=0.10' data_file
>
> for mode in off metadata falloc full; do
>
> diff --git a/tests/qemu-iotests/244 b/tests/qemu-iotests/244
> index 13263292b0..0d1efee6ef 100755
> --- a/tests/qemu-iotests/244
> +++ b/tests/qemu-iotests/244
> @@ -41,8 +41,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> -# External data files do not work with compat=0.10
> -_unsupported_imgopts 'compat=0.10'
> +# External data files do not work with compat=0.10, and because we use
> +# our own external data file, we cannot let the user specify one
> +_unsupported_imgopts 'compat=0.10' data_file
>
> echo
> echo "=== Create and open image with external data file ==="
> diff --git a/tests/qemu-iotests/250 b/tests/qemu-iotests/250
> index 670cf19076..9bb6b94d74 100755
> --- a/tests/qemu-iotests/250
> +++ b/tests/qemu-iotests/250
> @@ -39,6 +39,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
> +# This test does not make much sense with external data files
> +_unsupported_imgopts data_file
>
> # This test checks that qcow2_process_discards does not truncate a discard
> # request > 2G.
> diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
> index eda45449d4..d2f0e5df59 100755
> --- a/tests/qemu-iotests/267
> +++ b/tests/qemu-iotests/267
> @@ -41,8 +41,9 @@ _supported_fmt qcow2
> _supported_proto file
> _supported_os Linux
>
> -# Internal snapshots are (currently) impossible with refcount_bits=1
> -_unsupported_imgopts 'refcount_bits=1[^0-9]'
> +# Internal snapshots are (currently) impossible with refcount_bits=1,
> +# and generally impossible with external data files
> +_unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
>
> do_run_qemu()
> {
Overall I think that this patch reveals that we tend to have tests that have
too many subtests
which force us to restrict the test to only the common configuration
denominator of the
subtests, so it would be a good idea to go over these tests,
especially these that are blacklisted by this patch and try to split them.
Also there are too many tests that hardcode location of various metadata
structures in the qcow2 file,
which make both the debug and addition of new features harder that it should be.
These tests should instead be able to query the qcow2 file for the actual
metadata location
and cope with that. Having said that, its easier to say that than to do that,
especially for tests
that try to do various corruptions, unusual metadata object sizes, etc. So I
just want to note this.
Also there are various things like qcow2.py not working with external files
that should be eventually
be fixed.
Overall I think that this patch is 100% good enough for now, so thanks a lot,
and
Reviewed-by: Maxim Levitsky <address@hidden>
Best regards,
Maxim Levitsky
- Re: [PATCH v2 20/21] iotests: Disable data_file where it cannot be used,
Maxim Levitsky <=