| Message ID | 20191015142729.18123-6-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iotests: Allow ./check -o data_file | expand |
On Tue, 2019-10-15 at 16:27 +0200, Max Reitz wrote: > Some tests require compat=1.1 and thus set IMGOPTS='compat=1.1' > globally. That is not how it should be done; instead, they should > simply set _unsupported_imgopts to compat=0.10 (compat=1.1 is the > default anyway). > > This makes the tests heed user-specified $IMGOPTS. Some do not work > with all image options, though, so we need to disable them accordingly. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > tests/qemu-iotests/036 | 3 +-- > tests/qemu-iotests/060 | 4 ++-- > tests/qemu-iotests/062 | 3 ++- > tests/qemu-iotests/066 | 3 ++- > tests/qemu-iotests/068 | 3 ++- > tests/qemu-iotests/098 | 4 ++-- > 6 files changed, 11 insertions(+), 9 deletions(-) > > diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036 > index 5f929ad3be..bbaf0ef45b 100755 > --- a/tests/qemu-iotests/036 > +++ b/tests/qemu-iotests/036 > @@ -43,9 +43,8 @@ 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 > -IMGOPTS="compat=1.1" > +_unsupported_imgopts 'compat=0.10' > > echo > echo === Image with unknown incompatible feature bit === > diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060 > index b91d8321bb..9c2ef42522 100755 > --- a/tests/qemu-iotests/060 > +++ b/tests/qemu-iotests/060 > @@ -48,6 +48,8 @@ _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]\|$\)\)' Looks like the reason for that is that the test hardcodes (or guesses that is) various qcow2 structures thing I have seen few times already in the iotests. Not now but sometime in the future it would be nice to extend qcow2.py (or something like that) to dump location of all qcow2 structures so that the guesswork could be eliminated. > > rt_offset=65536 # 0x10000 (XXX: just an assumption) > rb_offset=131072 # 0x20000 (XXX: just an assumption) > @@ -55,8 +57,6 @@ l1_offset=196608 # 0x30000 (XXX: just an assumption) > l2_offset=262144 # 0x40000 (XXX: just an assumption) > l2_offset_after_snapshot=524288 # 0x80000 (XXX: just an assumption) > > -IMGOPTS="compat=1.1" > - > OPEN_RW="open -o overlap-check=all $TEST_IMG" > # Overlap checks are done before write operations only, therefore opening an > # image read-only makes the overlap-check option irrelevant > diff --git a/tests/qemu-iotests/062 b/tests/qemu-iotests/062 > index d5f818fcce..ac0d2a9a3b 100755 > --- a/tests/qemu-iotests/062 > +++ b/tests/qemu-iotests/062 > @@ -40,8 +40,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > # This tests qocw2-specific low-level functionality If you respin that patch for some reason, you could fix the typo above. > _supported_fmt qcow2 > _supported_proto generic > +# We need zero clusters and snapshots > +_unsupported_imgopts 'compat=0.10' 'refcount_bits=1[^0-9]' > > -IMGOPTS="compat=1.1" > IMG_SIZE=64M > > echo > diff --git a/tests/qemu-iotests/066 b/tests/qemu-iotests/066 > index 28f8c98412..00eb80d89e 100755 > --- a/tests/qemu-iotests/066 > +++ b/tests/qemu-iotests/066 > @@ -39,9 +39,10 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > # This tests qocw2-specific low-level functionality ^^ Same here > _supported_fmt qcow2 > _supported_proto generic > +# We need zero clusters and snapshots > +_unsupported_imgopts 'compat=0.10' 'refcount_bits=1[^0-9]' > > # Intentionally create an unaligned image > -IMGOPTS="compat=1.1" > IMG_SIZE=$((64 * 1024 * 1024 + 512)) > > echo > diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068 > index 22f5ca3ba6..65650fca9a 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]' > > -IMGOPTS="compat=1.1" > IMG_SIZE=128K > > case "$QEMU_DEFAULT_MACHINE" in > diff --git a/tests/qemu-iotests/098 b/tests/qemu-iotests/098 > index 1c1d1c468f..700068b328 100755 > --- a/tests/qemu-iotests/098 > +++ b/tests/qemu-iotests/098 > @@ -40,8 +40,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 > > _supported_fmt qcow2 > _supported_proto file > - > -IMGOPTS="compat=1.1" > +# The code path we want to test here only works for compat=1.1 images > +_unsupported_imgopts 'compat=0.10' > > for event in l1_update empty_image_prepare reftable_update refblock_alloc; do > Reviewed-by: Maxim Levitsky <mlevitsky@redhat.com> Best regards, Maxim Levitsky
On 06.11.19 16:45, Maxim Levitsky wrote: > On Tue, 2019-10-15 at 16:27 +0200, Max Reitz wrote: >> Some tests require compat=1.1 and thus set IMGOPTS='compat=1.1' >> globally. That is not how it should be done; instead, they should >> simply set _unsupported_imgopts to compat=0.10 (compat=1.1 is the >> default anyway). >> >> This makes the tests heed user-specified $IMGOPTS. Some do not work >> with all image options, though, so we need to disable them accordingly. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> tests/qemu-iotests/036 | 3 +-- >> tests/qemu-iotests/060 | 4 ++-- >> tests/qemu-iotests/062 | 3 ++- >> tests/qemu-iotests/066 | 3 ++- >> tests/qemu-iotests/068 | 3 ++- >> tests/qemu-iotests/098 | 4 ++-- >> 6 files changed, 11 insertions(+), 9 deletions(-) >> >> diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036 >> index 5f929ad3be..bbaf0ef45b 100755 >> --- a/tests/qemu-iotests/036 >> +++ b/tests/qemu-iotests/036 >> @@ -43,9 +43,8 @@ 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 >> -IMGOPTS="compat=1.1" >> +_unsupported_imgopts 'compat=0.10' >> >> echo >> echo === Image with unknown incompatible feature bit === >> diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060 >> index b91d8321bb..9c2ef42522 100755 >> --- a/tests/qemu-iotests/060 >> +++ b/tests/qemu-iotests/060 >> @@ -48,6 +48,8 @@ _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]\|$\)\)' > Looks like the reason for that is that the test hardcodes (or guesses that is) various qcow2 structures > thing I have seen few times already in the iotests. > Not now but sometime in the future it would be nice to extend qcow2.py (or something > like that) to dump location of all qcow2 structures so that the guesswork could be eliminated. With the peek_file* functions we have now it’s actually simple to dump that location ($(peek_file_be "$TEST_IMG" 48 8) for the refcount table offset, for example). But it wouldn’t help, because compat=0.10 or refcount_bits != 16 won’t change those locations. So the locations aren’t the reason why we need to forbid those options here. The reason we need refcount_bits=16 is that we’re going to directly manipulate a refcount block. To do so, we need to know the refcount width, and I don’t think it’s worth trying to implement something generic. We need compat=1.1 because compat=0.10 doesn’t have feature bits, so there’s no “corrupt” bit there. Max
On Thu, 2019-11-07 at 10:08 +0100, Max Reitz wrote: > On 06.11.19 16:45, Maxim Levitsky wrote: > > On Tue, 2019-10-15 at 16:27 +0200, Max Reitz wrote: > > > Some tests require compat=1.1 and thus set IMGOPTS='compat=1.1' > > > globally. That is not how it should be done; instead, they should > > > simply set _unsupported_imgopts to compat=0.10 (compat=1.1 is the > > > default anyway). > > > > > > This makes the tests heed user-specified $IMGOPTS. Some do not work > > > with all image options, though, so we need to disable them accordingly. > > > > > > Signed-off-by: Max Reitz <mreitz@redhat.com> > > > --- > > > tests/qemu-iotests/036 | 3 +-- > > > tests/qemu-iotests/060 | 4 ++-- > > > tests/qemu-iotests/062 | 3 ++- > > > tests/qemu-iotests/066 | 3 ++- > > > tests/qemu-iotests/068 | 3 ++- > > > tests/qemu-iotests/098 | 4 ++-- > > > 6 files changed, 11 insertions(+), 9 deletions(-) > > > > > > diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036 > > > index 5f929ad3be..bbaf0ef45b 100755 > > > --- a/tests/qemu-iotests/036 > > > +++ b/tests/qemu-iotests/036 > > > @@ -43,9 +43,8 @@ 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 > > > -IMGOPTS="compat=1.1" > > > +_unsupported_imgopts 'compat=0.10' > > > > > > echo > > > echo === Image with unknown incompatible feature bit === > > > diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060 > > > index b91d8321bb..9c2ef42522 100755 > > > --- a/tests/qemu-iotests/060 > > > +++ b/tests/qemu-iotests/060 > > > @@ -48,6 +48,8 @@ _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]\|$\)\)' > > > > Looks like the reason for that is that the test hardcodes (or guesses that is) various qcow2 structures > > thing I have seen few times already in the iotests. > > Not now but sometime in the future it would be nice to extend qcow2.py (or something > > like that) to dump location of all qcow2 structures so that the guesswork could be eliminated. > > With the peek_file* functions we have now it’s actually simple to dump > that location ($(peek_file_be "$TEST_IMG" 48 8) for the refcount table > offset, for example). > > But it wouldn’t help, because compat=0.10 or refcount_bits != 16 won’t > change those locations. So the locations aren’t the reason why we need > to forbid those options here. > > The reason we need refcount_bits=16 is that we’re going to directly > manipulate a refcount block. To do so, we need to know the refcount > width, and I don’t think it’s worth trying to implement something generic. > > We need compat=1.1 because compat=0.10 doesn’t have feature bits, so > there’s no “corrupt” bit there. > > Max > This makes sense! Sorry for the noise! Best regards, Maxim Levitsky
diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036 index 5f929ad3be..bbaf0ef45b 100755 --- a/tests/qemu-iotests/036 +++ b/tests/qemu-iotests/036 @@ -43,9 +43,8 @@ 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 -IMGOPTS="compat=1.1" +_unsupported_imgopts 'compat=0.10' echo echo === Image with unknown incompatible feature bit === diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060 index b91d8321bb..9c2ef42522 100755 --- a/tests/qemu-iotests/060 +++ b/tests/qemu-iotests/060 @@ -48,6 +48,8 @@ _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]\|$\)\)' rt_offset=65536 # 0x10000 (XXX: just an assumption) rb_offset=131072 # 0x20000 (XXX: just an assumption) @@ -55,8 +57,6 @@ l1_offset=196608 # 0x30000 (XXX: just an assumption) l2_offset=262144 # 0x40000 (XXX: just an assumption) l2_offset_after_snapshot=524288 # 0x80000 (XXX: just an assumption) -IMGOPTS="compat=1.1" - OPEN_RW="open -o overlap-check=all $TEST_IMG" # Overlap checks are done before write operations only, therefore opening an # image read-only makes the overlap-check option irrelevant diff --git a/tests/qemu-iotests/062 b/tests/qemu-iotests/062 index d5f818fcce..ac0d2a9a3b 100755 --- a/tests/qemu-iotests/062 +++ b/tests/qemu-iotests/062 @@ -40,8 +40,9 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 # This tests qocw2-specific low-level functionality _supported_fmt qcow2 _supported_proto generic +# We need zero clusters and snapshots +_unsupported_imgopts 'compat=0.10' 'refcount_bits=1[^0-9]' -IMGOPTS="compat=1.1" IMG_SIZE=64M echo diff --git a/tests/qemu-iotests/066 b/tests/qemu-iotests/066 index 28f8c98412..00eb80d89e 100755 --- a/tests/qemu-iotests/066 +++ b/tests/qemu-iotests/066 @@ -39,9 +39,10 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 # This tests qocw2-specific low-level functionality _supported_fmt qcow2 _supported_proto generic +# We need zero clusters and snapshots +_unsupported_imgopts 'compat=0.10' 'refcount_bits=1[^0-9]' # Intentionally create an unaligned image -IMGOPTS="compat=1.1" IMG_SIZE=$((64 * 1024 * 1024 + 512)) echo diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068 index 22f5ca3ba6..65650fca9a 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]' -IMGOPTS="compat=1.1" IMG_SIZE=128K case "$QEMU_DEFAULT_MACHINE" in diff --git a/tests/qemu-iotests/098 b/tests/qemu-iotests/098 index 1c1d1c468f..700068b328 100755 --- a/tests/qemu-iotests/098 +++ b/tests/qemu-iotests/098 @@ -40,8 +40,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 _supported_fmt qcow2 _supported_proto file - -IMGOPTS="compat=1.1" +# The code path we want to test here only works for compat=1.1 images +_unsupported_imgopts 'compat=0.10' for event in l1_update empty_image_prepare reftable_update refblock_alloc; do
Some tests require compat=1.1 and thus set IMGOPTS='compat=1.1' globally. That is not how it should be done; instead, they should simply set _unsupported_imgopts to compat=0.10 (compat=1.1 is the default anyway). This makes the tests heed user-specified $IMGOPTS. Some do not work with all image options, though, so we need to disable them accordingly. Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/036 | 3 +-- tests/qemu-iotests/060 | 4 ++-- tests/qemu-iotests/062 | 3 ++- tests/qemu-iotests/066 | 3 ++- tests/qemu-iotests/068 | 3 ++- tests/qemu-iotests/098 | 4 ++-- 6 files changed, 11 insertions(+), 9 deletions(-)