These tests rely on a default NIC to be available. Skip them if we
used the "--without-default-devices" configure option.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/meson.build | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 48cd35b5b2..8fec3103b5 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -35,9 +35,9 @@ qtests_cxl = \
   (config_all_devices.has_key('CONFIG_CXL') ? ['cxl-test'] : [])
 
 qtests_filter = \
-  (slirp.found() ? ['test-netfilter'] : []) + \
-  (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) + \
-  (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : [])
+  (get_option('default_devices') and slirp.found() ? ['test-netfilter'] : []) + \
+  (get_option('default_devices') and config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) + \
+  (get_option('default_devices') and config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : [])
 
 qtests_i386 = \
   (slirp.found() ? ['pxe-test'] : []) + \
@@ -221,9 +221,7 @@ qtests_aarch64 = \
    'migration-test']
 
 qtests_s390x = \
-  (slirp.found() ? ['pxe-test', 'test-netfilter'] : []) +                 \
-  (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) +                         \
-  (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : []) +                     \
+  qtests_filter + \
   ['boot-serial-test',
    'drive_del-test',
    'device-plug-test',
-- 
2.31.1

Il ven 12 mag 2023, 14:41 Thomas Huth <thuth@redhat.com> ha scritto:

> These tests rely on a default NIC to be available. Skip them if we
> used the "--without-default-devices" configure option.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
>

This is the only patch I have some qualms about, because it reduces
coverage in legitimate setups where the default NIC _is_ included in the
binary.

Still a lot better than before, but please add a FIXME here. We can perhaps
try to use QMP to check if the machines have a usable default NIC, and if
not skip the test, but this should not block the bulk of your work from
being merged.

So, apart from this issue, series

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

(Since I have only skimmed the contents of the individual patches but liked
them enough—or disliked for this one...—to reply already).

Paolo


---
>  tests/qtest/meson.build | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 48cd35b5b2..8fec3103b5 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -35,9 +35,9 @@ qtests_cxl = \
>    (config_all_devices.has_key('CONFIG_CXL') ? ['cxl-test'] : [])
>
>  qtests_filter = \
> -  (slirp.found() ? ['test-netfilter'] : []) + \
> -  (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) + \
> -  (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : [])
> +  (get_option('default_devices') and slirp.found() ? ['test-netfilter'] :
> []) + \
> +  (get_option('default_devices') and config_host.has_key('CONFIG_POSIX')
> ? ['test-filter-mirror'] : []) + \
> +  (get_option('default_devices') and config_host.has_key('CONFIG_POSIX')
> ? ['test-filter-redirector'] : [])
>
>  qtests_i386 = \
>    (slirp.found() ? ['pxe-test'] : []) + \
> @@ -221,9 +221,7 @@ qtests_aarch64 = \
>     'migration-test']
>
>  qtests_s390x = \
> -  (slirp.found() ? ['pxe-test', 'test-netfilter'] : []) +
>  \
> -  (config_host.has_key('CONFIG_POSIX') ? ['test-filter-mirror'] : []) +
>                        \
> -  (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : [])
> +                     \
> +  qtests_filter + \
>    ['boot-serial-test',
>     'drive_del-test',
>     'device-plug-test',
> --
> 2.31.1
>
>

On 14/05/2023 22.31, Paolo Bonzini wrote:
> 
> 
> Il ven 12 mag 2023, 14:41 Thomas Huth <thuth@redhat.com 
> <mailto:thuth@redhat.com>> ha scritto:
> 
>     These tests rely on a default NIC to be available. Skip them if we
>     used the "--without-default-devices" configure option.
> 
>     Signed-off-by: Thomas Huth <thuth@redhat.com <mailto:thuth@redhat.com>>
> 
> 
> This is the only patch I have some qualms about, because it reduces coverage 
> in legitimate setups where the default NIC _is_ included in the binary.
> 
> Still a lot better than before, but please add a FIXME here.

Ok, will do.

> We can perhaps 
> try to use QMP to check if the machines have a usable default NIC, and if 
> not skip the test, but this should not block the bulk of your work from 
> being merged.

I'm currently not aware of a way to query the default NIC via QMP, so this 
might need some extensions there ... but I'll keep it in mind.

The alternative would be to hard-code the default NICs for each architecture 
in the tests again and check for the availability with that hard-coded list 
... but that's also ugly, I think.

> So, apart from this issue, series
> 
> Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Thanks!

  Thomas

On Mon, May 15, 2023 at 11:46 AM Thomas Huth <thuth@redhat.com> wrote:
> On 14/05/2023 22.31, Paolo Bonzini wrote:
> > We can perhaps
> > try to use QMP to check if the machines have a usable default NIC, and if
> > not skip the test, but this should not block the bulk of your work from
> > being merged.
>
> I'm currently not aware of a way to query the default NIC via QMP, so this
> might need some extensions there ... but I'll keep it in mind.

Yes, it would need some changes to MachineInfo (in qapi/machine.json).

Paolo