check-block has its own test harness, unlike every other test.  If
we capture its output, as is in general nicer to do without V=1,
there will be no sign of progress.  So for lack of a better option
just move the invocation of the test back to Makefile rules.

As a side effect, this will also fix "make check" in --disable-tools
builds, as they were trying to run qemu-iotests without having
made qemu-img before.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 meson.build                    |  1 -
 tests/Makefile.include         | 15 ++++++++++++---
 tests/qemu-iotests/meson.build |  4 ----
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/meson.build b/meson.build
index 5aaa364730..85f3c01b98 100644
--- a/meson.build
+++ b/meson.build
@@ -1097,7 +1097,6 @@ if have_tools
              dependencies: [block, qemuutil], install: true)
   qemu_nbd = executable('qemu-nbd', files('qemu-nbd.c'),
                dependencies: [block, qemuutil], install: true)
-  qemu_block_tools += [qemu_img, qemu_io, qemu_nbd]
 
   subdir('storage-daemon')
   subdir('contrib/rdmacm-mux')
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 9ac8f5b86a..08301f5bc9 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -468,7 +468,6 @@ check-tcg: $(RUN_TCG_TARGET_RULES)
 .PHONY: clean-tcg
 clean-tcg: $(CLEAN_TCG_TARGET_RULES)
 
-
 # Python venv for running tests
 
 .PHONY: check-venv check-acceptance
@@ -523,8 +522,18 @@ check-acceptance: check-venv $(TESTS_RESULTS_DIR) get-vm-images
 # Consolidated targets
 
 .PHONY: check-block check-unit check check-clean get-vm-images
-check-block:
-check-build: build-unit
+check:
+
+ifeq ($(CONFIG_TOOLS)$(CONFIG_POSIX),yy)
+QEMU_IOTESTS_HELPERS-$(CONFIG_LINUX) = tests/qemu-iotests/socket_scm_helper$(EXESUF)
+check: check-block
+check-block: $(SRC_PATH)/tests/check-block.sh qemu-img$(EXESUF) \
+		qemu-io$(EXESUF) qemu-nbd$(EXESUF) $(QEMU_IOTESTS_HELPERS-y) \
+		$(patsubst %-softmmu,qemu-system-%,$(filter %-softmmu,$(TARGET_DIRS)))
+	@$<
+endif
+
+check-build: build-unit $(QEMU_IOTESTS_HELPERS-y)
 
 check-clean:
 	rm -rf $(check-unit-y) tests/*.o tests/*/*.o $(QEMU_IOTESTS_HELPERS-y)
diff --git a/tests/qemu-iotests/meson.build b/tests/qemu-iotests/meson.build
index 3de09fb8fa..60470936b4 100644
--- a/tests/qemu-iotests/meson.build
+++ b/tests/qemu-iotests/meson.build
@@ -4,7 +4,3 @@ if 'CONFIG_LINUX' in config_host
 else
     socket_scm_helper = []
 endif
-test('qemu-iotests', sh, args: [files('../check-block.sh')],
-     depends: [qemu_block_tools, emulators, socket_scm_helper],
-     suite: 'block', timeout: 10000)
-
-- 
2.26.2

On 06.09.20 19:53, Paolo Bonzini wrote:
> check-block has its own test harness, unlike every other test.  If
> we capture its output, as is in general nicer to do without V=1,
> there will be no sign of progress.  So for lack of a better option
> just move the invocation of the test back to Makefile rules.
> 
> As a side effect, this will also fix "make check" in --disable-tools
> builds, as they were trying to run qemu-iotests without having
> made qemu-img before.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  meson.build                    |  1 -
>  tests/Makefile.include         | 15 ++++++++++++---
>  tests/qemu-iotests/meson.build |  4 ----
>  3 files changed, 12 insertions(+), 8 deletions(-)

I’m not entirely sure why (or I would’ve sent a patch myself), but this
breaks all iotests using the socket_scm_helper program unless one runs
make check-block before (or make tests/qemu-iotests/socket_scm_helper
specifically).

It seems like the socket_scm_helper is now only built as a dependency of
check-block, instead of all the time.  That’s a bit of a shame.
(The obvious workaround of course is to specifically build the
socket_scm_helper, but that doesn’t seem right.)

As a reproducer:

[delete build directory, or most importantly the socket_scm_helper]
[configure]
[make]
$ cd tests/qemu-iotests
$ ./check -nbd 147
[...]
+qemu.machine.QEMUMachineError: socket_scm_helper does not exist
[...]

Max

On 11.09.20 12:58, Max Reitz wrote:
> On 06.09.20 19:53, Paolo Bonzini wrote:
>> check-block has its own test harness, unlike every other test.  If
>> we capture its output, as is in general nicer to do without V=1,
>> there will be no sign of progress.  So for lack of a better option
>> just move the invocation of the test back to Makefile rules.
>>
>> As a side effect, this will also fix "make check" in --disable-tools
>> builds, as they were trying to run qemu-iotests without having
>> made qemu-img before.
>>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
>>  meson.build                    |  1 -
>>  tests/Makefile.include         | 15 ++++++++++++---
>>  tests/qemu-iotests/meson.build |  4 ----
>>  3 files changed, 12 insertions(+), 8 deletions(-)
> 
> I’m not entirely sure why (or I would’ve sent a patch myself)

On closer inspection it seems like it’s because of the
“build_by_default: false”, which seems like a rather conscious decision.
 Was I only lucky that the socket_scm_helper was built by default so
far?  Should I have explicitly built it all this time?

Max

On 11/09/20 13:05, Max Reitz wrote:
> On closer inspection it seems like it’s because of the
> “build_by_default: false”, which seems like a rather conscious decision.
>  Was I only lucky that the socket_scm_helper was built by default so
> far?  Should I have explicitly built it all this time?

Yes, you were lucky but that's not a reason not to change it.

The problem with touching a whole build system is that you are not going
to test everybody's usecase, and yours is definitely reasonable and not
"M-x butterfly" territory.

Paolo

On 11/09/20 12:58, Max Reitz wrote:
> It seems like the socket_scm_helper is now only built as a dependency of
> check-block, instead of all the time.  That’s a bit of a shame.
> (The obvious workaround of course is to specifically build the
> socket_scm_helper, but that doesn’t seem right.)

Or just remove the build_by_default: false here:

    socket_scm_helper = executable('socket_scm_helper', 'socket_scm_helper.c',
                                    build_by_default: false)


I guess now is a good time to decide which executables to build by
default (static_libraries should never be built by default) and
document it in docs/devel/build-system.rst.

Right now, the only executables that aren't built by default are:

- rdmacm-mux and vhost-user-blk because they're broken

- gen-features because it's built anyway for s390 targets

- vhost-user-bridge, and that probably should be changed

- socket_scm_helper, which could/should be changed too

- fptest, not sure why that works at all O:-)  Tests are built by
default (and they trigger coverity quite a bit).  We will be
able to fix that, and at the same time respect the "tests(depends: ...)"
argument instead of just having "check: all", when meson 0.56.0
comes out.

Paolo