[Qemu-devel] [PATCH 0/2] qemu-iotests: add missing make rule for socket_scm_helper

[Qemu-devel] [PATCH 0/2] qemu-iotests: add missing make rule for socket_scm_helper

[Philippe Mathieu-Daudé]

Hi,

Apparently block-tests are only run on patchew CI, not Travis, so this went
unnoticed.

Now ./check does check for this helper, and errors are easier to figure out.

Regards,

Phil.

Philippe Mathieu-Daudé (2):
  qemu-iotests: add missing rule to build socket_scm_helper
  qemu-iotests: check socket_scm_helper is available

 tests/Makefile.include   | 2 +-
 tests/qemu-iotests/check | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

-- 
2.15.1

[Qemu-devel] [PATCH 1/2] qemu-iotests: add missing rule to build socket_scm_helper

[Philippe Mathieu-Daudé]

'make check-block' uses it on Linux.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/Makefile.include | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index b4bcc872f2..fa5e9597f1 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -919,7 +919,7 @@ check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-tests/qap
 check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
 check-unit: $(patsubst %,check-%, $(check-unit-y))
 check-speed: $(patsubst %,check-%, $(check-speed-y))
-check-block: $(patsubst %,check-%, $(check-block-y))
+check-block: $(patsubst %,check-%, $(check-block-y)) QEMU_IOTESTS_HELPERS-$(CONFIG_LINUX)
 check: check-qapi-schema check-unit check-qtest
 check-clean:
 	$(MAKE) -C tests/tcg clean
-- 
2.15.1

[Qemu-devel] [PATCH 2/2] qemu-iotests: check socket_scm_helper is available

[Philippe Mathieu-Daudé]

This now throws a more helpful error message when not found:

  tests/qemu-iotests$ ./check 045
  QEMU          -- "/qemu/build/x86_64-softmmu/qemu-system-x86_64" -nodefaults -machine accel=qtest
  QEMU_IMG      -- "/qemu/build/qemu-img"
  QEMU_IO       -- "/qemu/build/qemu-io"  --cache writeback -f raw
  QEMU_NBD      -- "/qemu/build/qemu-nbd"
  TEST_DIR      -- /qemu/build/tests/qemu-iotests/scratch
  SOCKET_SCM_HELPER --

  045 1s ... [failed, exit status 1] - output mismatch (see 045.out.bad)
  -...........
  +......EE.EE
  +======================================================================
  +ERROR: test_add_fd (__main__.TestSCMFd)
  +----------------------------------------------------------------------
  +Traceback (most recent call last):
  +  File "045", line 147, in test_add_fd
  +    self._send_fd_by_SCM()
  +  File "045", line 143, in _send_fd_by_SCM
  +    ret = self.vm.send_fd_scm(image0)
  +  File "/source/qemu/tests/qemu-iotests/../../scripts/qemu.py", line 125, in send_fd_scm
  +    self._socket_scm_helper)
  +QEMUMachineError: socket_scm_helper does not exist
  +
   ----------------------------------------------------------------------
  Failures: 045
  Failed 1 of 1 tests

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 tests/qemu-iotests/check | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index e6b6ff7a04..a2c0bab1cc 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -597,6 +597,8 @@ fi
 if [ -x "$build_iotests/socket_scm_helper" ]
 then
     export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
+else
+    _init_error "socket_scm_helper not found"
 fi
 
 default_machine=$($QEMU_PROG -machine help | sed -n '/(default)/ s/ .*//p')
-- 
2.15.1

Re: [Qemu-devel] [PATCH 0/2] qemu-iotests: add missing make rule for socket_scm_helper

[Paolo Bonzini]

On 19/12/2017 17:37, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> Apparently block-tests are only run on patchew CI, not Travis, so this went
> unnoticed.
> 
> Now ./check does check for this helper, and errors are easier to figure out.
> 
> Regards,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (2):
>   qemu-iotests: add missing rule to build socket_scm_helper
>   qemu-iotests: check socket_scm_helper is available
> 
>  tests/Makefile.include   | 2 +-
>  tests/qemu-iotests/check | 2 ++
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH 1/2] qemu-iotests: add missing rule to build socket_scm_helper

