[PATCH 0/4] tests/qtest: Introduce qtest_get_base_arch() and qtest_get_arch_bits()

[PATCH 0/4] tests/qtest: Introduce qtest_get_base_arch() and qtest_get_arch_bits()

[Philippe Mathieu-Daudé]

When unifying some 32/64 architectures as a single binary
(for example i386 & x86_64 -> x86), some qtests fail because
we lose some information (the arch bits).

This series introduce the tuple qtest_get_base_arch() /
qtest_get_arch_bits() to allow the tests to keep passing.

Eventually we should remove qtest_get_arch() entirely
and rename qtest_get_base_arch() -> qtest_get_arch().

So far these changes are sufficient to keep me progressing,
so posting up to this point.

Philippe Mathieu-Daudé (4):
  tests/libqtest: Introduce qtest_get_arch_bits()
  tests/qtest: Use qtest_get_arch_bits()
  tests/libqtest: Introduce qtest_get_base_arch()
  tests/qtest: Use qtest_get_base_arch()

 tests/qtest/libqtest.h            | 15 ++++++++++
 tests/qtest/ahci-test.c           |  5 ++--
 tests/qtest/am53c974-test.c       |  4 +--
 tests/qtest/arm-cpu-features.c    | 49 +++++++++++++++----------------
 tests/qtest/bios-tables-test.c    | 22 +++++++-------
 tests/qtest/boot-sector.c         |  6 ++--
 tests/qtest/device-plug-test.c    |  6 ++--
 tests/qtest/drive_del-test.c      | 16 ++++------
 tests/qtest/erst-test.c           |  4 +--
 tests/qtest/fuzz-e1000e-test.c    |  4 +--
 tests/qtest/ivshmem-test.c        |  6 ++--
 tests/qtest/libqos/qos_external.c |  6 ++--
 tests/qtest/libqtest.c            | 49 +++++++++++++++++++++++++++++++
 tests/qtest/lpc-ich9-test.c       |  4 +--
 tests/qtest/m48t59-test.c         |  6 ++--
 tests/qtest/pxe-test.c            |  6 ++--
 tests/qtest/qos-test.c            |  6 ++--
 tests/qtest/readconfig-test.c     |  6 +---
 tests/qtest/rtas-test.c           |  4 +--
 tests/qtest/ufs-test.c            |  4 +--
 tests/qtest/usb-hcd-uhci-test.c   |  6 ++--
 tests/qtest/virtio-net-test.c     |  3 +-
 tests/qtest/virtio-rng-test.c     |  4 +--
 23 files changed, 138 insertions(+), 103 deletions(-)

-- 
2.41.0

[PATCH 1/4] tests/libqtest: Introduce qtest_get_arch_bits()

[Philippe Mathieu-Daudé]

Add a method to return the architecture bits (currently 8/32/64).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/libqtest.h |  8 ++++++++
 tests/qtest/libqtest.c | 21 +++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index e53e350e3a..1e1b42241d 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -654,6 +654,14 @@ bool qtest_big_endian(QTestState *s);
  */
 const char *qtest_get_arch(void);
 
