[PATCH v5 0/6] Enable more iotests during "make check-block"

[PATCH v5 0/6] Enable more iotests during "make check-block"

[Thomas Huth]

As discussed here:

 https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg00697.html

and here:

 https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg01388.html

it would be good to have some more valuable iotests enabled in the
"auto" group to get better iotest coverage during "make check".

Since these Python-based tests require a QEMU that features a 'virtio-blk'
device, we can only run the Python tests if this device is available. With
binaries like qemu-system-tricore, the Python-based tests will be skipped.

v5:
 - Added $python_unusable_because text in the fifth patch
 - Rebased to master and checked that everything still works fine on
   Travis-CI, Cirrus-CI, Gitlab-CI, OpenBSD and NetBSD.

v4:
 - The check for 'virtio-blk' is now done in the tests/qemu-iotests/check
   script instead of tests/check-block.sh (to avoid to duplicate the code
   that searches for the right QEMU binary - and we can also still run
   the shell-based tests this way).
 - Added the new patch to check for the availability of virtio devices in
   the iotests 127 and 267.
 - The patch that drops test 130 from the "auto" group has already been
   merged and thus been dropped from this series.

v3:
 - Test 183 fails on Patchew, so I removed it from the "auto" group
   again

v2:
 - Checked the iotests with NetBSD, too (now that Eduardo has
   re-activated Gerd's patches for creating NetBSD VM images)
 - Use 'openbsd' instead of 'openbsd6'
 - Use 'grep -q' instead of 'grep' for grep'ing silently
 - Added the patch to disable 130 from the "auto" group

John Snow (1):
  iotests: remove 'linux' from default supported platforms

Thomas Huth (5):
  iotests: Test 041 only works on certain systems
  iotests: Test 183 does not work on macOS and OpenBSD
  iotests: Check for the availability of the required devices in 267 and
    127
  iotests: Skip Python-based tests if QEMU does not support virtio-blk
  iotests: Enable more tests in the 'auto' group to improve test
    coverage

 tests/qemu-iotests/041        |  3 ++-
 tests/qemu-iotests/127        |  2 ++
 tests/qemu-iotests/183        |  1 +
 tests/qemu-iotests/267        |  2 ++
 tests/qemu-iotests/check      | 12 ++++++++++--
 tests/qemu-iotests/common.rc  | 14 ++++++++++++++
 tests/qemu-iotests/group      | 14 +++++++-------
 tests/qemu-iotests/iotests.py | 16 +++++++++++-----
 8 files changed, 49 insertions(+), 15 deletions(-)

-- 
2.18.1

[PATCH v5 1/6] iotests: remove 'linux' from default supported platforms

[Thomas Huth]

From: John Snow <jsnow@redhat.com>

verify_platform will check an explicit whitelist and blacklist instead.
The default will now be assumed to be allowed to run anywhere.

For tests that do not specify their platforms explicitly, this has the effect of
enabling these tests on non-linux platforms. For tests that always specified
linux explicitly, there is no change.

For Python tests on FreeBSD at least; only seven python tests fail:
045 147 149 169 194 199 211

045 and 149 appear to be misconfigurations,
147 and 194 are the AF_UNIX path too long error,
169 and 199 are bitmap migration bugs, and
211 is a bug that shows up on Linux platforms, too.

This is at least good evidence that these tests are not Linux-only. If
they aren't suitable for other platforms, they should be disabled on a
per-platform basis as appropriate.

Therefore, let's switch these on and deal with the failures.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/iotests.py | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 13fd8b5cd2..1f6d918d6d 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -925,9 +925,14 @@ def verify_protocol(supported=[], unsupported=[]):
     if not_sup or (imgproto in unsupported):
         notrun('not suitable for this protocol: %s' % imgproto)
 
-def verify_platform(supported_oses=['linux']):
-    if True not in [sys.platform.startswith(x) for x in supported_oses]:
-        notrun('not suitable for this OS: %s' % sys.platform)
+def verify_platform(supported=None, unsupported=None):
+    if unsupported is not None:
+        if any((sys.platform.startswith(x) for x in unsupported)):
+            notrun('not suitable for this OS: %s' % sys.platform)
+
+    if supported is not None:
+        if not any((sys.platform.startswith(x) for x in supported)):
+            notrun('not suitable for this OS: %s' % sys.platform)
 
 def verify_cache_mode(supported_cache_modes=[]):
     if supported_cache_modes and (cachemode not in supported_cache_modes):
@@ -1018,7 +1023,8 @@ def execute_unittest(output, verbosity, debug):
             sys.stderr.write(out)
 
 def execute_test(test_function=None,
-                 supported_fmts=[], supported_oses=['linux'],
+                 supported_fmts=[],
+                 supported_platforms=None,
                  supported_cache_modes=[], unsupported_fmts=[],
                  supported_protocols=[], unsupported_protocols=[]):
     """Run either unittest or script-style tests."""
@@ -1035,7 +1041,7 @@ def execute_test(test_function=None,
     verbosity = 1
     verify_image_format(supported_fmts, unsupported_fmts)
     verify_protocol(supported_protocols, unsupported_protocols)
-    verify_platform(supported_oses)
+    verify_platform(supported=supported_platforms)
     verify_cache_mode(supported_cache_modes)
 
     if debug:
-- 
2.18.1

[PATCH v5 2/6] iotests: Test 041 only works on certain systems

[Thomas Huth]

041 works fine on Linux, FreeBSD, NetBSD and OpenBSD, but fails on macOS.
Let's mark it as only supported on the systems where we know that it is
working fine.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qemu-iotests/041 | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index c07437fda1..0181f7a9b6 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1134,4 +1134,5 @@ class TestOrphanedSource(iotests.QMPTestCase):
 
 if __name__ == '__main__':
     iotests.main(supported_fmts=['qcow2', 'qed'],
-                 supported_protocols=['file'])
+                 supported_protocols=['file'],
+                 supported_platforms=['linux', 'freebsd', 'netbsd', 'openbsd'])
-- 
2.18.1

[PATCH v5 3/6] iotests: Test 183 does not work on macOS and OpenBSD

[Thomas Huth]

In the long run, we might want to add test 183 to the "auto" group
(but it still fails occasionally, so we cannot do that yet). However,
when running 183 in Cirrus-CI on macOS, or with our vm-build-openbsd
target, it currently always fails with an "Timeout waiting for return
on handle 0" error.

Let's mark it as supported only on systems where the test is working
most of the time (i.e. Linux, FreeBSD and NetBSD).

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qemu-iotests/183 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183
index 3f74b9f62d..a5e0d7e8b7 100755
--- a/tests/qemu-iotests/183
+++ b/tests/qemu-iotests/183
@@ -42,6 +42,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 . ./common.qemu
 
+_supported_os Linux FreeBSD NetBSD
 _supported_fmt qcow2 raw qed quorum
 _supported_proto file
 
-- 
2.18.1

[PATCH v5 4/6] iotests: Check for the availability of the required devices in 267 and 127

[Thomas Huth]

We are going to enable 127 in the "auto" group, but it only works if
virtio-scsi and scsi-hd are available - which is not the case with
QEMU binaries like qemu-system-tricore for example, so we need a
proper check for the availability of these devices here.

A very similar problem exists in iotest 267 - it has been added to
the "auto" group already, but requires virtio-blk and thus currently
fails with qemu-system-tricore for example. Let's also add aproper
check there.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qemu-iotests/127       |  2 ++
 tests/qemu-iotests/267       |  2 ++
 tests/qemu-iotests/common.rc | 14 ++++++++++++++
 3 files changed, 18 insertions(+)

diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
index b64926ab31..a4fc866038 100755
--- a/tests/qemu-iotests/127
+++ b/tests/qemu-iotests/127
@@ -43,6 +43,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 _supported_fmt qcow2
 _supported_proto file
 
+_require_devices virtio-scsi scsi-hd
+
 IMG_SIZE=64K
 
 _make_test_img $IMG_SIZE
diff --git a/tests/qemu-iotests/267 b/tests/qemu-iotests/267
index c296877168..3146273eef 100755
--- a/tests/qemu-iotests/267
+++ b/tests/qemu-iotests/267
@@ -46,6 +46,8 @@ _require_drivers copy-on-read
 # and generally impossible with external data files
 _unsupported_imgopts 'refcount_bits=1[^0-9]' data_file
 
+_require_devices virtio-blk
+
 do_run_qemu()
 {
     echo Testing: "$@"
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index d088392ab6..4dfe92e6e3 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -699,5 +699,19 @@ _require_large_file()
     rm "$TEST_IMG"
 }
 
+# Check that a set of devices is available in the QEMU binary
+#
+_require_devices()
+{
+    available=$($QEMU -M none -device help | \
+                grep ^name | sed -e 's/^name "//' -e 's/".*$//')
+    for device
+    do
+        if ! echo "$available" | grep -q "$device" ; then
+            _notrun "$device not available"
+        fi
+    done
+}
+
 # make sure this script returns success
 true
-- 
2.18.1

[PATCH v5 5/6] iotests: Skip Python-based tests if QEMU does not support virtio-blk

[Thomas Huth]

We are going to enable some of the python-based tests in the "auto" group,
and these tests require virtio-blk to work properly. Running iotests
without virtio-blk likely does not make too much sense anyway, so instead
of adding a check for the availability of virtio-blk to each and every
test (which does not sound very appealing), let's rather add a check for
this a central spot in the "check" script instead (so that it is still
possible to run "make check" for qemu-system-tricore for example).

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qemu-iotests/check | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 2890785a10..1629b6c914 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -642,7 +642,15 @@ fi
 python_usable=false
 if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
 then
-    python_usable=true
+    # Our python framework also requires virtio-blk
+    if "$QEMU_PROG" -M none -device help | grep -q virtio-blk >/dev/null 2>&1
+    then
+        python_usable=true
+    else
+        python_unusable_because="Missing virtio-blk in QEMU binary"
+    fi
+else
+    python_unusable_because="Unsupported Python version"
 fi
 
 default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
@@ -830,7 +838,7 @@ do
                 run_command="$PYTHON $seq"
             else
                 run_command="false"
-                echo "Unsupported Python version" > $seq.notrun
+                echo "$python_unusable_because" > $seq.notrun
             fi
         else
             run_command="./$seq"
-- 
2.18.1

[PATCH v5 6/6] iotests: Enable more tests in the 'auto' group to improve test coverage

[Thomas Huth]

According to Kevin, tests 030, 040 and 041 are among the most valuable
tests that we have, so we should always run them if possible, even if
they take a little bit longer.

According to Max, it would be good to have a test for iothreads and
migration. 127 and 256 seem to be good candidates for iothreads. For
migration, let's enable 181 and 203 (which also tests iothreads).
(091 would be a good candidate for migration, too, but Alex Bennée
reported that this test fails on ZFS file systems, so it can't be
included yet)

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qemu-iotests/group | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index cb2b789e44..15a005b467 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -51,7 +51,7 @@
 027 rw auto quick
 028 rw backing quick
 029 rw auto quick
-030 rw backing
+030 rw auto backing
 031 rw auto quick
 032 rw auto quick
 033 rw auto quick
@@ -61,8 +61,8 @@
 037 rw auto backing quick
 038 rw auto backing quick
 039 rw auto quick
-040 rw
-041 rw backing
+040 rw auto
+041 rw auto backing
 042 rw auto quick
 043 rw auto backing
 044 rw
@@ -148,7 +148,7 @@
 124 rw backing
 125 rw
 126 rw auto backing
-127 rw backing quick
+127 rw auto backing quick
 128 rw quick
 129 rw quick
 130 rw quick
@@ -197,7 +197,7 @@
 177 rw auto quick
 178 img
 179 rw auto quick
-181 rw migration
+181 rw auto migration
 182 rw quick
 183 rw migration
 184 rw auto quick
@@ -218,7 +218,7 @@
 200 rw
 201 rw migration
 202 rw quick
-203 rw migration
+203 rw auto migration
 204 rw quick
 205 rw quick
 206 rw
@@ -270,7 +270,7 @@
 253 rw quick
 254 rw backing quick
 255 rw quick
-256 rw quick
+256 rw auto quick
 257 rw
 258 rw quick
 260 rw quick
-- 
2.18.1

Re: [PATCH v5 0/6] Enable more iotests during "make check-block"

[Max Reitz]

[-- Attachment #1.1: Type: text/plain, Size: 718 bytes --]

On 21.01.20 10:51, Thomas Huth wrote:
> As discussed here:
> 
>  https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg00697.html
> 
> and here:
> 
>  https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg01388.html
> 
> it would be good to have some more valuable iotests enabled in the
> "auto" group to get better iotest coverage during "make check".
> 
> Since these Python-based tests require a QEMU that features a 'virtio-blk'
> device, we can only run the Python tests if this device is available. With
> binaries like qemu-system-tricore, the Python-based tests will be skipped.

Thanks, applied to my block branch:

https://git.xanclic.moe/XanClic/qemu/commits/branch/block

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v5 5/6] iotests: Skip Python-based tests if QEMU does not support virtio-blk

[Philippe Mathieu-Daudé]

On 1/21/20 10:52 AM, Thomas Huth wrote:
> We are going to enable some of the python-based tests in the "auto" group,
> and these tests require virtio-blk to work properly. Running iotests
> without virtio-blk likely does not make too much sense anyway, so instead
> of adding a check for the availability of virtio-blk to each and every
> test (which does not sound very appealing), let's rather add a check for
> this a central spot in the "check" script instead (so that it is still
> possible to run "make check" for qemu-system-tricore for example).
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/qemu-iotests/check | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 2890785a10..1629b6c914 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -642,7 +642,15 @@ fi
>   python_usable=false
>   if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6) else 1)'
>   then
> -    python_usable=true
> +    # Our python framework also requires virtio-blk
> +    if "$QEMU_PROG" -M none -device help | grep -q virtio-blk >/dev/null 2>&1

