Steven Sistare <[email protected]> writes:
> On 9/30/2025 3:59 PM, Steven Sistare wrote:
>> On 9/30/2025 3:51 PM, Fabiano Rosas wrote:
>>> Steve Sistare <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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?
>
Yeah, could be.
> - 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);
>>>> }
>>