+/**
+ * qtest_get_arch_bits:
+ *
+ * Returns: The architecture bits (a.k.a. word size) for the QEMU executable
+ * under test.
+ */
+unsigned qtest_get_arch_bits(void);
+
 /**
  * qtest_has_accel:
  * @accel_name: Accelerator name to check for.
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index b1eba71ffe..a643a6309c 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -925,6 +925,27 @@ const char *qtest_get_arch(void)
     return end + 1;
 }
 
+unsigned qtest_get_arch_bits(void)
+{
+    static const char *const arch64[] = {
+        "aarch64", "hppa", "x86_64", "loongarch64", "mips64",
+        "mips64el", "ppc64", "riscv64", "s390x", "sparc64",
+    };
+    const char *arch = qtest_get_arch();
+
+    if (!strcmp(arch, "avr")) {
+        return 8;
+    }
+
+    for (unsigned i = 0; i < ARRAY_SIZE(arch64); i++) {
+        if (!strcmp(arch, arch64[i])) {
+            return 64;
+        }
+    }
+
+    return 32;
+}
+
 bool qtest_has_accel(const char *accel_name)
 {
     if (g_str_equal(accel_name, "tcg")) {
-- 
2.41.0

Re: [PATCH 1/4] tests/libqtest: Introduce qtest_get_arch_bits()

[Thomas Huth]

On 10/10/2023 09.49, Philippe Mathieu-Daudé wrote:
> Add a method to return the architecture bits (currently 8/32/64).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/qtest/libqtest.h |  8 ++++++++
>   tests/qtest/libqtest.c | 21 +++++++++++++++++++++
>   2 files changed, 29 insertions(+)
> 
> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
> index e53e350e3a..1e1b42241d 100644
> --- a/tests/qtest/libqtest.h
> +++ b/tests/qtest/libqtest.h
> @@ -654,6 +654,14 @@ bool qtest_big_endian(QTestState *s);
>    */
>   const char *qtest_get_arch(void);
>   
> +/**
> + * qtest_get_arch_bits:
> + *
> + * Returns: The architecture bits (a.k.a. word size) for the QEMU executable
> + * under test.
> + */
> +unsigned qtest_get_arch_bits(void);
> +
>   /**
>    * qtest_has_accel:
>    * @accel_name: Accelerator name to check for.
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index b1eba71ffe..a643a6309c 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -925,6 +925,27 @@ const char *qtest_get_arch(void)
>       return end + 1;
>   }
>   
> +unsigned qtest_get_arch_bits(void)
> +{
> +    static const char *const arch64[] = {
> +        "aarch64", "hppa", "x86_64", "loongarch64", "mips64",
> +        "mips64el", "ppc64", "riscv64", "s390x", "sparc64",
> +    };
> +    const char *arch = qtest_get_arch();
> +
> +    if (!strcmp(arch, "avr")) {

Just a matter of taste, but I prefer g_str_equal(), that's easier to read.

> +        return 8;
> +    }
> +
> +    for (unsigned i = 0; i < ARRAY_SIZE(arch64); i++) {
> +        if (!strcmp(arch, arch64[i])) {
> +            return 64;
> +        }
> +    }
> +
> +    return 32;
> +}

Since this function might get called multiple times, would it make sense to 
cache the value? I.e.:

   static const unsigned bits;

   if (!bits) {
       ... do all the magic to find out the right bits ...
   }

   return bits;

?

  Thomas

Re: [PATCH 1/4] tests/libqtest: Introduce qtest_get_arch_bits()

[Philippe Mathieu-Daudé]

On 10/10/23 11:46, Thomas Huth wrote:
> On 10/10/2023 09.49, Philippe Mathieu-Daudé wrote:
>> Add a method to return the architecture bits (currently 8/32/64).
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   tests/qtest/libqtest.h |  8 ++++++++
>>   tests/qtest/libqtest.c | 21 +++++++++++++++++++++
>>   2 files changed, 29 insertions(+)

>> +unsigned qtest_get_arch_bits(void)
>> +{
>> +    static const char *const arch64[] = {
>> +        "aarch64", "hppa", "x86_64", "loongarch64", "mips64",
>> +        "mips64el", "ppc64", "riscv64", "s390x", "sparc64",
>> +    };
>> +    const char *arch = qtest_get_arch();
>> +
>> +    if (!strcmp(arch, "avr")) {
> 
> Just a matter of taste, but I prefer g_str_equal(), that's easier to read.
> 
>> +        return 8;
>> +    }
>> +
>> +    for (unsigned i = 0; i < ARRAY_SIZE(arch64); i++) {
>> +        if (!strcmp(arch, arch64[i])) {
>> +            return 64;
>> +        }
>> +    }
>> +
>> +    return 32;
>> +}
> 
> Since this function might get called multiple times, would it make sense 
> to cache the value? I.e.:
> 
>    static const unsigned bits;
> 
>    if (!bits) {
>        ... do all the magic to find out the right bits ...
>    }
> 
>    return bits;
> 
> ?

Fine by me, I'll do as suggested in v2.

[PATCH 2/4] tests/qtest: Use qtest_get_arch_bits()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/arm-cpu-features.c | 49 ++++++++++++++++------------------
 tests/qtest/bios-tables-test.c | 16 +++++------
 2 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index a8a4c668ad..b6c1b430c1 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -462,7 +462,7 @@ static void test_query_cpu_model_expansion(const void *data)
     assert_has_not_feature(qts, "max", "kvm-no-adjvtime");
     assert_has_not_feature(qts, "max", "kvm-steal-time");
 
-    if (g_str_equal(qtest_get_arch(), "aarch64")) {
+    if (qtest_get_arch_bits() == 64) {
         assert_has_feature_enabled(qts, "max", "aarch64");
         assert_has_feature_enabled(qts, "max", "sve");
         assert_has_feature_enabled(qts, "max", "sve128");
@@ -507,7 +507,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
     assert_set_feature(qts, "host", "kvm-no-adjvtime", true);
     assert_set_feature(qts, "host", "kvm-no-adjvtime", false);
 
-    if (g_str_equal(qtest_get_arch(), "aarch64")) {
+    if (qtest_get_arch_bits() == 64) {
         bool kvm_supports_steal_time;
         bool kvm_supports_sve;
         char max_name[8], name[8];
@@ -636,34 +636,31 @@ int main(int argc, char **argv)
                             NULL, test_query_cpu_model_expansion);
     }
 
-    if (!g_str_equal(qtest_get_arch(), "aarch64")) {
-        goto out;
-    }
-
-    /*
-     * For now we only run KVM specific tests with AArch64 QEMU in
-     * order avoid attempting to run an AArch32 QEMU with KVM on
-     * AArch64 hosts. That won't work and isn't easy to detect.
-     */
-    if (qtest_has_accel("kvm")) {
+    if (qtest_get_arch_bits() == 64) {
         /*
-         * This tests target the 'host' CPU type, so register it only if
-         * KVM is available.
+         * For now we only run KVM specific tests with AArch64 QEMU in
+         * order avoid attempting to run an AArch32 QEMU with KVM on
+         * AArch64 hosts. That won't work and isn't easy to detect.
          */
-        qtest_add_data_func("/arm/kvm/query-cpu-model-expansion",
-                            NULL, test_query_cpu_model_expansion_kvm);
+        if (qtest_has_accel("kvm")) {
+            /*
+             * This tests target the 'host' CPU type, so register it only if
+             * KVM is available.
+             */
+            qtest_add_data_func("/arm/kvm/query-cpu-model-expansion",
+                                NULL, test_query_cpu_model_expansion_kvm);
 
-        qtest_add_data_func("/arm/kvm/query-cpu-model-expansion/sve-off",
-                            NULL, sve_tests_sve_off_kvm);
+            qtest_add_data_func("/arm/kvm/query-cpu-model-expansion/sve-off",
+                                NULL, sve_tests_sve_off_kvm);
+        }
+
+        if (qtest_has_accel("tcg")) {
+            qtest_add_data_func("/arm/max/query-cpu-model-expansion/sve-max-vq-8",
+                                NULL, sve_tests_sve_max_vq_8);
+            qtest_add_data_func("/arm/max/query-cpu-model-expansion/sve-off",
+                                NULL, sve_tests_sve_off);
+        }
     }
 
-    if (qtest_has_accel("tcg")) {
-        qtest_add_data_func("/arm/max/query-cpu-model-expansion/sve-max-vq-8",
-                            NULL, sve_tests_sve_max_vq_8);
-        qtest_add_data_func("/arm/max/query-cpu-model-expansion/sve-off",
-                            NULL, sve_tests_sve_off);
-    }
-
-out:
     return g_test_run();
 }
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index f8e03dfd46..7e708d78b3 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -2089,7 +2089,7 @@ int main(int argc, char *argv[])
             qtest_add_func("acpi/piix4/nohpet", test_acpi_piix4_tcg_nohpet);
 
             /* i386 does not support memory hotplug */
