Re: [PATCH] tests/qtest/cpu-plug-test: Add cpu hotplug support for LoongArch

[Thomas Huth <thuth@redhat.com>]

On 10/03/2025 07.24, Bibo Mao wrote:
> Add cpu hotplug testcase support for LoongArch system, it passes to
> run with command "make check-qtest-loongarch64" as following:
>    qemu:qtest+qtest-loongarch64 / qtest-loongarch64/cpu-plug-test OK 0.64s 1 subtests passed
> 
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>   tests/qtest/cpu-plug-test.c | 28 ++++++++++++++++++++++++++++
>   tests/qtest/meson.build     |  2 +-
>   2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/cpu-plug-test.c b/tests/qtest/cpu-plug-test.c
> index 6633abfc10..790b9693b3 100644
> --- a/tests/qtest/cpu-plug-test.c
> +++ b/tests/qtest/cpu-plug-test.c
> @@ -156,6 +156,32 @@ static void add_s390x_test_case(const char *mname)
>       g_free(path);
>   }
>   
> +static void add_loongarch_test_case(const char *mname)
> +{
> +    char *path;
> +    PlugTestData *data;
> +
> +    if (!g_str_has_prefix(mname, "virt")) {
> +        return;
> +    }
> +
> +    data = g_new(PlugTestData, 1);
> +    data->machine = g_strdup(mname);
> +    data->cpu_model = "la464";
> +    data->device_model = g_strdup("la464-loongarch-cpu");
> +    data->sockets = 1;
> +    data->cores = 3;
> +    data->threads = 1;
> +    data->maxcpus = data->sockets * data->cores * data->threads;
> +
> +    path = g_strdup_printf("cpu-plug/%s/device-add/%ux%ux%u&maxcpus=%u",
> +                           mname, data->sockets, data->cores,
> +                           data->threads, data->maxcpus);
> +    qtest_add_data_func_full(path, data, test_plug_with_device_add,
> +                             test_data_free);
> +    g_free(path);
> +}
> +
>   int main(int argc, char **argv)
>   {
>       const char *arch = qtest_get_arch();
> @@ -168,6 +194,8 @@ int main(int argc, char **argv)
>           qtest_cb_for_every_machine(add_pseries_test_case, g_test_quick());
>       } else if (g_str_equal(arch, "s390x")) {
>           qtest_cb_for_every_machine(add_s390x_test_case, g_test_quick());
> +    } else if (g_str_equal(arch, "loongarch64")) {
> +        qtest_cb_for_every_machine(add_loongarch_test_case, g_test_quick());

There seems to be only one unversioned "virt" machine for loongarch, so 
using qtest_cb_for_every_machine() and g_str_has_prefix(mname, "virt") in 
above function seems unnecessary? I'd suggest to call 
add_loongarch_test_case() here directly instead and only add the single test 
case for the virt machine there. Or do you plan to introduce other machines 
any time soon?

>       }
>   
>       return g_test_run();
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 8a6243382a..60e16614b2 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -149,7 +149,7 @@ qtests_hppa = \
>   
>   qtests_loongarch64 = qtests_filter + \
>     (config_all_devices.has_key('CONFIG_LOONGARCH_VIRT') ? ['numa-test'] : []) + \
> -  ['boot-serial-test']
> +  ['boot-serial-test', 'cpu-plug-test']

I'd like to suggest to put tests on separate lines here (we do it for most 
of the other targets already), that will make "git blame" more useful in the 
future when the list changed a couple of times.

  Thomas