Re: [Qemu-devel] [PATCHv2 04/18] qemu-iotests: fix test 013 to work with any protocol

[Fam Zheng]

On 2014年01月06日 14:48, Peter Lieven wrote:
> On 06.01.2014 06:31, Fam Zheng wrote:
> > On 2014年01月06日 01:21, Peter Lieven wrote:
> > > Signed-off-by: Peter Lieven <address@hidden>
> > > ---
> > >   tests/qemu-iotests/013     |    9 ++++-----
> > >   tests/qemu-iotests/013.out |    2 +-
> > >   2 files changed, 5 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/tests/qemu-iotests/013 b/tests/qemu-iotests/013
> > > index ea3cab9..0dbc934 100755
> > > --- a/tests/qemu-iotests/013
> > > +++ b/tests/qemu-iotests/013
> > > @@ -41,14 +41,14 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
> > >
> > >   # much of this could be generic for any format supporting compression.
> > >   _supported_fmt qcow qcow2
> > > -_supported_proto file
> > > +_supported_proto generic
> > >   _supported_os Linux
> > >
> > >   TEST_OFFSETS="0 4294967296"
> > >   TEST_OPS="writev read write readv"
> > >   CLUSTER_SIZE=4096
> >
> > I think dropping these three TEST_IMG overriding change...
> >
> > > -_make_test_img 6G
> > > +TEST_IMG=$TEST_IMG.orig _make_test_img 6G
> >
> > #1
> >
> > >   echo "Testing empty image"
> > >   echo
> > > @@ -56,16 +56,15 @@ echo
> > >   for offset in $TEST_OFFSETS; do
> > >       echo "At offset $offset:"
> > >       for op in $TEST_OPS; do
> > > -        io_test $op $offset $CLUSTER_SIZE 8
> > > +        TEST_IMG=$TEST_IMG.orig io_test $op $offset $CLUSTER_SIZE 8
> >
> > #2
> >
> > >       done
> > > -    _check_test_img
> > > +    TEST_IMG=$TEST_IMG.orig _check_test_img
> >
> > #3
> >
> > >   done
> > >
> > >
> > >   echo "Compressing image"
> > >   echo
> > >
> > > -mv "$TEST_IMG" "$TEST_IMG.orig"
> >
> > and changing this to
> >
> > TEST_IMG=$TEST_IMG.orig _make_test_img 6G
> >
> > Should work.
> Unfortunately it doesn't. All subsequent commands will then work
> on $TEST_IMG.orig altough they shouldn't. In case of
> 013 this is io_test, _check_test_img and the cleanup at the end.

Why? The overriding is temporary and subsequent commands are not affected.

My proposal above doesn't work, though, because an empty new image 
doesn't contain the right data, what is needed here is copy. So maybe 
change the "mv" line to:

$QEMU_IMG convert -f $IMGFMT -O $IMGFMT "$TEST_IMG" "$TEST_IMG.orig"

could do the work, but I'm not sure if this fits every case.

> There are 3 options:
>   - override it in every line that should use an alternate $TEST_IMG
>   - save the original $TEST_IMG and restore it.
>   - rework all commands to take the file as parameter and not use
>     a global variable for it.
>
> I choosed the first one because it makes clear which $TEST_IMG is acutally
> used. You see from the output and the code that you are dealing with the
> file that is later used as $TEST_IMG.orig. If you see $TEST_IMG there you
> can't distinguish if its the backing or original file or the actual image.
>
> But I thought that this would be controversal. This is I why I splitted
> the patch
> into individual ones. So its possible to drop all these patches and
> still be able
> to proceed with the integration of the NFS protocol driver.

I'll leave maintainers to decide.

Fam