Re: [PATCH V1 09/11] migration-test: migrate_args

[Fabiano Rosas <farosas@suse.de>]

Steve Sistare <steven.sistare@oracle.com> writes:

> Define the subroutine migrate_args to return the arguments that are
> used to exec the source or target qemu process.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  tests/qtest/migration/framework.h |  2 ++
>  tests/qtest/migration/framework.c | 64 ++++++++++++++++++++++++---------------
>  2 files changed, 41 insertions(+), 25 deletions(-)
>
> diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h
> index 7ff3187..51a8a7e 100644
> --- a/tests/qtest/migration/framework.h
> +++ b/tests/qtest/migration/framework.h
> @@ -226,6 +226,8 @@ typedef struct {
>  void wait_for_serial(const char *side);
>  void migrate_prepare_for_dirty_mem(QTestState *from);
>  void migrate_wait_for_dirty_mem(QTestState *from, QTestState *to);
> +
> +void migrate_args(char **from, char **to, const char *uri, MigrateStart *args);
>  int migrate_start(QTestState **from, QTestState **to, const char *uri,
>                    MigrateStart *args);
>  void migrate_end(QTestState *from, QTestState *to, bool test_dest);
> diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
> index 8f9e359..2dfb1ee 100644
> --- a/tests/qtest/migration/framework.c
> +++ b/tests/qtest/migration/framework.c
> @@ -258,13 +258,12 @@ static char *test_shmem_path(void)
>      return g_strdup_printf("/dev/shm/qemu-%d", getpid());
>  }
>  
> -int migrate_start(QTestState **from, QTestState **to, const char *uri,
> -                  MigrateStart *args)
> +void migrate_args(char **from, char **to, const char *uri, MigrateStart *args)
>  {
>      /* options for source and target */
>      g_autofree gchar *arch_opts = NULL;
> -    g_autofree gchar *cmd_source = NULL;
> -    g_autofree gchar *cmd_target = NULL;
> +    gchar *cmd_source = NULL;
> +    gchar *cmd_target = NULL;
>      const gchar *ignore_stderr;
>      g_autofree char *shmem_opts = NULL;
>      g_autofree char *shmem_path = NULL;
> @@ -273,23 +272,10 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>      const char *memory_size;
>      const char *machine_alias, *machine_opts = "";
>      g_autofree char *machine = NULL;
> -    const char *bootpath;
> -    g_autoptr(QList) capabilities = migrate_start_get_qmp_capabilities(args);
> +    const char *bootpath = bootfile_get();
>      g_autofree char *memory_backend = NULL;
>      const char *events;
>  
> -    if (args->use_shmem) {
> -        if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) {
> -            g_test_skip("/dev/shm is not supported");
> -            return -1;
> -        }
> -    }
> -
> -    dst_state = (QTestMigrationState) { };
> -    src_state = (QTestMigrationState) { };
> -    bootpath = bootfile_create(arch, tmpfs, args->suspend_me);
> -    src_state.suspend_me = args->suspend_me;
> -
>      if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>          memory_size = "150M";
>  
> @@ -365,7 +351,7 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>      if (!qtest_has_machine(machine_alias)) {
>          g_autofree char *msg = g_strdup_printf("machine %s not supported", machine_alias);
>          g_test_skip(msg);
> -        return -1;
> +        return;

A common pitfall is that g_test_skip() doesn't actually ends the
test. The -1 needs to be propagated up, otherwise the test will proceed
with the unsupported machine.

>      }
>  
>      machine = resolve_machine_version(machine_alias, QEMU_ENV_SRC,
> @@ -386,12 +372,6 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>                                   shmem_opts ? shmem_opts : "",
>                                   args->opts_source ? args->opts_source : "",
>                                   ignore_stderr);
> -    if (!args->only_target) {
> -        *from = qtest_init_ext(QEMU_ENV_SRC, cmd_source, capabilities, true);
> -        qtest_qmp_set_event_callback(*from,
> -                                     migrate_watch_for_events,
> -                                     &src_state);
> -    }
>  
>      /*
>       * If the monitor connection is deferred, enable events on the command line
> @@ -415,6 +395,39 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>                                   shmem_opts ? shmem_opts : "",
>                                   args->opts_target ? args->opts_target : "",
>                                   ignore_stderr);
> +
> +    *from = cmd_source;
> +    *to = cmd_target;
> +}
> +
> +int migrate_start(QTestState **from, QTestState **to, const char *uri,
> +                  MigrateStart *args)
> +{
> +    g_autofree gchar *cmd_source = NULL;
> +    g_autofree gchar *cmd_target = NULL;
> +    g_autoptr(QList) capabilities = migrate_start_get_qmp_capabilities(args);
> +
> +    if (args->use_shmem) {
> +        if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) {
> +            g_test_skip("/dev/shm is not supported");
> +            return -1;
> +        }
> +    }
> +
> +    dst_state = (QTestMigrationState) { };
> +    src_state = (QTestMigrationState) { };
> +    bootfile_create(qtest_get_arch(), tmpfs, args->suspend_me);
> +    src_state.suspend_me = args->suspend_me;
> +
> +    migrate_args(&cmd_source, &cmd_target, uri, args);
> +
> +    if (!args->only_target) {
> +        *from = qtest_init_ext(QEMU_ENV_SRC, cmd_source, capabilities, true);
> +        qtest_qmp_set_event_callback(*from,
> +                                     migrate_watch_for_events,
> +                                     &src_state);
> +    }
> +
>      if (!args->only_source) {
>          *to = qtest_init_ext(QEMU_ENV_DST, cmd_target, capabilities,
>                               !args->defer_target_connect);
> @@ -428,6 +441,7 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
>       * It's valid because QEMU has already opened this file
>       */
>      if (args->use_shmem) {
> +        g_autofree char *shmem_path = test_shmem_path();
>          unlink(shmem_path);
>      }