Re: [PATCH 02/53] tests: qtest: add qtest_has_accel() to check if tested binary supports accelerator

[Philippe Mathieu-Daudé]

On 6/25/21 11:17 AM, Igor Mammedov wrote:
> Currently it is not possible to create tests that have KVM as a hard
> requirement on a host that doesn't support KVM for tested target
> binary (modulo going through the trouble of compiling out
> the offending test case).
> 
> Following scenario makes test fail when it's run on non x86 host:
>   qemu-system-x86_64 -enable-kvm -M q35,kernel-irqchip=on -smp 1,maxcpus=288
> 
> This patch introduces qtest_has_accel() to let users check if accel is
> available in advance and avoid executing non run-able test-cases.
> 
> It implements detection of TCG and KVM only, the rest could be
> added later on, when we actually start testing them in qtest.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> PS:
> There is an alternative 'runtime' approach on list:
> 'query-accels' series.
> https://patchwork.kernel.org/project/qemu-devel/patch/20210503211020.894589-4-philmd@redhat.com/
> 
> on upside this impl. much cheaper to execute than the 'query-accels'
> as it doesn't need to run QEMU for probing.
> 
> v4:
>   - s/qtest_has_kvm/qtest_has_accel/
>   - add TCG detection (based on --disable-tcg)
> v3:
>   - make it work with --disable-kvm
>        (Claudio Fontana <cfontana@suse.de>)
> v2:
>   - fix access() check.
>      s/access()/!access()/
>   - format C array items at meson.build time
> 
> CC: thuth@redhat.com
> CC: lvivier@redhat.com
> CC: f4bug@amsat.org
> ---
>  tests/qtest/libqos/libqtest.h |  8 ++++++++
>  meson.build                   |  6 ++++++
>  tests/qtest/libqtest.c        | 27 +++++++++++++++++++++++++++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/tests/qtest/libqos/libqtest.h b/tests/qtest/libqos/libqtest.h
> index a68dcd79d4..59e9271195 100644
> --- a/tests/qtest/libqos/libqtest.h
> +++ b/tests/qtest/libqos/libqtest.h
> @@ -588,6 +588,14 @@ bool qtest_big_endian(QTestState *s);
>   */
>  const char *qtest_get_arch(void);
>  
> +/**
> + * qtest_has_accel:
> + * @accel_name: Accelerator name to check for.
> + *
> + * Returns: true if the accelerator is built in.
> + */
> +bool qtest_has_accel(const char *accel_name);
> +
>  /**
>   * qtest_add_func:
>   * @str: Test case path.
> diff --git a/meson.build b/meson.build
> index d8a92666fb..7eeb201179 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -76,6 +76,12 @@ else
>    kvm_targets = []
>  endif
>  
> +kvm_targets_c = ''
> +if not get_option('kvm').disabled() and targetos == 'linux'
> +  kvm_targets_c = '"' + '" ,"'.join(kvm_targets) + '"'
> +endif
> +config_host_data.set('CONFIG_KVM_TARGETS', kvm_targets_c)
> +
>  accelerator_targets = { 'CONFIG_KVM': kvm_targets }
>  if cpu in ['x86', 'x86_64', 'arm', 'aarch64']
>    # i368 emulator provides xenpv machine type for multiple architectures
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 825b13a44c..643769e416 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -920,6 +920,33 @@ const char *qtest_get_arch(void)
>      return end + 1;
>  }
>  
> +bool qtest_has_accel(const char *accel_name)
> +{
> +    if (g_str_equal(accel_name, "tcg")) {
> +#if defined(CONFIG_TCG)

The point of the "alternative 'runtime' approach" was to remove
access to accel-specific definitions in tests/ and you are
adding yet another one :( Maybe now it is acceptable, I am not
sure since I haven't followed the latest accelerator modularization
effort series. So I'll defer that to the active developers doing
it.

> +        return true;
> +#else
> +        return false;
> +#endif
> +    } else if (g_str_equal(accel_name, "kvm")) {
> +        int i;
> +        const char *arch = qtest_get_arch();
> +        const char *targets[] = { CONFIG_KVM_TARGETS };
> +
> +        for (i = 0; i < ARRAY_SIZE(targets); i++) {
> +            if (!strncmp(targets[i], arch, strlen(arch))) {
> +                if (!access("/dev/kvm", R_OK | W_OK)) {
> +                    return true;
> +                }
> +            }
> +        }
> +    } else {
> +        /* not implemented */
> +        g_assert_not_reached();
> +    }
> +    return false;
> +}
> +
>  bool qtest_get_irq(QTestState *s, int num)
>  {
>      /* dummy operation in order to make sure irq is up to date */
>