Re: [PATCH v8 08/11] tests/qtest: Fix tests when no KVM or TCG are present

[Thomas Huth <thuth@redhat.com>]

On 10/03/2023 16.37, Fabiano Rosas wrote:
> Thomas Huth <thuth@redhat.com> writes:
> 
>> On 10/03/2023 14.06, Fabiano Rosas wrote:
>>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>>>
>>>> On Thu, Mar 09, 2023 at 05:14:31PM -0300, Fabiano Rosas wrote:
>>>>> It is possible to have a build with both TCG and KVM disabled due to
>>>>> Xen requiring the i386 and x86_64 binaries to be present in an aarch64
>>>>> host.
>>>>>
>>>>> If we build with --disable-tcg on the aarch64 host, we will end-up
>>>>> with a QEMU binary (x86) that does not support TCG nor KVM.
>>>>>
>>>>> Fix tests that crash or hang in the above scenario. Do not include any
>>>>> test cases if TCG and KVM are missing.
>>>>>
>>>>> Make sure that calls to qtest_has_accel are placed after g_test_init
>>>>> in similar fashion to commit ae4b01b349 ("tests: Ensure TAP version is
>>>>> printed before other messages") to avoid TAP parsing errors.
>>>>>
>>>>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>>>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>>>>
>>>> I don't like it that we are hard-coding the list of accelerators
>>>> like this. Make a wrapper?
>>>>
>>>
>>> Are you thinking of some sort of "has_any_accel" wrapper?
>>
>> I think in the long run, we want something like what I described here:
>>
>> https://lore.kernel.org/qemu-devel/ee0cad00-a6f3-f0c1-adf0-ba32329354f3@redhat.com/
>>
> 
> Wont't that function be too generic? The choice of accelerator is quite
> specific to each individual test, some might not work with TCG, some
> might not work with HVF and so on. There is no link between build-time
> configuration and runtime test execution after all. We could always have
> a build without an accelerator and then try to run a test that uses that
> accelerator. And also have an accelerator present that the test does not
> support at all.
> 
> 
> For this particularly bizarre case of not having TCG nor KVM in the
> build I'm inclined to go with Michael's suggestion of checking it at
> build time and skipping all the hassle. This is what I'm preparing:
> 
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 29a4efb4c2..e698cdcb60 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -27,6 +27,12 @@ if config_host.has_key('CONFIG_MODULES')
>     qtests_generic += [ 'modules-test' ]
>   endif
>   
> +# For x86_64, i386 and aarch64 it is possible to have only Xen as an
> +# accelerator. Some tests require either TCG or KVM, so make sure they
> +# are present before building those tests.
> +tcg_or_kvm_available = (config_all.has_key('CONFIG_TCG') or
> +                        config_all.has_key('CONFIG_KVM'))
> +
>   qtests_pci = \
>     (config_all_devices.has_key('CONFIG_VGA') ? ['display-vga-test'] : []) +                  \
>     (config_all_devices.has_key('CONFIG_IVSHMEM_DEVICE') ? ['ivshmem-test'] : [])
> @@ -40,11 +46,12 @@ qtests_filter = \
>     (config_host.has_key('CONFIG_POSIX') ? ['test-filter-redirector'] : [])
>   
>   qtests_i386 = \
> -  (slirp.found() ? ['pxe-test'] : []) + \
> +  (slirp.found() and tcg_or_kvm_available ? ['pxe-test'] : []) +                            \
> ---
> 
> What do you think?

It's likely ok as additional check for Xen-only builds, but we certainly 
cannot remove the runtime checks for KVM (it's perfectly fine to have KVM 
included during compilation - but unavailable during runtime (e.g. if the 
kernel module is not loaded).

  Thomas