[PATCH 0/4] tests/qtest: Check for devices before using them

[PATCH 0/4] tests/qtest: Check for devices before using them

[Thomas Huth]

Here are some more patches that are required for running the qtests
with builds that have been configured with "--without-default-devices".
We need to check whether the required devices are really available
in the binaries before we can use them, otherwise the tests will
fail.

Thomas Huth (4):
  tests/qtest/usb-hcd-uhci-test: Check whether "usb-storage" is
    available
  tests/qtest: Check for virtio-blk before using -cdrom with the arm
    virt machine
  tests/qtest/rtl8139-test: Check whether the rtl8139 device is
    available
  tests/qtest/usb-hcd-ehci-test: Check for EHCI and UHCI HCDs before
    using them

 tests/qtest/bios-tables-test.c  | 2 +-
 tests/qtest/cdrom-test.c        | 6 +++++-
 tests/qtest/rtl8139-test.c      | 4 ++++
 tests/qtest/usb-hcd-ehci-test.c | 5 +++++
 tests/qtest/usb-hcd-uhci-test.c | 4 +++-
 5 files changed, 18 insertions(+), 3 deletions(-)

-- 
2.31.1

[PATCH 1/4] tests/qtest/usb-hcd-uhci-test: Check whether "usb-storage" is available

[Thomas Huth]

The "usb-storage" device might not have been compiled into the binary
(e.g. when compiling with "--without-default-devices"), so we have to
check first before using it.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/usb-hcd-uhci-test.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/usb-hcd-uhci-test.c b/tests/qtest/usb-hcd-uhci-test.c
index 84ac2f3c1a..28751f53da 100644
--- a/tests/qtest/usb-hcd-uhci-test.c
+++ b/tests/qtest/usb-hcd-uhci-test.c
@@ -74,7 +74,9 @@ int main(int argc, char **argv)
     qtest_add_func("/uhci/pci/init", test_uhci_init);
     qtest_add_func("/uhci/pci/port1", test_port_1);
     qtest_add_func("/uhci/pci/hotplug", test_uhci_hotplug);
-    qtest_add_func("/uhci/pci/hotplug/usb-storage", test_usb_storage_hotplug);
+    if (qtest_has_device("usb-storage")) {
+        qtest_add_func("/uhci/pci/hotplug/usb-storage", test_usb_storage_hotplug);
+    }
 
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         qs = qtest_pc_boot("%s", cmd);
-- 
2.31.1

Re: [PATCH 1/4] tests/qtest/usb-hcd-uhci-test: Check whether "usb-storage" is available

[Ani Sinha]

> On 25-May-2023, at 1:40 PM, Thomas Huth <thuth@redhat.com> wrote:
> 
> The "usb-storage" device might not have been compiled into the binary
> (e.g. when compiling with "--without-default-devices"), so we have to
> check first before using it.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Ani Sinha <anisinha@redhat.com>

> ---
> tests/qtest/usb-hcd-uhci-test.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/usb-hcd-uhci-test.c b/tests/qtest/usb-hcd-uhci-test.c
> index 84ac2f3c1a..28751f53da 100644
> --- a/tests/qtest/usb-hcd-uhci-test.c
> +++ b/tests/qtest/usb-hcd-uhci-test.c
> @@ -74,7 +74,9 @@ int main(int argc, char **argv)
>     qtest_add_func("/uhci/pci/init", test_uhci_init);
>     qtest_add_func("/uhci/pci/port1", test_port_1);
>     qtest_add_func("/uhci/pci/hotplug", test_uhci_hotplug);
> -    qtest_add_func("/uhci/pci/hotplug/usb-storage", test_usb_storage_hotplug);
> +    if (qtest_has_device("usb-storage")) {
> +        qtest_add_func("/uhci/pci/hotplug/usb-storage", test_usb_storage_hotplug);
> +    }
> 
>     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>         qs = qtest_pc_boot("%s", cmd);
> -- 
> 2.31.1
> 

[PATCH 2/4] tests/qtest: Check for virtio-blk before using -cdrom with the arm virt machine

[Thomas Huth]

