[Qemu-devel] [PATCH 0/2] iotests: Skip 181 and 201 without userfaultfd

[Qemu-devel] [PATCH 0/2] iotests: Skip 181 and 201 without userfaultfd

[Max Reitz]

My non-Fedora testing system does not have a kernel with userfaultfd
support which causes 181 and 201 to fail.  That is annoying.  This
series makes those tests recognize the issue and convert it into a
_notrun.


Max Reitz (2):
  iotests: Add failure matching to common.qemu
  iotests: Skip 181 and 201 without userfaultfd

 tests/qemu-iotests/181         | 13 ++++++++++
 tests/qemu-iotests/201         | 13 ++++++++++
 tests/qemu-iotests/common.qemu | 58 +++++++++++++++++++++++++++++++++++++-----
 3 files changed, 77 insertions(+), 7 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 1/2] iotests: Add failure matching to common.qemu

[Max Reitz]

Currently, common.qemu only allows to match for results indicating
success.  The only way to fail is by provoking a timeout.  However,
sometimes we do have a defined failure output and can match for that,
which saves us from having to wait for the timeout in case of failure.
Because failure can sometimes just result in a _notrun in the test, it
is actually important to care about being able to fail quickly.

Also, sometimes we simply do not get any specific output in case of
success.  The only way to handle this currently would be to define an
error message as the string to look for, which means that actual success
results in a timeout.  This is really bad because it unnecessarily slows
down a succeeding test.

Therefore, this patch adds a new parameter $success_or_failure to
_timed_wait_for and _send_qemu_cmd.  Setting this to a non-empty string
makes both commands expect two match parameters: If the first matches,
the function succeeds.  If the second matches, the function fails.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/common.qemu | 58 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 51 insertions(+), 7 deletions(-)

diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
index 85f66b852c..f285484951 100644
--- a/tests/qemu-iotests/common.qemu
+++ b/tests/qemu-iotests/common.qemu
@@ -52,11 +52,29 @@ _in_fd=4
 # response is not echoed out.
 # If $mismatch_only is set, only non-matching responses will
 # be echoed.
+#
+# If $success_or_failure is set, the meaning of the arguments is
+# changed as follows:
+# $2: A string to search for in the response; if found, this indicates
+#     success and ${QEMU_STATUS[$1]} is set to 0.
+# $3: A string to search for in the response; if found, this indicates
+#     failure and the test is either aborted (if $qemu_error_no_exit
+#     is not set) or ${QEMU_STATUS[$1]} is set to -1 (otherwise).
 function _timed_wait_for()
 {
     local h=${1}
     shift
 
+    if [ -z "${success_or_failure}" ]; then
+        success_match=${*}
+        failure_match=
+    else
+        success_match=${1}
+        failure_match=${2}
+    fi
+
+    timeout=yes
+
     QEMU_STATUS[$h]=0
     while IFS= read -t ${QEMU_COMM_TIMEOUT} resp <&${QEMU_OUT[$h]}
     do
@@ -64,10 +82,18 @@ function _timed_wait_for()
             echo "${resp}" | _filter_testdir | _filter_qemu \
                            | _filter_qemu_io | _filter_qmp | _filter_hmp
         fi
-        grep -q "${*}" < <(echo "${resp}")
+        if [ -n "${failure_match}" ]; then
+            grep -q "${failure_match}" < <(echo "${resp}")
+            if [ $? -eq 0 ]; then
+                timeout=
+                break
+            fi
+        fi
+        grep -q "${success_match}" < <(echo "${resp}")
         if [ $? -eq 0 ]; then
             return
-        elif [ -z "${silent}" ] && [ -n "${mismatch_only}" ]; then
+        fi
+        if [ -z "${silent}" ] && [ -n "${mismatch_only}" ]; then
             echo "${resp}" | _filter_testdir | _filter_qemu \
                            | _filter_qemu_io | _filter_qmp | _filter_hmp
         fi
@@ -75,8 +101,12 @@ function _timed_wait_for()
     done
     QEMU_STATUS[$h]=-1
     if [ -z "${qemu_error_no_exit}" ]; then
-        echo "Timeout waiting for ${*} on handle ${h}"
-        exit 1  # Timeout means the test failed
+        if [ -n "${timeout}" ]; then
+            echo "Timeout waiting for ${success_match} on handle ${h}"
+        else
+            echo "Wrong response matching ${failure_match} on handle ${h}"
+        fi
+        exit 1  # Timeout or wrong match mean the test failed
     fi
 }
 
