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

[Steven Sistare <steven.sistare@oracle.com>]

On 9/30/2025 3:51 PM, Fabiano Rosas wrote:
> 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.

Thanks.
migrate_args() will return an error code.
I'll send a V2 of this patch, and fix the call to migrate_args in patch
"cpr-exec test".

- Steve

> 
>>       }
>>   
>>       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);
>>       }