Re: [PATCH v2 05/21] iotests: Replace IMGOPTS by _unsupported_imgopts

[Maxim Levitsky <mlevitsk@redhat.com>]

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