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

[Thomas Huth <thuth@redhat.com>]

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