Re: [Qemu-devel] [PATCHv2 05/18] qemu-iotests: fix tests 014 and 023 to work with any protocol

[Peter Lieven <pl@kamp.de>]

Am 10.01.2014 20:14, schrieb Kevin Wolf:
> Am 10.01.2014 um 20:06 hat Peter Lieven geschrieben:
>> Am 10.01.2014 20:04, schrieb Kevin Wolf:
>>> Am 06.01.2014 um 07:49 hat Peter Lieven geschrieben:
>>>> On 06.01.2014 06:40, Fam Zheng wrote:
>>>>> On 2014年01月06日 01:21, Peter Lieven wrote:
>>>>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>>>>>> ---
>>>>>>  tests/qemu-iotests/014            |    4 ++--
>>>>>>  tests/qemu-iotests/014.out        |    2 +-
>>>>>>  tests/qemu-iotests/023            |   11 +++++------
>>>>>>  tests/qemu-iotests/023.out        |   16 ++++++++--------
>>>>>>  tests/qemu-iotests/common.pattern |    7 +++----
>>>>>>  5 files changed, 19 insertions(+), 21 deletions(-)
>>>>>>
>>>>>> diff --git a/tests/qemu-iotests/014 b/tests/qemu-iotests/014
>>>>>> index b23c2db..01fb614 100755
>>>>>> --- a/tests/qemu-iotests/014
>>>>>> +++ b/tests/qemu-iotests/014
>>>>>> @@ -43,14 +43,14 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>>>>>>
>>>>>>  # much of this could be generic for any format supporting snapshots
>>>>>>  _supported_fmt qcow2
>>>>>> -_supported_proto file
>>>>>> +_supported_proto generic
>>>>>>  _supported_os Linux
>>>>>>
>>>>>>  TEST_OFFSETS="0 4294967296"
>>>>>>  TEST_OPS="writev read write readv"
>>>>>>  CLUSTER_SIZE=4096
>>>>>>
>>>>>> -_make_test_img 6G
>>>>>> +TEST_IMG=$TEST_IMG.orig _make_test_img 6G
>>>>>>
>>>>>>  echo "Testing empty image:"
>>>>>>  for offset in $TEST_OFFSETS; do
>>>>>> diff --git a/tests/qemu-iotests/014.out b/tests/qemu-iotests/014.out
>>>>>> index 4744b4b..6459af0 100644
>>>>>> --- a/tests/qemu-iotests/014.out
>>>>>> +++ b/tests/qemu-iotests/014.out
>>>>>> @@ -1,5 +1,5 @@
>>>>>>  QA output created by 014
>>>>>> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=6442450944
>>>>>> +Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=6442450944
>>>>>>  Testing empty image:
>>>>>>  test2: With offset 0
>>>>>>  === Clusters to be compressed [1]
>>>>>> diff --git a/tests/qemu-iotests/023 b/tests/qemu-iotests/023
>>>>>> index 9ad06b9..2357696 100755
>>>>>> --- a/tests/qemu-iotests/023
>>>>>> +++ b/tests/qemu-iotests/023
>>>>>> @@ -41,7 +41,7 @@ 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"
>>>>>> @@ -55,7 +55,7 @@ for CLUSTER_SIZE in $CLUSTER_SIZES; do
>>>>>>      echo "Creating new image; cluster size: $CLUSTER_SIZE"
>>>>>>      echo
>>>>>>
>>>>>> -    _make_test_img 8G
>>>>>> +    TEST_IMG=$TEST_IMG.orig _make_test_img 8G
>>>>>>
>>>>>>      echo "Testing empty image"
>>>>>>      echo
>>>>>> @@ -63,15 +63,14 @@ for CLUSTER_SIZE in $CLUSTER_SIZES; do
>>>>>>      for offset in $TEST_OFFSETS; do
>>>>>>          echo "At offset $offset:"
>>>>>>          for op in $TEST_OPS; do
>>>>>> -            io_test $op $offset $CLUSTER_SIZE 3
>>>>>> +            TEST_IMG=$TEST_IMG.orig io_test $op $offset $CLUSTER_SIZE 3
>>>>>>          done
>>>>>> -        _check_test_img
>>>>>> +        TEST_IMG=$TEST_IMG.orig _check_test_img
>>>>>>      done
>>>>>>
>>>>>>      echo "Compressing image"
>>>>>>      echo
>>>>>>
>>>>>> -    mv "$TEST_IMG" "$TEST_IMG.orig"
>>>>>>      $QEMU_IMG convert -f $IMGFMT -O $IMGFMT -c "$TEST_IMG.orig" "$TEST_IMG"
>>>>>>
>>>>>>      echo "Testing compressed image"
>>>>>> @@ -101,7 +100,7 @@ for CLUSTER_SIZE in $CLUSTER_SIZES; do
>>>>>>      echo "Creating another new image"
>>>>>>      echo
>>>>>>
>>>>>> -    _make_test_img 8G
>>>>>> +    TEST_IMG=$TEST_IMG.orig _make_test_img 8G
>>>>>>
>>>>>>      echo "More complex patterns"
>>>>>>      echo
>>>>>> diff --git a/tests/qemu-iotests/023.out b/tests/qemu-iotests/023.out
>>>>>> index ec32341..b80836d 100644
>>>>>> --- a/tests/qemu-iotests/023.out
>>>>>> +++ b/tests/qemu-iotests/023.out
>>>>>> @@ -1,7 +1,7 @@
>>>>>>  QA output created by 023
>>>>>>  Creating new image; cluster size: 1024
>>>>>>
>>>>>> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=8589934592
>>>>>> +Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=8589934592
>>>>>>  Testing empty image
>>>>>>
>>>>>>  At offset 0:
>>>>>> @@ -5664,7 +5664,7 @@ read 3072/3072 bytes at offset 4295491072
>>>>>>  No errors were found on the image.
>>>>>>  Creating another new image
>>>>>>
>>>>>> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=8589934592
>>>>>> +Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=8589934592
>>>>>>  More complex patterns
>>>>>>
>>>>>>  test2: With offset 0
>>>>>> @@ -5887,7 +5887,7 @@ read 2048/2048 bytes at offset 4295001088
>>>>>>  No errors were found on the image.
>>>>>>  Creating new image; cluster size: 4096
>>>>>>
>>>>>> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=8589934592
>>>>>> +Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=8589934592
>>>>>>  Testing empty image
>>>>>>
>>>>>>  At offset 0:
>>>>>> @@ -12270,7 +12270,7 @@ read 12288/12288 bytes at offset 4301256704
>>>>>>  No errors were found on the image.
>>>>>>  Creating another new image
>>>>>>
>>>>>> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=8589934592
>>>>>> +Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=8589934592
>>>>>>  More complex patterns
>>>>>>
>>>>>>  test2: With offset 0
>>>>>> @@ -12493,7 +12493,7 @@ read 8192/8192 bytes at offset 4295102464
>>>>>>  No errors were found on the image.
>>>>>>  Creating new image; cluster size: 16384
>>>>>>
>>>>>> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=8589934592
>>>>>> +Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=8589934592
>>>>>>  Testing empty image
>>>>>>
>>>>>>  At offset 0:
>>>>>> @@ -18876,7 +18876,7 @@ read 49152/49152 bytes at offset 4395622400
>>>>>>  No errors were found on the image.
>>>>>>  Creating another new image
>>>>>>
>>>>>> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=8589934592
>>>>>> +Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=8589934592
>>>>>>  More complex patterns
>>>>>>
>>>>>>  test2: With offset 0
>>>>>> @@ -19099,7 +19099,7 @@ read 32768/32768 bytes at offset 4295507968
>>>>>>  No errors were found on the image.
>>>>>>  Creating new image; cluster size: 65536
>>>>>>
>>>>>> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=8589934592
>>>>>> +Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=8589934592
>>>>>>  Testing empty image
>>>>>>
>>>>>>  At offset 0:
>>>>>> @@ -25482,7 +25482,7 @@ read 196608/196608 bytes at offset 5905547264
>>>>>>  No errors were found on the image.
>>>>>>  Creating another new image
>>>>>>
>>>>>> -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=8589934592
>>>>>> +Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=8589934592
>>>>>>  More complex patterns
>>>>>>
>>>>> The same comment for this as for [04/18], there are two many overrides which are not necessary because you can replace "mv $TEST_IMG $TEST_IMG.orig" with "TEST_IMG=$TEST_IMG.orig _make_test_img 8G" without touching other parts.
>>>>>
>>>>>>  test2: With offset 0
>>>>>> diff --git a/tests/qemu-iotests/common.pattern b/tests/qemu-iotests/common.pattern
>>>>>> index ddfbca1..d6ed5c1 100644
>>>>>> --- a/tests/qemu-iotests/common.pattern
>>>>>> +++ b/tests/qemu-iotests/common.pattern
>>>>>> @@ -111,13 +111,12 @@ function io_test2() {
>>>>>>
>>>>>>      # Write the clusters to be compressed
>>>>>>      echo === Clusters to be compressed [1]
>>>>>> -    io_pattern writev $((offset + 4 * $cluster_size)) $cluster_size $((9 * $cluster_size)) $num 165
>>>>>> +    TEST_IMG=$TEST_IMG.orig io_pattern writev $((offset + 4 * $cluster_size)) $cluster_size $((9 * $cluster_size)) $num 165
>>>>>>      echo === Clusters to be compressed [2]
>>>>>> -    io_pattern writev $((offset + 5 * $cluster_size)) $cluster_size $((9 * $cluster_size)) $num 165
>>>>>> +    TEST_IMG=$TEST_IMG.orig io_pattern writev $((offset + 5 * $cluster_size)) $cluster_size $((9 * $cluster_size)) $num 165
>>>>>>      echo === Clusters to be compressed [3]
>>>>>> -    io_pattern writev $((offset + 8 * $cluster_size)) $cluster_size $((9 * $cluster_size)) $num 165
>>>>>> +    TEST_IMG=$TEST_IMG.orig io_pattern writev $((offset + 8 * $cluster_size)) $cluster_size $((9 * $cluster_size)) $num 165
>>>>>>
>>>>>> -    mv "$TEST_IMG" "$TEST_IMG.orig"
>>>>>>      $QEMU_IMG convert -f $IMGFMT -O $IMGFMT -c "$TEST_IMG.orig" "$TEST_IMG"
>>>>>>
>>>>>>      # Write the used clusters
>>>>>>
>>>>> Especially here, overriding TEST_IMG inside a utility function is not right.
>>>> The problem is, that in its original form io_test2 only works with protocol file. Maybe its better to split io_test2 into
>>>> 2 parts.
>>> Perhaps you should leave $TEST_IMG as it is and instead of using mv to
>>> move the old version away create a new one with a different name? I
>>> guess io_pattern would need to get a filename then, but that shouldn't
>>> be hard.
>> The problem is not only io_pattern, also all the other commands. Or are you just referring to the changes in io_test2
>> and are fine with the overrides outside a function?
> I haven't looked at the full series yet, but yes, probably all the other
> functions in common.pattern as well.
>
> In general, I think the primary test image should be $TEST_IMG, not
> $TEST_IMG.orig. Hiding the creation of a copy in common.pattern, after
> which $TEST_IMG becomes valid, doesn't make it easier to understand
> either.
Maybe thats a little overkill just testing the NFS driver works. Its a rework of almost the whole test suite.

If you agree I would send an update with v6 of the NFS driver that just includes patch 1,2,3,16,17 of this series.
This enables for basic testing of the driver. The indepth tests for the formats should trigger a bug on any protocol
and I think its ok to test this with file.

Peter