Re: [PATCH 4/7] tests/qtest/migration: Pass accelerator arguments as machine option

[Fabiano Rosas]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> The '-accel' CLI option is handler as sugar property as
> '-machine,accel='. Replace the migration tests command
> line, only using the best accelerator available (first
> hardware, then software).
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  tests/qtest/migration/framework.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/tests/qtest/migration/framework.c 
> b/tests/qtest/migration/framework.c
> index 38a0a1a5264..e567296b014 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -214,8 +214,9 @@ int migrate_start(QTestState **from, QTestState **to, 
> const char *uri,
>      const gchar *ignore_stderr;
>      g_autofree char *shmem_opts = NULL;
>      g_autofree char *shmem_path = NULL;
> -    const char *kvm_opts = NULL;
> -    const char *arch = qtest_get_arch();
> +    const char *accel_args = NULL;
> +    const MigrationTestEnv *env = migration_get_env();
> +    const char *arch = env->arch;
>      const char *memory_size;
>      const char *machine_alias, *machine_opts = "";
>      g_autofree char *machine = NULL;
> @@ -296,8 +297,15 @@ int migrate_start(QTestState **from, QTestState **to, 
> const char *uri,
>              memory_size, shmem_path);
>      }
>  
> -    if (args->use_dirty_ring) {
> -        kvm_opts = ",dirty-ring-size=4096";
> +    if (env->has_kvm) {
> +        if (args->use_dirty_ring) {
> +            accel_args = "kvm,dirty-ring-size=4096";
> +        } else {
> +            accel_args = "kvm";
> +        }
> +    } else {
> +        assert(env->has_tcg);
> +        accel_args = "tcg";
>      }

I don't think this approach works when testing across
architectures. IIUC has_kvm will be true whenever the *test* can access
/dev/kvm. Which means if we run on a x86 while testing
qemu-system-s390x, then has_kvm==true but we actually need to use TCG,
so the fallback in the QEMU binary (-accel kvm -accel tcg) will be
automatically used.

>  
>      if (!qtest_has_machine(machine_alias)) {
> @@ -311,14 +319,12 @@ int migrate_start(QTestState **from, QTestState **to, 
> const char *uri,
>  
>      g_test_message("Using machine type: %s", machine);
>  
> -    cmd_source = g_strdup_printf("-accel kvm%s -accel tcg "
> -                                 "-machine %s,%s "
> +    cmd_source = g_strdup_printf("-machine %s,%s,accel=%s "
>                                   "-name source,debug-threads=on "
>                                   "-m %s "
>                                   "-serial file:%s/src_serial "
>                                   "%s %s %s %s",
> -                                 kvm_opts ? kvm_opts : "",
> -                                 machine, machine_opts,
> +                                 machine, machine_opts, accel_args,
>                                   memory_size, tmpfs,
>                                   arch_opts ? arch_opts : "",
>                                   shmem_opts ? shmem_opts : "",
> @@ -332,15 +338,13 @@ int migrate_start(QTestState **from, QTestState **to, 
> const char *uri,
>                                       &src_state);
>      }
>  
> -    cmd_target = g_strdup_printf("-accel kvm%s -accel tcg "
> -                                 "-machine %s,%s "
> +    cmd_target = g_strdup_printf("-machine %s,%s,accel=%s "
>                                   "-name target,debug-threads=on "
>                                   "-m %s "
>                                   "-serial file:%s/dest_serial "
>                                   "-incoming %s "
>                                   "%s %s %s %s",
> -                                 kvm_opts ? kvm_opts : "",
> -                                 machine, machine_opts,
> +                                 machine, machine_opts, accel_args,
>                                   memory_size, tmpfs, uri,
>                                   arch_opts ? arch_opts : "",
>                                   shmem_opts ? shmem_opts : "",