-            if (strcmp(arch, "i386")) {
+            if (qtest_get_arch_bits() == 64) {
                 qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
                 qtest_add_func("acpi/piix4/dimmpxm",
                                test_acpi_piix4_tcg_dimm_pxm);
@@ -2127,7 +2127,7 @@ int main(int argc, char *argv[])
                            test_acpi_q35_tcg_acpi_hmat_noinitiator);
 
             /* i386 does not support memory hotplug */
-            if (strcmp(arch, "i386")) {
+            if (qtest_get_arch_bits() == 64) {
                 qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
                 qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
                 qtest_add_func("acpi/q35/acpihmat",
@@ -2164,15 +2164,13 @@ int main(int argc, char *argv[])
                            test_acpi_microvm_ioapic2_tcg);
             qtest_add_func("acpi/microvm/oem-fields",
                            test_acpi_microvm_oem_fields);
-            if (has_tcg) {
-                if (strcmp(arch, "x86_64") == 0) {
-                    qtest_add_func("acpi/microvm/pcie",
-                                   test_acpi_microvm_pcie_tcg);
+            if (has_tcg && qtest_get_arch_bits() == 64) {
+                qtest_add_func("acpi/microvm/pcie",
+                               test_acpi_microvm_pcie_tcg);
 #ifdef CONFIG_POSIX
-                    qtest_add_func("acpi/microvm/acpierst",
-                                   test_acpi_microvm_acpi_erst);
+                qtest_add_func("acpi/microvm/acpierst",
+                               test_acpi_microvm_acpi_erst);
 #endif
-                }
             }
         }
     } else if (strcmp(arch, "aarch64") == 0) {
-- 
2.41.0

Re: [PATCH 2/4] tests/qtest: Use qtest_get_arch_bits()

[Thomas Huth]

On 10/10/2023 09.49, Philippe Mathieu-Daudé wrote:

Some short patch description, please! Why is this necessary/useful?
(I think I know, but other might not, and it is also important for the history)

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/qtest/arm-cpu-features.c | 49 ++++++++++++++++------------------
>   tests/qtest/bios-tables-test.c | 16 +++++------
>   2 files changed, 30 insertions(+), 35 deletions(-)

  Thomas

Re: [PATCH 2/4] tests/qtest: Use qtest_get_arch_bits()

[Ani Sinha]

> On 10-Oct-2023, at 3:19 PM, Thomas Huth <thuth@redhat.com> wrote:
> 
> On 10/10/2023 09.49, Philippe Mathieu-Daudé wrote:
> 
> Some short patch description, please! Why is this necessary/useful?
> (I think I know, but other might not, and it is also important for the history)

Other than this,
Reviewed-by: Ani Sinha <anisinha@redhat.com>

> 
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>  tests/qtest/arm-cpu-features.c | 49 ++++++++++++++++------------------
>>  tests/qtest/bios-tables-test.c | 16 +++++------
>>  2 files changed, 30 insertions(+), 35 deletions(-)
> 
> Thomas
> 

[PATCH 3/4] tests/libqtest: Introduce qtest_get_base_arch()

[Philippe Mathieu-Daudé]

While qtest_get_arch() returns the target architecture name,
such "i386" or "x86_64", qtest_get_base_arch() return the
"base" (or real underlying) architecture, in this example
that is "x86".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/libqtest.h |  7 +++++++
 tests/qtest/libqtest.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 1e1b42241d..54071e74ec 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -654,6 +654,13 @@ bool qtest_big_endian(QTestState *s);
  */
 const char *qtest_get_arch(void);
 
+/**
+ * qtest_get_base_arch:
+ *
+ * Returns: The base architecture for the QEMU executable under test.
+ */
+const char *qtest_get_base_arch(void);
+
 /**
  * qtest_get_arch_bits:
  *
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index a643a6309c..51cc92af21 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -925,6 +925,34 @@ const char *qtest_get_arch(void)
     return end + 1;
 }
 
+const char *qtest_get_base_arch(void)
+{
+    static const struct {
+        const char *const arch;
+        const char *const base;
+    } basearch[] = {
+        { "aarch64", "arm" },
+        { "i386", "x86" },
+        { "loongarch64", "loongarch" },
+        { "mipsel", "mips" },
+        { "mips64", "mips" },
+        { "mips64el", "mips" },
+        { "ppc64", "ppc" },
+        { "riscv32", "riscv" },
+        { "riscv64", "riscv" },
+        { "sparc64", "sparc" },
+        { "x86_64", "x86" },
+    };
+    const char *arch = qtest_get_arch();
+
+    for (unsigned i = 0; i < ARRAY_SIZE(basearch); i++) {
+        if (!strcmp(arch, basearch[i].arch)) {
+            return basearch[i].base;
+        }
+    }
+    g_assert_not_reached();
+}
+
 unsigned qtest_get_arch_bits(void)
 {
     static const char *const arch64[] = {
-- 
2.41.0

Re: [PATCH 3/4] tests/libqtest: Introduce qtest_get_base_arch()

[Philippe Mathieu-Daudé]

On 10/10/23 09:49, Philippe Mathieu-Daudé wrote:
> While qtest_get_arch() returns the target architecture name,
> such "i386" or "x86_64", qtest_get_base_arch() return the
> "base" (or real underlying) architecture, in this example
> that is "x86".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/qtest/libqtest.h |  7 +++++++
>   tests/qtest/libqtest.c | 28 ++++++++++++++++++++++++++++
>   2 files changed, 35 insertions(+)
> 
> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
> index 1e1b42241d..54071e74ec 100644
> --- a/tests/qtest/libqtest.h
> +++ b/tests/qtest/libqtest.h
> @@ -654,6 +654,13 @@ bool qtest_big_endian(QTestState *s);
>    */
>   const char *qtest_get_arch(void);
>   
> +/**
> + * qtest_get_base_arch:
> + *
> + * Returns: The base architecture for the QEMU executable under test.
> + */
> +const char *qtest_get_base_arch(void);
> +
>   /**
>    * qtest_get_arch_bits:
>    *
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index a643a6309c..51cc92af21 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -925,6 +925,34 @@ const char *qtest_get_arch(void)
>       return end + 1;
>   }
>   
> +const char *qtest_get_base_arch(void)
> +{
> +    static const struct {
> +        const char *const arch;
> +        const char *const base;
> +    } basearch[] = {
> +        { "aarch64", "arm" },
> +        { "i386", "x86" },
> +        { "loongarch64", "loongarch" },
> +        { "mipsel", "mips" },
> +        { "mips64", "mips" },
> +        { "mips64el", "mips" },
> +        { "ppc64", "ppc" },
> +        { "riscv32", "riscv" },
> +        { "riscv64", "riscv" },
> +        { "sparc64", "sparc" },
> +        { "x86_64", "x86" },
> +    };
> +    const char *arch = qtest_get_arch();
> +
> +    for (unsigned i = 0; i < ARRAY_SIZE(basearch); i++) {
> +        if (!strcmp(arch, basearch[i].arch)) {
> +            return basearch[i].base;
> +        }
> +    }
> +    g_assert_not_reached();

Sorry, I forgot to commit this change:

-- >8 --
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -950,7 +950,8 @@ const char *qtest_get_base_arch(void)
              return basearch[i].base;
          }
      }
-    g_assert_not_reached();
+
+    return arch;
  }

---

> +}
> +
>   unsigned qtest_get_arch_bits(void)
>   {
>       static const char *const arch64[] = {

Re: [PATCH 3/4] tests/libqtest: Introduce qtest_get_base_arch()

[Thomas Huth]

On 10/10/2023 10.42, Philippe Mathieu-Daudé wrote:
> On 10/10/23 09:49, Philippe Mathieu-Daudé wrote:
>> While qtest_get_arch() returns the target architecture name,
>> such "i386" or "x86_64", qtest_get_base_arch() return the
>> "base" (or real underlying) architecture, in this example
>> that is "x86".
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   tests/qtest/libqtest.h |  7 +++++++
>>   tests/qtest/libqtest.c | 28 ++++++++++++++++++++++++++++
>>   2 files changed, 35 insertions(+)
>>
>> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
>> index 1e1b42241d..54071e74ec 100644
>> --- a/tests/qtest/libqtest.h
>> +++ b/tests/qtest/libqtest.h
>> @@ -654,6 +654,13 @@ bool qtest_big_endian(QTestState *s);
>>    */
>>   const char *qtest_get_arch(void);
>> +/**
>> + * qtest_get_base_arch:
>> + *
>> + * Returns: The base architecture for the QEMU executable under test.
>> + */
>> +const char *qtest_get_base_arch(void);
>> +
>>   /**
>>    * qtest_get_arch_bits:
>>    *
>> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
>> index a643a6309c..51cc92af21 100644
>> --- a/tests/qtest/libqtest.c
>> +++ b/tests/qtest/libqtest.c
>> @@ -925,6 +925,34 @@ const char *qtest_get_arch(void)
>>       return end + 1;
>>   }
>> +const char *qtest_get_base_arch(void)
>> +{
>> +    static const struct {
>> +        const char *const arch;
>> +        const char *const base;
>> +    } basearch[] = {
>> +        { "aarch64", "arm" },
>> +        { "i386", "x86" },
>> +        { "loongarch64", "loongarch" },
>> +        { "mipsel", "mips" },
>> +        { "mips64", "mips" },
>> +        { "mips64el", "mips" },
>> +        { "ppc64", "ppc" },
>> +        { "riscv32", "riscv" },
>> +        { "riscv64", "riscv" },
>> +        { "sparc64", "sparc" },
>> +        { "x86_64", "x86" },
>> +    };
>> +    const char *arch = qtest_get_arch();
>> +
>> +    for (unsigned i = 0; i < ARRAY_SIZE(basearch); i++) {
>> +        if (!strcmp(arch, basearch[i].arch)) {
>> +            return basearch[i].base;
>> +        }
>> +    }
>> +    g_assert_not_reached();
> 
> Sorry, I forgot to commit this change:
> 
> -- >8 --
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -950,7 +950,8 @@ const char *qtest_get_base_arch(void)
>               return basearch[i].base;
>           }
>       }
> -    g_assert_not_reached();
> +
> +    return arch;
>   }

I'd maybe also do a caching here, as suggested in the first patch.

  Thomas

[PATCH 4/4] tests/qtest: Use qtest_get_base_arch()

[Philippe Mathieu-Daudé]

Additionally use qtest_get_arch_bits() when relevant.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/ahci-test.c           |  5 ++---
 tests/qtest/am53c974-test.c       |  4 +---
 tests/qtest/bios-tables-test.c    |  6 +++---
 tests/qtest/boot-sector.c         |  6 +++---
 tests/qtest/device-plug-test.c    |  6 ++----
 tests/qtest/drive_del-test.c      | 16 ++++++----------
 tests/qtest/erst-test.c           |  4 +---
 tests/qtest/fuzz-e1000e-test.c    |  4 +---
 tests/qtest/ivshmem-test.c        |  6 +++---
 tests/qtest/libqos/qos_external.c |  6 ++++--
 tests/qtest/lpc-ich9-test.c       |  4 +---
 tests/qtest/m48t59-test.c         |  6 ++----
 tests/qtest/pxe-test.c            |  6 +++---
 tests/qtest/qos-test.c            |  6 ++++--
 tests/qtest/readconfig-test.c     |  6 +-----
 tests/qtest/rtas-test.c           |  4 +---
 tests/qtest/ufs-test.c            |  4 +---
 tests/qtest/usb-hcd-uhci-test.c   |  6 +++---
 tests/qtest/virtio-net-test.c     |  3 +--
 tests/qtest/virtio-rng-test.c     |  4 +---
 20 files changed, 44 insertions(+), 68 deletions(-)

diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index eea8b5f77b..93d1e14896 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1835,7 +1835,7 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
 
 int main(int argc, char **argv)
 {
-    const char *arch, *base;
+    const char *base;
     int ret;
     int fd;
     int c;
@@ -1867,8 +1867,7 @@ int main(int argc, char **argv)
     }
 
     /* Check architecture */
-    arch = qtest_get_arch();
-    if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
+    if (strcmp(qtest_get_base_arch(), "x86")) {
         g_test_message("Skipping test for non-x86");
         return 0;
     }
diff --git a/tests/qtest/am53c974-test.c b/tests/qtest/am53c974-test.c
index ed3ac7db20..dc41182a38 100644
--- a/tests/qtest/am53c974-test.c
+++ b/tests/qtest/am53c974-test.c
@@ -253,11 +253,9 @@ static void test_reset_before_transfer_ok(void)
 
 int main(int argc, char **argv)
 {
-    const char *arch = qtest_get_arch();
-
     g_test_init(&argc, &argv, NULL);
 
-    if (strcmp(arch, "i386") == 0) {
+    if (!strcmp(qtest_get_base_arch(), "x86") && qtest_get_arch_bits() == 32) {
         qtest_add_func("am53c974/test_cmdfifo_underflow_ok",
                        test_cmdfifo_underflow_ok);
         qtest_add_func("am53c974/test_cmdfifo_underflow2_ok",
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 7e708d78b3..573e7dfcd9 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -2044,7 +2044,7 @@ static void test_acpi_virt_oem_fields(void)
 
 int main(int argc, char *argv[])
 {
-    const char *arch = qtest_get_arch();
+    const char *arch = qtest_get_base_arch();
     bool has_kvm, has_tcg;
     char *v_env = getenv("V");
     int ret;
@@ -2063,7 +2063,7 @@ int main(int argc, char *argv[])
         return 0;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(arch, "x86") == 0) {
         ret = boot_sector_init(disk);
         if (ret) {
             return ret;
@@ -2173,7 +2173,7 @@ int main(int argc, char *argv[])
 #endif
             }
         }
-    } else if (strcmp(arch, "aarch64") == 0) {
+    } else if (strcmp(arch, "arm") == 0 && qtest_get_arch_bits() == 64) {
         if (has_tcg && qtest_has_device("virtio-blk-pci")) {
             qtest_add_func("acpi/virt", test_acpi_virt_tcg);
             qtest_add_func("acpi/virt/acpihmatvirt",
diff --git a/tests/qtest/boot-sector.c b/tests/qtest/boot-sector.c
index 679ee17e2a..f13e9a12ff 100644
--- a/tests/qtest/boot-sector.c
+++ b/tests/qtest/boot-sector.c
@@ -90,7 +90,7 @@ int boot_sector_init(char *fname)
     int fd, ret;
     size_t len;
     char *boot_code;
-    const char *arch = qtest_get_arch();
+    const char *arch = qtest_get_base_arch();
 
     fd = mkstemp(fname);
     if (fd < 0) {
@@ -98,12 +98,12 @@ int boot_sector_init(char *fname)
         return 1;
     }
 
-    if (g_str_equal(arch, "i386") || g_str_equal(arch, "x86_64")) {
+    if (g_str_equal(arch, "x86")) {
         /* Q35 requires a minimum 0x7e000 bytes disk (bug or feature?) */
         len = MAX(0x7e000, sizeof(x86_boot_sector));
         boot_code = g_malloc0(len);
         memcpy(boot_code, x86_boot_sector, sizeof(x86_boot_sector));
-    } else if (g_str_equal(arch, "ppc64")) {
+    } else if (g_str_equal(arch, "ppc") && qtest_get_arch_bits() == 64) {
         /* For Open Firmware based system, use a Forth script */
         boot_code = g_strdup_printf("\\ Bootscript\n%x %x c! %x %x c!\n",
                                     LOW(SIGNATURE), SIGNATURE_ADDR,
diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
index c6f33153eb..59153e3493 100644
--- a/tests/qtest/device-plug-test.c
+++ b/tests/qtest/device-plug-test.c
@@ -65,7 +65,6 @@ static void process_device_remove(QTestState *qtest, const char *id)
 static void test_pci_unplug_request(void)
 {
     QTestState *qtest;
-    const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
     if (!qtest_has_device("virtio-mouse-pci")) {
@@ -73,7 +72,7 @@ static void test_pci_unplug_request(void)
         return;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         machine_addition = "-machine pc";
     }
 
@@ -107,7 +106,6 @@ static void test_q35_pci_unplug_request(void)
 static void test_pci_unplug_json_request(void)
 {
     QTestState *qtest;
-    const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
     if (!qtest_has_device("virtio-mouse-pci")) {
@@ -115,7 +113,7 @@ static void test_pci_unplug_json_request(void)
         return;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         machine_addition = "-machine pc";
     }
 
diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index 8a6f3ac963..ccc42acce9 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -125,9 +125,9 @@ static void drive_del(QTestState *qts)
  */
 static const char *qvirtio_get_dev_type(void)
 {
-    const char *arch = qtest_get_arch();
+    const char *arch = qtest_get_base_arch();
 
-    if (g_str_equal(arch, "arm") || g_str_equal(arch, "aarch64")) {
+    if (g_str_equal(arch, "arm")) {
         return "device";  /* for virtio-mmio */
     } else if (g_str_equal(arch, "s390x")) {
         return "ccw";
@@ -249,7 +249,6 @@ static void test_drive_del_device_del(void)
 static void test_cli_device_del(void)
 {
     QTestState *qts;
-    const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
     if (!has_device_builtin("virtio-blk")) {
@@ -257,7 +256,7 @@ static void test_cli_device_del(void)
         return;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         machine_addition = "-machine pc";
     }
 
@@ -323,7 +322,6 @@ static void test_empty_device_del(void)
 static void test_device_add_and_del(void)
 {
     QTestState *qts;
-    const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
     if (!has_device_builtin("virtio-blk")) {
@@ -331,7 +329,7 @@ static void test_device_add_and_del(void)
         return;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         machine_addition = "-machine pc";
     }
 
@@ -394,7 +392,6 @@ static void test_device_add_and_del_q35(void)
 static void test_drive_add_device_add_and_del(void)
 {
     QTestState *qts;
-    const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
     if (!has_device_builtin("virtio-blk")) {
@@ -402,7 +399,7 @@ static void test_drive_add_device_add_and_del(void)
         return;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         machine_addition = "-machine pc";
     }
 
@@ -447,7 +444,6 @@ static void test_drive_add_device_add_and_del_q35(void)
 static void test_blockdev_add_device_add_and_del(void)
 {
     QTestState *qts;
-    const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
     if (!has_device_builtin("virtio-blk")) {
@@ -455,7 +451,7 @@ static void test_blockdev_add_device_add_and_del(void)
         return;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         machine_addition = "-machine pc";
     }
 
diff --git a/tests/qtest/erst-test.c b/tests/qtest/erst-test.c
index c45bee7f05..8d3665b82c 100644
--- a/tests/qtest/erst-test.c
+++ b/tests/qtest/erst-test.c
@@ -95,9 +95,7 @@ static void cleanup_vm(ERSTState *s)
 
 static void setup_vm_cmd(ERSTState *s, const char *cmd)
 {
-    const char *arch = qtest_get_arch();
-
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         s->qs = qtest_pc_boot("%s", cmd);
     } else {
         g_printerr("erst-test tests are only available on x86\n");
diff --git a/tests/qtest/fuzz-e1000e-test.c b/tests/qtest/fuzz-e1000e-test.c
index 5052883fb6..a8d3596f2c 100644
--- a/tests/qtest/fuzz-e1000e-test.c
+++ b/tests/qtest/fuzz-e1000e-test.c
@@ -40,11 +40,9 @@ static void test_lp1879531_eth_get_rss_ex_dst_addr(void)
 
 int main(int argc, char **argv)
 {
-    const char *arch = qtest_get_arch();
-
     g_test_init(&argc, &argv, NULL);
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         qtest_add_func("fuzz/test_lp1879531_eth_get_rss_ex_dst_addr",
                        test_lp1879531_eth_get_rss_ex_dst_addr);
     }
diff --git a/tests/qtest/ivshmem-test.c b/tests/qtest/ivshmem-test.c
index 9bf8e78df6..32024d9243 100644
--- a/tests/qtest/ivshmem-test.c
+++ b/tests/qtest/ivshmem-test.c
@@ -397,9 +397,9 @@ static void test_ivshmem_hotplug_q35(void)
 static void test_ivshmem_hotplug(void)
 {
     QTestState *qts;
-    const char *arch = qtest_get_arch();
+    const char *arch = qtest_get_base_arch();
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(arch, "x86") == 0) {
         qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1"
                          " -machine pc");
     } else {
@@ -409,7 +409,7 @@ static void test_ivshmem_hotplug(void)
     qtest_qmp_device_add(qts, "ivshmem-plain", "iv1",
                          "{'addr': %s, 'memdev': 'mb1'}",
                          stringify(PCI_SLOT_HP));
-    if (strcmp(arch, "ppc64") != 0) {
+    if (strcmp(arch, "ppc") != 0 && qtest_get_arch_bits() == 64) {
         qpci_unplug_acpi_device_test(qts, "iv1", PCI_SLOT_HP);
     }
 
diff --git a/tests/qtest/libqos/qos_external.c b/tests/qtest/libqos/qos_external.c
index c6bb8bff09..f01bb17628 100644
--- a/tests/qtest/libqos/qos_external.c
+++ b/tests/qtest/libqos/qos_external.c
@@ -31,10 +31,12 @@
 
 static void machine_apply_to_node(const char *name)
 {
-    char *machine_name = g_strconcat(qtest_get_arch(), "/", name, NULL);
+    g_autofree char *machine_name = g_strdup_printf("%s/%u/%s",
+                                                    qtest_get_base_arch(),
+                                                    qtest_get_arch_bits(),
+                                                    name);
 
     qos_graph_node_set_availability(machine_name, true);
-    g_free(machine_name);
 }
 
 void machines_apply_to_node(MachineInfoList *mach_info)
diff --git a/tests/qtest/lpc-ich9-test.c b/tests/qtest/lpc-ich9-test.c
index 8ac95b89f7..9f27591b74 100644
--- a/tests/qtest/lpc-ich9-test.c
+++ b/tests/qtest/lpc-ich9-test.c
@@ -28,11 +28,9 @@ static void test_lp1878642_pci_bus_get_irq_level_assert(void)
 
 int main(int argc, char **argv)
 {
-    const char *arch = qtest_get_arch();
-
     g_test_init(&argc, &argv, NULL);
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         qtest_add_func("ich9/test_lp1878642_pci_bus_get_irq_level_assert",
                        test_lp1878642_pci_bus_get_irq_level_assert);
     }
diff --git a/tests/qtest/m48t59-test.c b/tests/qtest/m48t59-test.c
index b9cd209165..3579effdf4 100644
--- a/tests/qtest/m48t59-test.c
+++ b/tests/qtest/m48t59-test.c
@@ -239,15 +239,13 @@ static void fuzz_registers(void)
 
 static void base_setup(void)
 {
-    const char *arch = qtest_get_arch();
-
-    if (g_str_equal(arch, "sparc")) {
+    if (g_str_equal(qtest_get_arch(), "sparc")) {
         /* Note: For sparc64, we'd need to map-in the PCI bridge memory first */
         base = 0x71200000;
         base_year = 1968;
         base_machine = "SS-5";
         use_mmio = true;
-    } else if (g_str_equal(arch, "ppc") || g_str_equal(arch, "ppc64")) {
+    } else if (g_str_equal(qtest_get_base_arch(), "ppc")) {
         base = 0xF0000000;
         base_year = 1968;
         base_machine = "ref405ep";
diff --git a/tests/qtest/pxe-test.c b/tests/qtest/pxe-test.c
index e4b48225a5..5112bceb76 100644
--- a/tests/qtest/pxe-test.c
+++ b/tests/qtest/pxe-test.c
@@ -129,7 +129,7 @@ static void test_batch(const testdef_t *tests, bool ipv6)
 int main(int argc, char *argv[])
 {
     int ret;
-    const char *arch = qtest_get_arch();
+    const char *arch = qtest_get_base_arch();
 
     g_test_init(&argc, &argv, NULL);
 
@@ -143,12 +143,12 @@ int main(int argc, char *argv[])
         return ret;
 
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(arch, "x86") == 0) {
         test_batch(x86_tests, false);
         if (g_test_slow()) {
             test_batch(x86_tests_slow, false);
         }
-    } else if (strcmp(arch, "ppc64") == 0) {
+    } else if (strcmp(arch, "ppc") == 0 && qtest_get_arch_bits() == 64) {
         test_batch(ppc64_tests, g_test_slow());
         if (g_test_slow()) {
             test_batch(ppc64_tests_slow, true);
diff --git a/tests/qtest/qos-test.c b/tests/qtest/qos-test.c
index 5da4091ec3..5271582f6a 100644
--- a/tests/qtest/qos-test.c
+++ b/tests/qtest/qos-test.c
@@ -293,8 +293,10 @@ static void walk_path(QOSGraphNode *orig_path, int len)
     path_vec[0] = g_string_free(cmd_line, false);
 
     if (path->u.test.subprocess) {
-        gchar *subprocess_path = g_strdup_printf("/%s/%s/subprocess",
-                                                 qtest_get_arch(), path_str);
+        gchar *subprocess_path = g_strdup_printf("/%s/%u/%s/subprocess",
+                                                 qtest_get_base_arch(),
+                                                 qtest_get_arch_bits(),
+                                                 path_str);
         qtest_add_data_func(path_str, subprocess_path, subprocess_run_one_test);
         g_test_add_data_func(subprocess_path, path_vec, run_one_test);
     } else {
diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c
index 760f974e63..7837997c6f 100644
--- a/tests/qtest/readconfig-test.c
+++ b/tests/qtest/readconfig-test.c
@@ -382,13 +382,9 @@ static void test_docs_q35_virtio_serial(void)
 
 int main(int argc, char *argv[])
 {
-    const char *arch;
     g_test_init(&argc, &argv, NULL);
 
-    arch = qtest_get_arch();
-
-    if (g_str_equal(arch, "i386") ||
-        g_str_equal(arch, "x86_64")) {
+    if (g_str_equal(qtest_get_base_arch(), "x86")) {
         qtest_add_func("readconfig/x86/memdev", test_x86_memdev);
         if (qtest_has_device("ich9-usb-ehci1") &&
             qtest_has_device("ich9-usb-uhci1")) {
diff --git a/tests/qtest/rtas-test.c b/tests/qtest/rtas-test.c
index 1ba42b37d2..26c8e8e926 100644
--- a/tests/qtest/rtas-test.c
+++ b/tests/qtest/rtas-test.c
@@ -36,11 +36,9 @@ static void test_rtas_get_time_of_day_vof(void)
 
 int main(int argc, char *argv[])
 {
-    const char *arch = qtest_get_arch();
-
     g_test_init(&argc, &argv, NULL);
 
-    if (strcmp(arch, "ppc64")) {
+    if (strcmp(qtest_get_base_arch(), "ppc") && qtest_get_arch_bits() != 64) {
         g_printerr("RTAS requires qemu-system-ppc64\n");
         exit(EXIT_FAILURE);
     }
diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c
index ed3dbca154..5a5b10b520 100644
--- a/tests/qtest/ufs-test.c
+++ b/tests/qtest/ufs-test.c
@@ -552,7 +552,6 @@ static void *ufs_blk_test_setup(GString *cmd_line, void *arg)
 
 static void ufs_register_nodes(void)
 {
-    const char *arch;
     QOSGraphEdgeOptions edge_opts = {
         .before_cmd_line = "-blockdev null-co,node-name=drv0,read-zeroes=on",
         .after_cmd_line = "-device ufs-lu,bus=ufs0,drive=drv0,lun=0",
@@ -575,8 +574,7 @@ static void ufs_register_nodes(void)
      * Check architecture
      * TODO: Enable ufs io tests for ppc64
      */
-    arch = qtest_get_arch();
-    if (!strcmp(arch, "ppc64")) {
+    if (!strcmp(qtest_get_base_arch(), "ppc") && qtest_get_arch_bits() == 64) {
         g_test_message("Skipping ufs io tests for ppc64");
         return;
     }
diff --git a/tests/qtest/usb-hcd-uhci-test.c b/tests/qtest/usb-hcd-uhci-test.c
index 4446555f08..144a5541f0 100644
--- a/tests/qtest/usb-hcd-uhci-test.c
+++ b/tests/qtest/usb-hcd-uhci-test.c
@@ -53,7 +53,7 @@ static void test_usb_storage_hotplug(void)
 
 int main(int argc, char **argv)
 {
-    const char *arch = qtest_get_arch();
+    const char *arch = qtest_get_base_arch();
     const char *cmd = "-device piix3-usb-uhci,id=uhci,addr=1d.0"
                       " -drive id=drive0,if=none,file=null-co://,"
                       "file.read-zeroes=on,format=raw"
@@ -73,9 +73,9 @@ int main(int argc, char **argv)
         qtest_add_func("/uhci/pci/hotplug/usb-storage", test_usb_storage_hotplug);
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(arch, "x86") == 0) {
         qs = qtest_pc_boot("%s", cmd);
-    } else if (strcmp(arch, "ppc64") == 0) {
+    } else if (strcmp(arch, "ppc") == 0 && qtest_get_arch_bits() == 64) {
         qs = qtest_spapr_boot("%s", cmd);
     } else {
         g_printerr("usb-hcd-uhci-test tests are only "
diff --git a/tests/qtest/virtio-net-test.c b/tests/qtest/virtio-net-test.c
index fab5dd8b05..c37fe36af6 100644
--- a/tests/qtest/virtio-net-test.c
+++ b/tests/qtest/virtio-net-test.c
@@ -169,7 +169,6 @@ static void hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtioPCIDevice *dev = obj;
     QTestState *qts = dev->pdev->bus->qts;
-    const char *arch = qtest_get_arch();
 
     if (dev->pdev->bus->not_hotpluggable) {
         g_test_skip("pci bus does not support hotplug");
@@ -179,7 +178,7 @@ static void hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
     qtest_qmp_device_add(qts, "virtio-net-pci", "net1",
                          "{'addr': %s}", stringify(PCI_SLOT_HP));
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         qpci_unplug_acpi_device_test(qts, "net1", PCI_SLOT_HP);
     }
 }
diff --git a/tests/qtest/virtio-rng-test.c b/tests/qtest/virtio-rng-test.c
index 64e131cd8c..173f42ff7b 100644
--- a/tests/qtest/virtio-rng-test.c
+++ b/tests/qtest/virtio-rng-test.c
@@ -25,12 +25,10 @@ static void rng_hotplug(void *obj, void *data, QGuestAllocator *alloc)
         return;
     }
 
-    const char *arch = qtest_get_arch();
-
     qtest_qmp_device_add(qts, "virtio-rng-pci", "rng1",
                          "{'addr': %s}", stringify(PCI_SLOT_HP));
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         qpci_unplug_acpi_device_test(qts, "rng1", PCI_SLOT_HP);
     }
 }
-- 
2.41.0

Re: [PATCH 4/4] tests/qtest: Use qtest_get_base_arch()

[Akihiko Odaki]

On 2023/10/10 16:49, Philippe Mathieu-Daudé wrote:
> Additionally use qtest_get_arch_bits() when relevant.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH 4/4] tests/qtest: Use qtest_get_base_arch()

[Thomas Huth]

On 10/10/2023 09.49, Philippe Mathieu-Daudé wrote:
> Additionally use qtest_get_arch_bits() when relevant.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/qtest/ahci-test.c           |  5 ++---
>   tests/qtest/am53c974-test.c       |  4 +---
>   tests/qtest/bios-tables-test.c    |  6 +++---
>   tests/qtest/boot-sector.c         |  6 +++---
>   tests/qtest/device-plug-test.c    |  6 ++----
>   tests/qtest/drive_del-test.c      | 16 ++++++----------
>   tests/qtest/erst-test.c           |  4 +---
>   tests/qtest/fuzz-e1000e-test.c    |  4 +---
>   tests/qtest/ivshmem-test.c        |  6 +++---
>   tests/qtest/libqos/qos_external.c |  6 ++++--
>   tests/qtest/lpc-ich9-test.c       |  4 +---
>   tests/qtest/m48t59-test.c         |  6 ++----
>   tests/qtest/pxe-test.c            |  6 +++---
>   tests/qtest/qos-test.c            |  6 ++++--
>   tests/qtest/readconfig-test.c     |  6 +-----
>   tests/qtest/rtas-test.c           |  4 +---
>   tests/qtest/ufs-test.c            |  4 +---
>   tests/qtest/usb-hcd-uhci-test.c   |  6 +++---
>   tests/qtest/virtio-net-test.c     |  3 +--
>   tests/qtest/virtio-rng-test.c     |  4 +---
>   20 files changed, 44 insertions(+), 68 deletions(-)
> 
> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
> index eea8b5f77b..93d1e14896 100644
> --- a/tests/qtest/ahci-test.c
> +++ b/tests/qtest/ahci-test.c
> @@ -1835,7 +1835,7 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
>   
>   int main(int argc, char **argv)
>   {
> -    const char *arch, *base;
> +    const char *base;
>       int ret;
>       int fd;
>       int c;
> @@ -1867,8 +1867,7 @@ int main(int argc, char **argv)
>       }
>   
>       /* Check architecture */
> -    arch = qtest_get_arch();
> -    if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
> +    if (strcmp(qtest_get_base_arch(), "x86")) {
>           g_test_message("Skipping test for non-x86");
>           return 0;
>       }

While this change makes sense (unifying two checks into one) ...

> diff --git a/tests/qtest/am53c974-test.c b/tests/qtest/am53c974-test.c
> index ed3ac7db20..dc41182a38 100644
> --- a/tests/qtest/am53c974-test.c
> +++ b/tests/qtest/am53c974-test.c
> @@ -253,11 +253,9 @@ static void test_reset_before_transfer_ok(void)
>   
>   int main(int argc, char **argv)
>   {
> -    const char *arch = qtest_get_arch();
> -
>       g_test_init(&argc, &argv, NULL);
>   
> -    if (strcmp(arch, "i386") == 0) {
> +    if (!strcmp(qtest_get_base_arch(), "x86") && qtest_get_arch_bits() == 32) {

... this change looks more cumbersome now (doing two checks now instead of 
one), at least at the current point in time. Do you urgently need this for 
your refactoring? If not, I'd maybe postpone such changes that make the 
checks more compilcated to a later point in time.

  Thomas

Re: [PATCH 4/4] tests/qtest: Use qtest_get_base_arch()

[Philippe Mathieu-Daudé]

On 10/10/23 11:58, Thomas Huth wrote:
> On 10/10/2023 09.49, Philippe Mathieu-Daudé wrote:
>> Additionally use qtest_get_arch_bits() when relevant.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   tests/qtest/ahci-test.c           |  5 ++---
>>   tests/qtest/am53c974-test.c       |  4 +---
>>   tests/qtest/bios-tables-test.c    |  6 +++---
>>   tests/qtest/boot-sector.c         |  6 +++---
>>   tests/qtest/device-plug-test.c    |  6 ++----
>>   tests/qtest/drive_del-test.c      | 16 ++++++----------


>> @@ -1867,8 +1867,7 @@ int main(int argc, char **argv)
>>       }
>>       /* Check architecture */
>> -    arch = qtest_get_arch();
>> -    if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
>> +    if (strcmp(qtest_get_base_arch(), "x86")) {
>>           g_test_message("Skipping test for non-x86");
>>           return 0;
>>       }
> 
> While this change makes sense (unifying two checks into one) ...
> 
>> diff --git a/tests/qtest/am53c974-test.c b/tests/qtest/am53c974-test.c
>> index ed3ac7db20..dc41182a38 100644
>> --- a/tests/qtest/am53c974-test.c
>> +++ b/tests/qtest/am53c974-test.c
>> @@ -253,11 +253,9 @@ static void test_reset_before_transfer_ok(void)
>>   int main(int argc, char **argv)
>>   {
>> -    const char *arch = qtest_get_arch();
>> -
>>       g_test_init(&argc, &argv, NULL);
>> -    if (strcmp(arch, "i386") == 0) {
>> +    if (!strcmp(qtest_get_base_arch(), "x86") && 
>> qtest_get_arch_bits() == 32) {
> 
> ... this change looks more cumbersome now (doing two checks now instead 
> of one), at least at the current point in time. Do you urgently need 
> this for your refactoring? If not, I'd maybe postpone such changes that 
> make the checks more compilcated to a later point in time.

I wanted to replace qtest_get_arch() completely, but not that simple :(