The arm "virt" machine needs "virtio-blk-pci" for devices that get attached
via the "-cdrom" option. Since this is an optional device that might not
be available in the binary, we should check for the availability of this
device first before using it.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/bios-tables-test.c | 2 +-
 tests/qtest/cdrom-test.c       | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 159e4edb8f..ed1c69cf01 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -2164,7 +2164,7 @@ int main(int argc, char *argv[])
             }
         }
     } else if (strcmp(arch, "aarch64") == 0) {
-        if (has_tcg) {
+        if (has_tcg && qtest_has_device("virtio-blk-pci")) {
             qtest_add_func("acpi/virt", test_acpi_virt_tcg);
             qtest_add_func("acpi/virt/acpihmatvirt",
                             test_acpi_virt_tcg_acpi_hmat);
diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
index d1cc375849..f2a8d91929 100644
--- a/tests/qtest/cdrom-test.c
+++ b/tests/qtest/cdrom-test.c
@@ -264,9 +264,13 @@ int main(int argc, char **argv)
         const char *armmachines[] = {
             "realview-eb", "realview-eb-mpcore", "realview-pb-a8",
             "realview-pbx-a9", "versatileab", "versatilepb", "vexpress-a15",
-            "vexpress-a9", "virt", NULL
+            "vexpress-a9", NULL
         };
         add_cdrom_param_tests(armmachines);
+        if (qtest_has_device("virtio-blk-pci")) {
+            const char *virtmachine[] = { "virt", NULL };
+            add_cdrom_param_tests(virtmachine);
+        }
     } else {
         const char *nonemachine[] = { "none", NULL };
         add_cdrom_param_tests(nonemachine);
-- 
2.31.1

Re: [PATCH 2/4] tests/qtest: Check for virtio-blk before using -cdrom with the arm virt machine

[Fabiano Rosas]

Thomas Huth <thuth@redhat.com> writes:

> The arm "virt" machine needs "virtio-blk-pci" for devices that get attached
> via the "-cdrom" option. Since this is an optional device that might not
> be available in the binary, we should check for the availability of this
> device first before using it.
>

We're still producing a binary that exposes runtime options that may not
work depending on the state of seemingly unrelated config options. And
barfs QEMU implementation details at the user:

qemu-system-aarch64: -cdrom foo.qcow2: 'virtio-blk' (alias
'virtio-blk-pci') is not a valid device model name

Anyway, this patch is good so:

Reviewed-by: Fabiano Rosas <farosas@suse.de>

[PATCH 3/4] tests/qtest/rtl8139-test: Check whether the rtl8139 device is available

[Thomas Huth]

Though we are already using CONFIG_RTL8139_PCI in the meson.build file
for testing whether the rtl8139 device is available or not, this is not
enough: The CONFIG switch might have been selected by another target
(e.g. the mips fuloong2e machine has the rtl8139 chip soldered on the
board), so CONFIG_RTL8139_PCI ends up in config_all_devices and the
test then gets executed on x86. We need an additional run-time check
to be on the safe side to make this test also work when configure has
been run with "--without-default-devices".

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/rtl8139-test.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/qtest/rtl8139-test.c b/tests/qtest/rtl8139-test.c
index 4bd240e9ee..4dc0a0d22e 100644
--- a/tests/qtest/rtl8139-test.c
+++ b/tests/qtest/rtl8139-test.c
@@ -209,6 +209,10 @@ int main(int argc, char **argv)
 
     g_test_init(&argc, &argv, NULL);
 
+    if (!qtest_has_device("rtl8139")) {
+        return 0;
+    }
+
     qtest_start("-device rtl8139");
 
     qtest_add_func("/rtl8139/nop", nop);
-- 
2.31.1

Re: [PATCH 3/4] tests/qtest/rtl8139-test: Check whether the rtl8139 device is available

[Fabiano Rosas]

Thomas Huth <thuth@redhat.com> writes:

> Though we are already using CONFIG_RTL8139_PCI in the meson.build file
> for testing whether the rtl8139 device is available or not, this is not
> enough: The CONFIG switch might have been selected by another target
> (e.g. the mips fuloong2e machine has the rtl8139 chip soldered on the
> board), so CONFIG_RTL8139_PCI ends up in config_all_devices and the
> test then gets executed on x86. We need an additional run-time check
> to be on the safe side to make this test also work when configure has
> been run with "--without-default-devices".
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

[PATCH 4/4] tests/qtest/usb-hcd-ehci-test: Check for EHCI and UHCI HCDs before using them

[Thomas Huth]

The devices might not be available in the binary (e.g. when compiling
with "--without-default-devices"), so we have to check before we can
use them.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/usb-hcd-ehci-test.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/qtest/usb-hcd-ehci-test.c b/tests/qtest/usb-hcd-ehci-test.c
index c51e8bb223..87e37cdd7c 100644
--- a/tests/qtest/usb-hcd-ehci-test.c
+++ b/tests/qtest/usb-hcd-ehci-test.c
@@ -149,6 +149,11 @@ int main(int argc, char **argv)
 
     g_test_init(&argc, &argv, NULL);
 
+    if (!qtest_has_device("ich9-usb-ehci1") ||
+        !qtest_has_device("ich9-usb-uhci1")) {
+        return 0;
+    }
+
     qtest_add_func("/ehci/pci/uhci-port-1", pci_uhci_port_1);
     qtest_add_func("/ehci/pci/ehci-port-1", pci_ehci_port_1);
     qtest_add_func("/ehci/pci/ehci-config", pci_ehci_config);
-- 
2.31.1

Re: [PATCH 4/4] tests/qtest/usb-hcd-ehci-test: Check for EHCI and UHCI HCDs before using them

[Ani Sinha]

> On 25-May-2023, at 1:40 PM, Thomas Huth <thuth@redhat.com> wrote:
> 
> The devices might not be available in the binary (e.g. when compiling
> with "--without-default-devices"), so we have to check before we can
> use them.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Ani Sinha <anisinha@redhat.com>

> ---
> tests/qtest/usb-hcd-ehci-test.c | 5 +++++
> 1 file changed, 5 insertions(+)
> 
> diff --git a/tests/qtest/usb-hcd-ehci-test.c b/tests/qtest/usb-hcd-ehci-test.c
> index c51e8bb223..87e37cdd7c 100644
> --- a/tests/qtest/usb-hcd-ehci-test.c
> +++ b/tests/qtest/usb-hcd-ehci-test.c
> @@ -149,6 +149,11 @@ int main(int argc, char **argv)
> 
>     g_test_init(&argc, &argv, NULL);
> 
> +    if (!qtest_has_device("ich9-usb-ehci1") ||
> +        !qtest_has_device("ich9-usb-uhci1")) {
> +        return 0;
> +    }
> +
>     qtest_add_func("/ehci/pci/uhci-port-1", pci_uhci_port_1);
>     qtest_add_func("/ehci/pci/ehci-port-1", pci_ehci_port_1);
>     qtest_add_func("/ehci/pci/ehci-config", pci_ehci_config);
> -- 
> 2.31.1
> 

Re: [PATCH 0/4] tests/qtest: Check for devices before using them

[Michael S. Tsirkin]

On Thu, May 25, 2023 at 10:10:12AM +0200, Thomas Huth wrote:
> Here are some more patches that are required for running the qtests
> with builds that have been configured with "--without-default-devices".
> We need to check whether the required devices are really available
> in the binaries before we can use them, otherwise the tests will
> fail.
> 
> Thomas Huth (4):
>   tests/qtest/usb-hcd-uhci-test: Check whether "usb-storage" is
>     available
>   tests/qtest: Check for virtio-blk before using -cdrom with the arm
>     virt machine
>   tests/qtest/rtl8139-test: Check whether the rtl8139 device is
>     available
>   tests/qtest/usb-hcd-ehci-test: Check for EHCI and UHCI HCDs before
>     using them
> 
>  tests/qtest/bios-tables-test.c  | 2 +-
>  tests/qtest/cdrom-test.c        | 6 +++++-
>  tests/qtest/rtl8139-test.c      | 4 ++++
>  tests/qtest/usb-hcd-ehci-test.c | 5 +++++
>  tests/qtest/usb-hcd-uhci-test.c | 4 +++-
>  5 files changed, 18 insertions(+), 3 deletions(-)

I am worried that if an uninitentional change disables some devices
by default our CI will no longer catch this.
Any way to address this? E.g. maybe introduce a "for CI" or
"test all" configure flag and then make test fail if something
hasn't been configured?

> -- 
> 2.31.1