[Max Reitz]

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

On 2017-12-19 17:37, Philippe Mathieu-Daudé wrote:
> 'make check-block' uses it on Linux.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/Makefile.include | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index b4bcc872f2..fa5e9597f1 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -919,7 +919,7 @@ check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y)) check-tests/qap
>  check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
>  check-unit: $(patsubst %,check-%, $(check-unit-y))
>  check-speed: $(patsubst %,check-%, $(check-speed-y))
> -check-block: $(patsubst %,check-%, $(check-block-y))
> +check-block: $(patsubst %,check-%, $(check-block-y)) QEMU_IOTESTS_HELPERS-$(CONFIG_LINUX)
>  check: check-qapi-schema check-unit check-qtest
>  check-clean:
>  	$(MAKE) -C tests/tcg clean

First, shouldn't it be $(QEMU_IOTESTS_HELPERS-y)?

Secondly, it seems that is already a dependency of
check-tests/qemu-iotests-quick.sh, which is basically what
$(check-block-y) is.  So even without that dependency here,
socket_scm_helper is built when I run make check-block and it's missing.

Max


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

Re: [Qemu-devel] [PATCH 2/2] qemu-iotests: check socket_scm_helper is available

[Max Reitz]

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

On 2017-12-19 17:37, Philippe Mathieu-Daudé wrote:
> This now throws a more helpful error message when not found:
> 
>   tests/qemu-iotests$ ./check 045
>   QEMU          -- "/qemu/build/x86_64-softmmu/qemu-system-x86_64" -nodefaults -machine accel=qtest
>   QEMU_IMG      -- "/qemu/build/qemu-img"
>   QEMU_IO       -- "/qemu/build/qemu-io"  --cache writeback -f raw
>   QEMU_NBD      -- "/qemu/build/qemu-nbd"
>   TEST_DIR      -- /qemu/build/tests/qemu-iotests/scratch
>   SOCKET_SCM_HELPER --
> 
>   045 1s ... [failed, exit status 1] - output mismatch (see 045.out.bad)
>   -...........
>   +......EE.EE
>   +======================================================================
>   +ERROR: test_add_fd (__main__.TestSCMFd)
>   +----------------------------------------------------------------------
>   +Traceback (most recent call last):
>   +  File "045", line 147, in test_add_fd
>   +    self._send_fd_by_SCM()
>   +  File "045", line 143, in _send_fd_by_SCM
>   +    ret = self.vm.send_fd_scm(image0)
>   +  File "/source/qemu/tests/qemu-iotests/../../scripts/qemu.py", line 125, in send_fd_scm
>   +    self._socket_scm_helper)
>   +QEMUMachineError: socket_scm_helper does not exist
>   +
>    ----------------------------------------------------------------------
>   Failures: 045
>   Failed 1 of 1 tests
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  tests/qemu-iotests/check | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index e6b6ff7a04..a2c0bab1cc 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -597,6 +597,8 @@ fi
>  if [ -x "$build_iotests/socket_scm_helper" ]
>  then
>      export SOCKET_SCM_HELPER="$build_iotests/socket_scm_helper"
> +else
> +    _init_error "socket_scm_helper not found"
>  fi

Hmm... I'm not so sure about this.  Now if you don't have
socket_scm_helper for some reason and you want to run a test that does
not need it, you cannot do that anymore (but currently you can).

OTOH, I'm not sure how important that use case is.  Usually, you can run
the iotests only if you can build the socket_scm_helper, too.

Also, the error message you mention in the commit message is exactly
what I get without this patch.  Your patch just changes it to
“socket_scm_helper not found”.  So even if nobody really would care
about the “regression”, I don't quite see the point of this change...

Max


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