FYI I proposed a patch adding a binary_get_devices() helper:
https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg07314.html

You could use something such

   @SkipUntil('virtio-blk' in binary_get_devices(qemu_bin))

> +    then
> +        python_usable=true
> +    else
> +        python_unusable_because="Missing virtio-blk in QEMU binary"
> +    fi
> +else
> +    python_unusable_because="Unsupported Python version"
>   fi
>   
>   default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
> @@ -830,7 +838,7 @@ do
>                   run_command="$PYTHON $seq"
>               else
>                   run_command="false"
> -                echo "Unsupported Python version" > $seq.notrun
> +                echo "$python_unusable_because" > $seq.notrun
>               fi
>           else
>               run_command="./$seq"
> 

Re: [PATCH v5 5/6] iotests: Skip Python-based tests if QEMU does not support virtio-blk

[Thomas Huth]

On 30/01/2020 23.31, Philippe Mathieu-Daudé wrote:
> On 1/21/20 10:52 AM, Thomas Huth wrote:
>> We are going to enable some of the python-based tests in the "auto"
>> group,
>> and these tests require virtio-blk to work properly. Running iotests
>> without virtio-blk likely does not make too much sense anyway, so instead
>> of adding a check for the availability of virtio-blk to each and every
>> test (which does not sound very appealing), let's rather add a check for
>> this a central spot in the "check" script instead (so that it is still
>> possible to run "make check" for qemu-system-tricore for example).
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/qemu-iotests/check | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>> index 2890785a10..1629b6c914 100755
>> --- a/tests/qemu-iotests/check
>> +++ b/tests/qemu-iotests/check
>> @@ -642,7 +642,15 @@ fi
>>   python_usable=false
>>   if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6)
>> else 1)'
>>   then
>> -    python_usable=true
>> +    # Our python framework also requires virtio-blk
>> +    if "$QEMU_PROG" -M none -device help | grep -q virtio-blk
>> >/dev/null 2>&1
> 
> FYI I proposed a patch adding a binary_get_devices() helper:
> https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg07314.html
> 
> You could use something such
> 
>   @SkipUntil('virtio-blk' in binary_get_devices(qemu_bin))

