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

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

On 9/30/2025 3:59 PM, Steven Sistare wrote:
> 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, 

Do you prefer I send a patch with just the fix, if you have already
pulled the patches into your tree?

- Steve

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