[v2,05/21] iotests: Replace IMGOPTS by _unsupported_imgopts
diff mbox series

Message ID 20191015142729.18123-6-mreitz@redhat.com
State New
Headers show
Series
  • iotests: Allow ./check -o data_file
Related show

Commit Message

Max Reitz Oct. 15, 2019, 2:27 p.m. UTC
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(-)

Comments

Maxim Levitsky Nov. 6, 2019, 3:45 p.m. UTC | #1
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
Max Reitz Nov. 7, 2019, 9:08 a.m. UTC | #2
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
Maxim Levitsky Nov. 7, 2019, 9:56 a.m. UTC | #3
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

Patch
diff mbox series

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