Unfortunately, that doesn't scale here. You'd have to add this to almost
all python-based iotests, since the virtio-blk dependency is hard-wired
deep in the code there (look at the add_drive function).

 Thomas

Re: [PATCH v5 5/6] iotests: Skip Python-based tests if QEMU does not support virtio-blk

[Philippe Mathieu-Daudé]

On 1/31/20 8:08 AM, Thomas Huth wrote:
> On 30/01/2020 23.31, Philippe Mathieu-Daudé wrote:
>> On 1/21/20 10:52 AM, Thomas Huth wrote:
>>> We are going to enable some of the python-based tests in the "auto"
>>> group,
>>> and these tests require virtio-blk to work properly. Running iotests
>>> without virtio-blk likely does not make too much sense anyway, so instead
>>> of adding a check for the availability of virtio-blk to each and every
>>> test (which does not sound very appealing), let's rather add a check for
>>> this a central spot in the "check" script instead (so that it is still
>>> possible to run "make check" for qemu-system-tricore for example).
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>    tests/qemu-iotests/check | 12 ++++++++++--
>>>    1 file changed, 10 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
>>> index 2890785a10..1629b6c914 100755
>>> --- a/tests/qemu-iotests/check
>>> +++ b/tests/qemu-iotests/check
>>> @@ -642,7 +642,15 @@ fi
>>>    python_usable=false
>>>    if $PYTHON -c 'import sys; sys.exit(0 if sys.version_info >= (3,6)
>>> else 1)'
>>>    then
>>> -    python_usable=true
>>> +    # Our python framework also requires virtio-blk
>>> +    if "$QEMU_PROG" -M none -device help | grep -q virtio-blk
>>>> /dev/null 2>&1
>>
>> FYI I proposed a patch adding a binary_get_devices() helper:
>> https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg07314.html
>>
>> You could use something such
>>
>>    @SkipUntil('virtio-blk' in binary_get_devices(qemu_bin))
> 
> Unfortunately, that doesn't scale here. You'd have to add this to almost
> all python-based iotests, since the virtio-blk dependency is hard-wired
> deep in the code there (look at the add_drive function).

I see.

Back to your patch:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>