@@ -96,6 +126,11 @@ function _timed_wait_for()
 # If $qemu_error_no_exit is set, then even if the expected response
 # is not seen, we will not exit.  $QEMU_STATUS[$1] will be set it -1 in
 # that case.
+#
+# If $success_or_failure is set, then the last two strings are the
+# strings the response will be scanned for.  The first of the two
+# indicates success, the latter indicates failure.  Failure is handled
+# like a timeout.
 function _send_qemu_cmd()
 {
     local h=${1}
@@ -109,14 +144,23 @@ function _send_qemu_cmd()
         use_error="no"
     fi
     # This array element extraction is done to accommodate pathnames with spaces
-    cmd=${@: 1:${#@}-1}
-    shift $(($# - 1))
+    if [ -z "${success_or_failure}" ]; then
+        cmd=${@: 1:${#@}-1}
+        shift $(($# - 1))
+    else
+        cmd=${@: 1:${#@}-2}
+        shift $(($# - 2))
+    fi
 
     while [ ${count} -gt 0 ]
     do
         echo "${cmd}" >&${QEMU_IN[${h}]}
         if [ -n "${1}" ]; then
-            qemu_error_no_exit=${use_error} _timed_wait_for ${h} "${1}"
+            if [ -z "${success_or_failure}" ]; then
+                qemu_error_no_exit=${use_error} _timed_wait_for ${h} "${1}"
+            else
+                qemu_error_no_exit=${use_error} _timed_wait_for ${h} "${1}" "${2}"
+            fi
             if [ ${QEMU_STATUS[$h]} -eq 0 ]; then
                 return
             fi
-- 
2.14.3

[Qemu-devel] [PATCH 2/2] iotests: Skip 181 and 201 without userfaultfd

[Max Reitz]

userfaultfd support depends on the host kernel, so it may not be
available.  If so, 181 and 201 should be skipped.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/181 | 13 +++++++++++++
 tests/qemu-iotests/201 | 13 +++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/tests/qemu-iotests/181 b/tests/qemu-iotests/181
index 5e767c6195..e02979378d 100755
--- a/tests/qemu-iotests/181
+++ b/tests/qemu-iotests/181
@@ -96,6 +96,19 @@ echo
 # Enable postcopy-ram capability both on source and destination
 silent=yes
 _send_qemu_cmd $dest 'migrate_set_capability postcopy-ram on' "(qemu)"
+
+qemu_error_no_exit=yes success_or_failure=yes \
+    _send_qemu_cmd $dest '' "(qemu)" "Postcopy is not supported"
+if [ ${QEMU_STATUS[$dest]} -lt 0 ]; then
+    _send_qemu_cmd $dest '' "(qemu)"
+
+    _send_qemu_cmd $src 'quit' ""
+    _send_qemu_cmd $dest 'quit' ""
+    wait=1 _cleanup_qemu
+
+    _notrun 'Postcopy is not supported'
+fi
+
 _send_qemu_cmd $src 'migrate_set_speed 4k' "(qemu)"
 _send_qemu_cmd $src 'migrate_set_capability postcopy-ram on' "(qemu)"
 _send_qemu_cmd $src "migrate -d unix:${MIG_SOCKET}" "(qemu)"
diff --git a/tests/qemu-iotests/201 b/tests/qemu-iotests/201
index 11f640f5df..c1a1e00077 100755
--- a/tests/qemu-iotests/201
+++ b/tests/qemu-iotests/201
@@ -82,6 +82,19 @@ echo
 
 silent=yes
 _send_qemu_cmd $dest 'migrate_set_capability postcopy-ram on' "(qemu)"
+
+qemu_error_no_exit=yes success_or_failure=yes \
+    _send_qemu_cmd $dest '' "(qemu)" "Postcopy is not supported"
+if [ ${QEMU_STATUS[$dest]} -lt 0 ]; then
+    _send_qemu_cmd $dest '' "(qemu)"
+
+    _send_qemu_cmd $src 'quit' ""
+    _send_qemu_cmd $dest 'quit' ""
+    wait=1 _cleanup_qemu
+
+    _notrun 'Postcopy is not supported'
+fi
+
 _send_qemu_cmd $src 'migrate_set_capability postcopy-ram on' "(qemu)"
 _send_qemu_cmd $src "migrate -d unix:${MIG_SOCKET}" "(qemu)"
 
-- 
2.14.3

Re: [Qemu-devel] [PATCH 0/2 for-2.12?] iotests: Skip 181 and 201 without userfaultfd

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 961 bytes --]

On 04/06/2018 10:17 AM, Max Reitz wrote:
> My non-Fedora testing system does not have a kernel with userfaultfd
> support which causes 181 and 201 to fail.  That is annoying.  This
> series makes those tests recognize the issue and convert it into a
> _notrun.

As this is just testsuite fixes, is this worth including in 2.12?  Then
again, as we are now building for -rc3, missing the release merely means
tests fail, and not a broken binary.

> 
> 
> Max Reitz (2):
>   iotests: Add failure matching to common.qemu
>   iotests: Skip 181 and 201 without userfaultfd
> 
>  tests/qemu-iotests/181         | 13 ++++++++++
>  tests/qemu-iotests/201         | 13 ++++++++++
>  tests/qemu-iotests/common.qemu | 58 +++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 77 insertions(+), 7 deletions(-)
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


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

Re: [Qemu-devel] [PATCH 0/2 for-2.12?] iotests: Skip 181 and 201 without userfaultfd

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1083 bytes --]

On 2018-04-06 17:30, Eric Blake wrote:
> On 04/06/2018 10:17 AM, Max Reitz wrote:
>> My non-Fedora testing system does not have a kernel with userfaultfd
>> support which causes 181 and 201 to fail.  That is annoying.  This
>> series makes those tests recognize the issue and convert it into a
>> _notrun.
> 
> As this is just testsuite fixes, is this worth including in 2.12?  Then
> again, as we are now building for -rc3, missing the release merely means
> tests fail, and not a broken binary.

I usually merge test patches independently of freeze, but on the other
hand I prefer to only take the really necessary things for the later
RCs.  Sooo... I haven't decided yet. :-)

Max

>> Max Reitz (2):
>>   iotests: Add failure matching to common.qemu
>>   iotests: Skip 181 and 201 without userfaultfd
>>
>>  tests/qemu-iotests/181         | 13 ++++++++++
>>  tests/qemu-iotests/201         | 13 ++++++++++
>>  tests/qemu-iotests/common.qemu | 58 +++++++++++++++++++++++++++++++++++++-----
>>  3 files changed, 77 insertions(+), 7 deletions(-)
>>
> 



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

Re: [Qemu-devel] [PATCH 0/2] iotests: Skip 181 and 201 without userfaultfd

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]

On 2018-04-06 17:17, Max Reitz wrote:
> My non-Fedora testing system does not have a kernel with userfaultfd
> support which causes 181 and 201 to fail.  That is annoying.  This
> series makes those tests recognize the issue and convert it into a
> _notrun.
> 
> 
> Max Reitz (2):
>   iotests: Add failure matching to common.qemu
>   iotests: Skip 181 and 201 without userfaultfd
> 
>  tests/qemu-iotests/181         | 13 ++++++++++
>  tests/qemu-iotests/201         | 13 ++++++++++
>  tests/qemu-iotests/common.qemu | 58 +++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 77 insertions(+), 7 deletions(-)

Ping (considering these are test suite fixes, I'd merge them without
review, too, though.)

Max


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

Re: [Qemu-devel] [PATCH 0/2] iotests: Skip 181 and 201 without userfaultfd

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

On 2018-04-06 17:17, Max Reitz wrote:
> My non-Fedora testing system does not have a kernel with userfaultfd
> support which causes 181 and 201 to fail.  That is annoying.  This
> series makes those tests recognize the issue and convert it into a
> _notrun.
> 
> 
> Max Reitz (2):
>   iotests: Add failure matching to common.qemu
>   iotests: Skip 181 and 201 without userfaultfd
> 
>  tests/qemu-iotests/181         | 13 ++++++++++
>  tests/qemu-iotests/201         | 13 ++++++++++
>  tests/qemu-iotests/common.qemu | 58 +++++++++++++++++++++++++++++++++++++-----
>  3 files changed, 77 insertions(+), 7 deletions(-)

Applied to my block-next branch.

Max


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