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

[PATCH v3 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".

And once Max' "iotests: Add and use $SOCK_DIR" patch series has been
merged, we can indeed enable these Python-based tests, too.

There is just one small downside: Since these tests require a QEMU
that features a 'virtio-blk' device, we cannot run the iotests
with binaries like qemu-system-tricore anymore. But since the iotests
were not very useful with such binaries anyway, I think it's ok now
if we skip them there.

I've also added a patch that removes test 130 from the "auto" group
instead. Test 130 has been reported to fail intermittently, so we
should not use it in "make check" block until it is fixed.

Based-on: 20191010152457.17713-1-mreitz@redhat.com

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: Skip "make check-block" if QEMU does not support virtio-blk
  iotests: Enable more tests in the 'auto' group to improve test
    coverage
  iotests: Remove 130 from the "auto" group

 tests/check-block.sh          | 16 +++++++++++++++-
 tests/qemu-iotests/041        |  3 ++-
 tests/qemu-iotests/183        |  1 +
 tests/qemu-iotests/group      | 18 +++++++++---------
 tests/qemu-iotests/iotests.py | 16 +++++++++++-----
 5 files changed, 38 insertions(+), 16 deletions(-)

-- 
2.18.1

[PATCH v3 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 5373149ae1..7d6c2d3641 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -871,9 +871,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):
@@ -935,7 +940,8 @@ def execute_unittest(output, verbosity, debug):
                                     r'Ran \1 tests', output.getvalue()))
 
 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."""
@@ -952,7 +958,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 v3 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.

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 8568426311..0326888c98 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1123,4 +1123,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 v3 3/6] iotests: Test 183 does not work on macOS and OpenBSD

[Thomas Huth]

In the long term, 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
fine (i.e. Linux, FreeBSD and NetBSD).

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 bced83fae0..0bbae13647 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 v3 4/6] iotests: Skip "make check-block" if QEMU does not support virtio-blk

[Thomas Huth]

The next patch is going to add some python-based tests to 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 at the top level in the check-block.sh script instead
(so that it is possible to run "make check" without the "check-block"
part for qemu-system-tricore for example).

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/check-block.sh | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tests/check-block.sh b/tests/check-block.sh
index 679aedec50..e9e2978818 100755
--- a/tests/check-block.sh
+++ b/tests/check-block.sh
@@ -26,10 +26,24 @@ if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
     exit 0
 fi
 
-if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then
+if [ -n "$QEMU_PROG" ]; then
+    qemu_prog="$QEMU_PROG"
+else
+    for binary in *-softmmu/qemu-system-* ; do
+        if [ -x "$binary" ]; then
+            qemu_prog="$binary"
+            break
+        fi
+    done
+fi
+if [ -z "$qemu_prog" ]; then
     echo "No qemu-system binary available ==> Not running the qemu-iotests."
     exit 0
 fi
+if ! "$qemu_prog" -M none -device help | grep -q virtio-blk >/dev/null 2>&1 ; then
+    echo "$qemu_prog does not support virtio-blk ==> Not running the qemu-iotests."
+    exit 0
+fi
 
 if ! command -v bash >/dev/null 2>&1 ; then
     echo "bash not available ==> Not running the qemu-iotests."
-- 
2.18.1

[PATCH v3 5/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 091, 181, and 203 (which also tests iothreads).

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

diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index a73df279e5..33b499ed41 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
@@ -112,7 +112,7 @@
 088 rw quick
 089 rw auto quick
 090 rw auto quick
-091 rw migration
+091 rw auto migration
 092 rw quick
 093 throttle
 094 rw quick
@@ -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 auto 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

[PATCH v3 6/6] iotests: Remove 130 from the "auto" group

[Thomas Huth]

Peter hit a "Could not open 'TEST_DIR/t.IMGFMT': Failed to get shared
'write' lock - Is another process using the image [TEST_DIR/t.IMGFMT]?"
error with 130 already twice. Looks like this test is a little bit
shaky, so for the time being, let's disable it from the "auto" group so
that it does not gate the pull requests.

Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qemu-iotests/group | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 33b499ed41..4596497bce 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -151,7 +151,7 @@
 127 rw auto backing quick
 128 rw quick
 129 rw quick
-130 rw auto quick
+130 rw quick
 131 rw quick
 132 rw quick
 133 auto quick
-- 
2.18.1

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

[Thomas Huth]

On 22/10/2019 09.21, 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".
> 
> And once Max' "iotests: Add and use $SOCK_DIR" patch series has been
> merged, we can indeed enable these Python-based tests, too.

Oh well, some Travis jobs are now running too long and hit the 50
minutes limit:

 https://travis-ci.com/huth/qemu/jobs/248158477

... so we either might need to remove some other iotests from the "auto"
group again, or change the Travis jobs to include less targets...

That "clang + sanitizer" job was already running 45 minutes before my
change, so it was already close to the limit. So I'd suggest to change
it to include less targets. Opinions?

 Thomas

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

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> On 22/10/2019 09.21, 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".
>>
>> And once Max' "iotests: Add and use $SOCK_DIR" patch series has been
>> merged, we can indeed enable these Python-based tests, too.
>
> Oh well, some Travis jobs are now running too long and hit the 50
> minutes limit:
>
>  https://travis-ci.com/huth/qemu/jobs/248158477
>
> ... so we either might need to remove some other iotests from the "auto"
> group again, or change the Travis jobs to include less targets...
>
> That "clang + sanitizer" job was already running 45 minutes before my
> change, so it was already close to the limit. So I'd suggest to change
> it to include less targets. Opinions?

Which one is clang with sanitizers? I think we only build softmmu for
gcc + sanitizer at the moment.

>
>  Thomas


--
Alex Bennée

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

[Thomas Huth]

On 22/10/2019 13.39, Alex Bennée wrote:
> 
> Thomas Huth <thuth@redhat.com> writes:
> 
>> On 22/10/2019 09.21, 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".
>>>
>>> And once Max' "iotests: Add and use $SOCK_DIR" patch series has been
>>> merged, we can indeed enable these Python-based tests, too.
>>
>> Oh well, some Travis jobs are now running too long and hit the 50
>> minutes limit:
>>
>>  https://travis-ci.com/huth/qemu/jobs/248158477
>>
>> ... so we either might need to remove some other iotests from the "auto"
>> group again, or change the Travis jobs to include less targets...
>>
>> That "clang + sanitizer" job was already running 45 minutes before my
>> change, so it was already close to the limit. So I'd suggest to change
>> it to include less targets. Opinions?
> 
> Which one is clang with sanitizers? I think we only build softmmu for
> gcc + sanitizer at the moment.

I meant this one here:

    - env:
        - CONFIG="--target-list=${MAIN_SOFTMMU_TARGETS} "
      compiler: clang
      before_script:
        - ./configure ${CONFIG} --extra-cflags="-fsanitize=undefined
-Werror" || { cat config.log && exit 1; }

 Thomas

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

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> On 22/10/2019 13.39, Alex Bennée wrote:
>>
>> Thomas Huth <thuth@redhat.com> writes:
>>
>>> On 22/10/2019 09.21, 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".
>>>>
>>>> And once Max' "iotests: Add and use $SOCK_DIR" patch series has been
>>>> merged, we can indeed enable these Python-based tests, too.
>>>
>>> Oh well, some Travis jobs are now running too long and hit the 50
>>> minutes limit:
>>>
>>>  https://travis-ci.com/huth/qemu/jobs/248158477
>>>
>>> ... so we either might need to remove some other iotests from the "auto"
>>> group again, or change the Travis jobs to include less targets...
>>>
>>> That "clang + sanitizer" job was already running 45 minutes before my
>>> change, so it was already close to the limit. So I'd suggest to change
>>> it to include less targets. Opinions?
>>
>> Which one is clang with sanitizers? I think we only build softmmu for
>> gcc + sanitizer at the moment.
>
> I meant this one here:
>
>     - env:
>         - CONFIG="--target-list=${MAIN_SOFTMMU_TARGETS} "
>       compiler: clang
>       before_script:
>         - ./configure ${CONFIG} --extra-cflags="-fsanitize=undefined
> -Werror" || { cat config.log && exit 1; }

Hmm we already only do the main SOFTMMU targets. I wonder if we could be
caching better?

>
>  Thomas


--
Alex Bennée

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

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> 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

Queued to testing/next, thanks.

>
> it would be good to have some more valuable iotests enabled in the
> "auto" group to get better iotest coverage during "make check".
>
> And once Max' "iotests: Add and use $SOCK_DIR" patch series has been
> merged, we can indeed enable these Python-based tests, too.
>
> There is just one small downside: Since these tests require a QEMU
> that features a 'virtio-blk' device, we cannot run the iotests
> with binaries like qemu-system-tricore anymore. But since the iotests
> were not very useful with such binaries anyway, I think it's ok now
> if we skip them there.
>
> I've also added a patch that removes test 130 from the "auto" group
> instead. Test 130 has been reported to fail intermittently, so we
> should not use it in "make check" block until it is fixed.
>
> Based-on: 20191010152457.17713-1-mreitz@redhat.com
>
> 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: Skip "make check-block" if QEMU does not support virtio-blk
>   iotests: Enable more tests in the 'auto' group to improve test
>     coverage
>   iotests: Remove 130 from the "auto" group
>
>  tests/check-block.sh          | 16 +++++++++++++++-
>  tests/qemu-iotests/041        |  3 ++-
>  tests/qemu-iotests/183        |  1 +
>  tests/qemu-iotests/group      | 18 +++++++++---------
>  tests/qemu-iotests/iotests.py | 16 +++++++++++-----
>  5 files changed, 38 insertions(+), 16 deletions(-)


--
Alex Bennée

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

[Max Reitz]

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

On 22.10.19 15:11, Alex Bennée wrote:
> 
> Thomas Huth <thuth@redhat.com> writes:
> 
>> 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
> 
> Queued to testing/next, thanks.

It should be noted that this series depends on my SOCK_DIR series (which
I have in my block branch), or the newly added tests are likely to fail
in the CI environment.

Max

>>
>> it would be good to have some more valuable iotests enabled in the
>> "auto" group to get better iotest coverage during "make check".
>>
>> And once Max' "iotests: Add and use $SOCK_DIR" patch series has been
>> merged, we can indeed enable these Python-based tests, too.
>>
>> There is just one small downside: Since these tests require a QEMU
>> that features a 'virtio-blk' device, we cannot run the iotests
>> with binaries like qemu-system-tricore anymore. But since the iotests
>> were not very useful with such binaries anyway, I think it's ok now
>> if we skip them there.
>>
>> I've also added a patch that removes test 130 from the "auto" group
>> instead. Test 130 has been reported to fail intermittently, so we
>> should not use it in "make check" block until it is fixed.
>>
>> Based-on: 20191010152457.17713-1-mreitz@redhat.com
>>
>> 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: Skip "make check-block" if QEMU does not support virtio-blk
>>   iotests: Enable more tests in the 'auto' group to improve test
>>     coverage
>>   iotests: Remove 130 from the "auto" group
>>
>>  tests/check-block.sh          | 16 +++++++++++++++-
>>  tests/qemu-iotests/041        |  3 ++-
>>  tests/qemu-iotests/183        |  1 +
>>  tests/qemu-iotests/group      | 18 +++++++++---------
>>  tests/qemu-iotests/iotests.py | 16 +++++++++++-----
>>  5 files changed, 38 insertions(+), 16 deletions(-)
> 
> 
> --
> Alex Bennée
> 



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

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

[Alex Bennée]

Max Reitz <mreitz@redhat.com> writes:

> On 22.10.19 15:11, Alex Bennée wrote:
>>
>> Thomas Huth <thuth@redhat.com> writes:
>>
>>> 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
>>
>> Queued to testing/next, thanks.
>
> It should be noted that this series depends on my SOCK_DIR series (which
> I have in my block branch), or the newly added tests are likely to fail
> in the CI environment.

Ahh I misread....
<snip>

>>> it would be good to have some more valuable iotests enabled in the
>>> "auto" group to get better iotest coverage during "make check".
>>>
>>> And once Max' "iotests: Add and use $SOCK_DIR" patch series has been
>>> merged, we can indeed enable these Python-based tests, too.

I though these weren't enabled in this series. Do I need to drop all the
patches?

--
Alex Bennée

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

[Thomas Huth]

On 22/10/2019 15.48, Alex Bennée wrote:
> 
> Max Reitz <mreitz@redhat.com> writes:
> 
>> On 22.10.19 15:11, Alex Bennée wrote:
>>>
>>> Thomas Huth <thuth@redhat.com> writes:
>>>
>>>> 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
>>>
>>> Queued to testing/next, thanks.
>>
>> It should be noted that this series depends on my SOCK_DIR series (which
>> I have in my block branch), or the newly added tests are likely to fail
>> in the CI environment.
> 
> Ahh I misread....
> <snip>
> 
>>>> it would be good to have some more valuable iotests enabled in the
>>>> "auto" group to get better iotest coverage during "make check".
>>>>
>>>> And once Max' "iotests: Add and use $SOCK_DIR" patch series has been
>>>> merged, we can indeed enable these Python-based tests, too.
> 
> I though these weren't enabled in this series. Do I need to drop all the
> patches?

I think it's better if the iotest patches go through Max' or Kevin's
block tree.

 Thomas

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

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> On 22/10/2019 15.48, Alex Bennée wrote:
>>
>> Max Reitz <mreitz@redhat.com> writes:
>>
>>> On 22.10.19 15:11, Alex Bennée wrote:
>>>>
>>>> Thomas Huth <thuth@redhat.com> writes:
>>>>
>>>>> 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
>>>>
>>>> Queued to testing/next, thanks.
>>>
>>> It should be noted that this series depends on my SOCK_DIR series (which
>>> I have in my block branch), or the newly added tests are likely to fail
>>> in the CI environment.
>>
>> Ahh I misread....
>> <snip>
>>
>>>>> it would be good to have some more valuable iotests enabled in the
>>>>> "auto" group to get better iotest coverage during "make check".
>>>>>
>>>>> And once Max' "iotests: Add and use $SOCK_DIR" patch series has been
>>>>> merged, we can indeed enable these Python-based tests, too.
>>
>> I though these weren't enabled in this series. Do I need to drop all the
>> patches?
>
> I think it's better if the iotest patches go through Max' or Kevin's
> block tree.

OK I can drop them from my tree.

>
>  Thomas


--
Alex Bennée

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

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

<snip>
>
> 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 091, 181, and 203 (which also tests iothreads).
<snip>
> @@ -112,7 +112,7 @@
>  088 rw quick
>  089 rw auto quick
>  090 rw auto quick
> -091 rw migration
> +091 rw auto migration


This is breaking consistently on my ZFS machine:

TEST    iotest-qcow2: 091 [fail]
QEMU          -- "/home/alex.bennee/lsrc/qemu.git/tests/qemu-iotests/../../x86_64-softmmu/qemu-system-x86_64" -nodefaults -display none -machine accel=qtest
QEMU_IMG      -- "/home/alex.bennee/lsrc/qemu.git/tests/qemu-iotests/../../qemu-img"
QEMU_IO       -- "/home/alex.bennee/lsrc/qemu.git/tests/qemu-iotests/../../qemu-io"  --cache writeback -f qcow2
QEMU_NBD      -- "/home/alex.bennee/lsrc/qemu.git/tests/qemu-iotests/../../qemu-nbd"
IMGFMT        -- qcow2 (compat=1.1)
IMGPROTO      -- file
PLATFORM      -- Linux/x86_64 hackbox2 4.15.0-66-generic
TEST_DIR      -- /home/alex.bennee/lsrc/qemu.git/tests/qemu-iotests/scratch
SOCKET_SCM_HELPER -- /home/alex.bennee/lsrc/qemu.git/tests/qemu-iotests/socket_scm_helper

--- /home/alex.bennee/lsrc/qemu.git/tests/qemu-iotests/091.out  2019-02-19 12:32:03.231730789 +0000
+++ /home/alex.bennee/lsrc/qemu.git/tests/qemu-iotests/091.out.bad      2019-10-24 12:07:00.624967556 +0100
@@ -9,20 +9,4 @@

 === VM 1: Migrate from VM1 to VM2 ===

-vm1: qemu-io disk write complete
-vm1: live migration started
-vm1: live migration completed
-
-=== VM 2: Post-migration, write to disk, verify running ===
-
-vm2: qemu-io disk write complete
-vm2: qemu process running successfully
-vm2: flush io, and quit
-Check image pattern
-read 4194304/4194304 bytes at offset 0
-4 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-Running 'qemu-img check -r all $TEST_IMG'
-No errors were found on the image.
-80/16384 = 0.49% allocated, 0.00% fragmented, 0.00% compressed clusters
-Image end offset: 5570560
-*** done
+Timeout waiting for (qemu) on handle 0


--
Alex Bennée

Re: [PATCH v3 4/6] iotests: Skip "make check-block" if QEMU does not support virtio-blk

[Max Reitz]

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

On 22.10.19 09:21, Thomas Huth wrote:
> The next patch is going to add some python-based tests to 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 at the top level in the check-block.sh script instead
> (so that it is possible to run "make check" without the "check-block"
> part for qemu-system-tricore for example).
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  tests/check-block.sh | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/check-block.sh b/tests/check-block.sh
> index 679aedec50..e9e2978818 100755
> --- a/tests/check-block.sh
> +++ b/tests/check-block.sh
> @@ -26,10 +26,24 @@ if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
>      exit 0
>  fi
>  
> -if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then
> +if [ -n "$QEMU_PROG" ]; then
> +    qemu_prog="$QEMU_PROG"
> +else
> +    for binary in *-softmmu/qemu-system-* ; do

Hm, I know I’ve already given my R-b, but looking at this again – what
if the user builds qemu for multiple targets?  Then this will just test
any target, whereas the iotests might test something else, because the
algorithm there is slightly different:

First, check $QEMU_PROG (same as here).

Second, check $build_iotests/qemu.  I think we can do this here, because
we know that $build_iotests is $PWD/tests/qemu-iotests (or invoking
./check below wouldn’t work).

Third, and this is actually important, I think, is that we first look
for the qemu that matches the host architecture (uname -m, with an
exception for ppc64).  I think we really should do that here, too.

Fourth, look for any qemu, as is done here.


So I think we could do without #2, but it probably doesn’t hurt to check
that, too.  I don’t think we should do without #3, though.

Max


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

Re: [PATCH v3 4/6] iotests: Skip "make check-block" if QEMU does not support virtio-blk

[Thomas Huth]

On 30/10/2019 12.21, Max Reitz wrote:
> On 22.10.19 09:21, Thomas Huth wrote:
>> The next patch is going to add some python-based tests to 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 at the top level in the check-block.sh script instead
>> (so that it is possible to run "make check" without the "check-block"
>> part for qemu-system-tricore for example).
>>
>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>  tests/check-block.sh | 16 +++++++++++++++-
>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/check-block.sh b/tests/check-block.sh
>> index 679aedec50..e9e2978818 100755
>> --- a/tests/check-block.sh
>> +++ b/tests/check-block.sh
>> @@ -26,10 +26,24 @@ if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
>>      exit 0
>>  fi
>>  
>> -if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then
>> +if [ -n "$QEMU_PROG" ]; then
>> +    qemu_prog="$QEMU_PROG"
>> +else
>> +    for binary in *-softmmu/qemu-system-* ; do
> 
> Hm, I know I’ve already given my R-b, but looking at this again – what
> if the user builds qemu for multiple targets?  Then this will just test
> any target, whereas the iotests might test something else, because the
> algorithm there is slightly different:
> 
> First, check $QEMU_PROG (same as here).
> 
> Second, check $build_iotests/qemu.  I think we can do this here, because
> we know that $build_iotests is $PWD/tests/qemu-iotests (or invoking
> ./check below wouldn’t work).
> 
> Third, and this is actually important, I think, is that we first look
> for the qemu that matches the host architecture (uname -m, with an
> exception for ppc64).  I think we really should do that here, too.
> 
> Fourth, look for any qemu, as is done here.
> 
> So I think we could do without #2, but it probably doesn’t hurt to check
> that, too.  I don’t think we should do without #3, though.

Maybe we should simply move the check into tests/qemu-iotests/check to
avoid duplication of that logic?
We could then also only simply skip the python tests instead of skipping
everything, in case the chosen QEMU binary does not support virtio-blk.

 Thomas

Re: [PATCH v3 4/6] iotests: Skip "make check-block" if QEMU does not support virtio-blk

[Max Reitz]

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

On 11.11.19 15:02, Thomas Huth wrote:
> On 30/10/2019 12.21, Max Reitz wrote:
>> On 22.10.19 09:21, Thomas Huth wrote:
>>> The next patch is going to add some python-based tests to 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 at the top level in the check-block.sh script instead
>>> (so that it is possible to run "make check" without the "check-block"
>>> part for qemu-system-tricore for example).
>>>
>>> Reviewed-by: Max Reitz <mreitz@redhat.com>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>  tests/check-block.sh | 16 +++++++++++++++-
>>>  1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/check-block.sh b/tests/check-block.sh
>>> index 679aedec50..e9e2978818 100755
>>> --- a/tests/check-block.sh
>>> +++ b/tests/check-block.sh
>>> @@ -26,10 +26,24 @@ if grep -q "CFLAGS.*-fsanitize" config-host.mak 2>/dev/null ; then
>>>      exit 0
>>>  fi
>>>  
>>> -if [ -z "$(find . -name 'qemu-system-*' -print)" ]; then
>>> +if [ -n "$QEMU_PROG" ]; then
>>> +    qemu_prog="$QEMU_PROG"
>>> +else
>>> +    for binary in *-softmmu/qemu-system-* ; do
>>
>> Hm, I know I’ve already given my R-b, but looking at this again – what
>> if the user builds qemu for multiple targets?  Then this will just test
>> any target, whereas the iotests might test something else, because the
>> algorithm there is slightly different:
>>
>> First, check $QEMU_PROG (same as here).
>>
>> Second, check $build_iotests/qemu.  I think we can do this here, because
>> we know that $build_iotests is $PWD/tests/qemu-iotests (or invoking
>> ./check below wouldn’t work).
>>
>> Third, and this is actually important, I think, is that we first look
>> for the qemu that matches the host architecture (uname -m, with an
>> exception for ppc64).  I think we really should do that here, too.
>>
>> Fourth, look for any qemu, as is done here.
>>
>> So I think we could do without #2, but it probably doesn’t hurt to check
>> that, too.  I don’t think we should do without #3, though.
> 
> Maybe we should simply move the check into tests/qemu-iotests/check to
> avoid duplication of that logic?
> We could then also only simply skip the python tests instead of skipping
> everything, in case the chosen QEMU binary does not support virtio-blk.

You mean by adding a new flag e.g. -batch that’s supposed to generally
skip cases when in doubt that they can be run on the current host?
Sounds good to me.

Max


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

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

[Thomas Huth]

On 24/10/2019 13.14, Alex Bennée wrote:
> 
> Thomas Huth <thuth@redhat.com> writes:
> 
> <snip>
>>
>> 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 091, 181, and 203 (which also tests iothreads).
> <snip>
>> @@ -112,7 +112,7 @@
>>   088 rw quick
>>   089 rw auto quick
>>   090 rw auto quick
>> -091 rw migration
>> +091 rw auto migration
> 
> 
> This is breaking consistently on my ZFS machine:
> 
> TEST    iotest-qcow2: 091 [fail]

OK, I'll drop 091 again from the auto group in the next version of this 
series.

  Thomas