[PATCH V1 00/11] cpr-exec test

[PATCH V1 00/11] cpr-exec test

[Steve Sistare]

Add a migration test for cpr-exec mode.
Depends on the patch series "Live update: cpr-exec".

Steve Sistare (11):
  tests/qtest: export qtest_qemu_binary
  tests/qtest: qtest_qemu_args
  tests/qtest: qtest_create_test_state
  tests/qtest: qtest_qemu_spawn_func
  tests/qtest: qtest_init_after_exec
  migration-test: only_source option
  migration-test: shm path accessor
  migration-test: misc exports
  migration-test: migrate_args
  migration-test: strv parameter
  migration-test: test cpr-exec

 tests/qtest/libqtest.h                |  25 +++++++
 tests/qtest/migration/bootfile.h      |   1 +
 tests/qtest/migration/framework.h     |   8 +++
 tests/qtest/migration/migration-qmp.h |   2 +
 tests/qtest/libqtest.c                | 108 ++++++++++++++++++++----------
 tests/qtest/migration/bootfile.c      |   5 ++
 tests/qtest/migration/cpr-tests.c     | 120 ++++++++++++++++++++++++++++++++++
 tests/qtest/migration/framework.c     | 102 ++++++++++++++++++-----------
 tests/qtest/migration/migration-qmp.c |  16 +++++
 9 files changed, 317 insertions(+), 70 deletions(-)

-- 
1.8.3.1

[PATCH V1 01/11] tests/qtest: export qtest_qemu_binary

[Steve Sistare]

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 tests/qtest/libqtest.h | 9 +++++++++
 tests/qtest/libqtest.c | 2 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index b3f2e7f..6d3199f 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -48,6 +48,15 @@ QTestState *qtest_initf(const char *fmt, ...) G_GNUC_PRINTF(1, 2);
 QTestState *qtest_vinitf(const char *fmt, va_list ap) G_GNUC_PRINTF(1, 0);
 
 /**
+ * qtest_qemu_binary:
+ * @var: environment variable name
+ *
+ * Look up @var and return its value as the qemu binary path.
+ * If @var is NULL, look up  the default var name.
+ */
+const char *qtest_qemu_binary(const char *var);
+
+/**
  * qtest_init:
  * @extra_args: other arguments to pass to QEMU.  CAUTION: these
  * arguments are subject to word splitting and shell evaluation.
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index f3d4e08..6f76be1 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -357,7 +357,7 @@ void qtest_remove_abrt_handler(void *data)
     }
 }
 
-static const char *qtest_qemu_binary(const char *var)
+const char *qtest_qemu_binary(const char *var)
 {
     const char *qemu_bin;
 
-- 
1.8.3.1

[PATCH V1 02/11] tests/qtest: qtest_qemu_args

[Steve Sistare]

Define an accessor that returns all the arguments used to exec QEMU.
Collect the arguments that were passed to qtest_spawn_qemu, plus the trace
arguments that were composed inside qtest_spawn_qemu, and move them to a
new function qtest_qemu_args.

This will be needed to test the cpr-exec migration mode.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 tests/qtest/libqtest.h |  8 ++++++++
 tests/qtest/libqtest.c | 54 +++++++++++++++++++++++++++++---------------------
 2 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 6d3199f..a164f58 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -57,6 +57,14 @@ QTestState *qtest_vinitf(const char *fmt, va_list ap) G_GNUC_PRINTF(1, 0);
 const char *qtest_qemu_binary(const char *var);
 
 /**
+ * qtest_qemu_args:
+ * @extra_args: Other arguments to pass to QEMU.
+ *
+ * Return the command line used to start QEMU, sans binary.
+ */
+gchar *qtest_qemu_args(const char *extra_args);
+
+/**
  * qtest_init:
  * @extra_args: other arguments to pass to QEMU.  CAUTION: these
  * arguments are subject to word splitting and shell evaluation.
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 6f76be1..551bc8c 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -409,20 +409,12 @@ static pid_t qtest_create_process(char *cmd)
 }
 #endif /* _WIN32 */
 
-static QTestState *G_GNUC_PRINTF(2, 3) qtest_spawn_qemu(const char *qemu_bin,
-                                                        const char *fmt, ...)
+static QTestState *qtest_spawn_qemu(const char *qemu_bin, const char *args)
 {
-    va_list ap;
     QTestState *s = g_new0(QTestState, 1);
-    const char *trace = g_getenv("QTEST_TRACE");
-    g_autofree char *tracearg = trace ?
-        g_strdup_printf("-trace %s ", trace) : g_strdup("");
     g_autoptr(GString) command = g_string_new("");
 
-    va_start(ap, fmt);
-    g_string_append_printf(command, CMD_EXEC "%s %s", qemu_bin, tracearg);
-    g_string_append_vprintf(command, fmt, ap);
-    va_end(ap);
+    g_string_printf(command, CMD_EXEC "%s %s", qemu_bin, args);
 
     qtest_add_abrt_handler(kill_qemu_hook_func, s);
 
@@ -466,6 +458,33 @@ static char *qtest_socket_path(const char *suffix)
     return g_strdup_printf("%s/qtest-%d.%s", g_get_tmp_dir(), getpid(), suffix);
 }
 
+gchar *qtest_qemu_args(const char *extra_args)
+{
+    g_autofree gchar *socket_path = qtest_socket_path("sock");
+    g_autofree gchar *qmp_socket_path = qtest_socket_path("qmp");
+    const char *trace = g_getenv("QTEST_TRACE");
+    g_autofree char *tracearg = trace ? g_strdup_printf("-trace %s ", trace) :
+                                        g_strdup("");
+    gchar *args = g_strdup_printf(
+                      "%s"
+                      "-qtest unix:%s "
+                      "-qtest-log %s "
+                      "-chardev socket,path=%s,id=char0 "
+                      "-mon chardev=char0,mode=control "
+                      "-display none "
+                      "-audio none "
+                      "%s"
+                      " -accel qtest",
+
+                      tracearg,
+                      socket_path,
+                      getenv("QTEST_LOG") ? DEV_STDERR : DEV_NULL,
+                      qmp_socket_path,
+                      extra_args ?: "");
+
+    return args;
+}
+
 static QTestState *qtest_init_internal(const char *qemu_bin,
                                        const char *extra_args,
                                        bool do_connect)
@@ -474,6 +493,7 @@ static QTestState *qtest_init_internal(const char *qemu_bin,
     int sock, qmpsock, i;
     g_autofree gchar *socket_path = qtest_socket_path("sock");
     g_autofree gchar *qmp_socket_path = qtest_socket_path("qmp");
+    g_autofree gchar *args = qtest_qemu_args(extra_args);
 
     /*
      * It's possible that if an earlier test run crashed it might
@@ -488,19 +508,7 @@ static QTestState *qtest_init_internal(const char *qemu_bin,
     sock = init_socket(socket_path);
     qmpsock = init_socket(qmp_socket_path);
 
-    s = qtest_spawn_qemu(qemu_bin,
-                         "-qtest unix:%s "
-                         "-qtest-log %s "
-                         "-chardev socket,path=%s,id=char0 "
-                         "-mon chardev=char0,mode=control "
-                         "-display none "
-                         "-audio none "
-                         "%s"
-                         " -accel qtest",
-                         socket_path,
-                         getenv("QTEST_LOG") ? DEV_STDERR : DEV_NULL,
-                         qmp_socket_path,
-                         extra_args ?: "");
+    s = qtest_spawn_qemu(qemu_bin, args);
 
     qtest_client_set_rx_handler(s, qtest_client_socket_recv_line);
     qtest_client_set_tx_handler(s, qtest_client_socket_send);
-- 
1.8.3.1

[PATCH V1 03/11] tests/qtest: qtest_create_test_state

[Steve Sistare]

Refactor qtest_spawn_qemu and create a subroutine to create a QTestState
object, to be used in a subsequent patch.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 tests/qtest/libqtest.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 551bc8c..3fa9317 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -409,22 +409,29 @@ static pid_t qtest_create_process(char *cmd)
 }
 #endif /* _WIN32 */
 
-static QTestState *qtest_spawn_qemu(const char *qemu_bin, const char *args)
+static QTestState *qtest_create_test_state(int pid)
 {
     QTestState *s = g_new0(QTestState, 1);
+
+    s->qemu_pid = pid;
+    qtest_add_abrt_handler(kill_qemu_hook_func, s);
+    return s;
+}
+
+static QTestState *qtest_spawn_qemu(const char *qemu_bin, const char *args)
+{
+    int pid;
     g_autoptr(GString) command = g_string_new("");
 
     g_string_printf(command, CMD_EXEC "%s %s", qemu_bin, args);
 
-    qtest_add_abrt_handler(kill_qemu_hook_func, s);
-
     if (!silence_spawn_log) {
         g_test_message("starting QEMU: %s", command->str);
     }
 
 #ifndef _WIN32
-    s->qemu_pid = fork();
-    if (s->qemu_pid == 0) {
+    pid = fork();
+    if (pid == 0) {
 #ifdef __linux__
         /*
          * Although we register a ABRT handler to kill off QEMU
@@ -447,10 +454,10 @@ static QTestState *qtest_spawn_qemu(const char *qemu_bin, const char *args)
         exit(1);
     }
 #else
-    s->qemu_pid = qtest_create_process(command->str);
+    pid = qtest_create_process(command->str);
 #endif /* _WIN32 */
 
-    return s;
+    return qtest_create_test_state(pid);
 }
 
 static char *qtest_socket_path(const char *suffix)
-- 
1.8.3.1

[PATCH V1 04/11] tests/qtest: qtest_qemu_spawn_func

[Steve Sistare]

Allow the qtest_qemu_spawn caller to pass the function to be called
to perform the spawn.  The opaque argument is needed by a new spawn
function in a subsequent patch.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 tests/qtest/libqtest.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 3fa9317..d97144e 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -418,7 +418,8 @@ static QTestState *qtest_create_test_state(int pid)
     return s;
 }
 
-static QTestState *qtest_spawn_qemu(const char *qemu_bin, const char *args)
+static QTestState *qtest_spawn_qemu(const char *qemu_bin, const char *args,
+                                    void *opaque)
 {
     int pid;
     g_autoptr(GString) command = g_string_new("");
@@ -492,9 +493,15 @@ gchar *qtest_qemu_args(const char *extra_args)
     return args;
 }
 
+typedef QTestState *(*qtest_qemu_spawn_func)(const char *qemu_bin,
+                                             const char *extra_args,
+                                             void *opaque);
+
 static QTestState *qtest_init_internal(const char *qemu_bin,
                                        const char *extra_args,
-                                       bool do_connect)
+                                       bool do_connect,
+                                       qtest_qemu_spawn_func spawn,
+                                       void *opaque)
 {
     QTestState *s;
     int sock, qmpsock, i;
@@ -515,7 +522,7 @@ static QTestState *qtest_init_internal(const char *qemu_bin,
     sock = init_socket(socket_path);
     qmpsock = init_socket(qmp_socket_path);
 
-    s = qtest_spawn_qemu(qemu_bin, args);
+    s = spawn(qemu_bin, args, opaque);
 
     qtest_client_set_rx_handler(s, qtest_client_socket_recv_line);
     qtest_client_set_tx_handler(s, qtest_client_socket_send);
@@ -570,7 +577,8 @@ void qtest_connect(QTestState *s)
 
 QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
 {
-    return qtest_init_internal(qtest_qemu_binary(NULL), extra_args, true);
+    return qtest_init_internal(qtest_qemu_binary(NULL), extra_args, true,
+                               qtest_spawn_qemu, NULL);
 }
 
 void qtest_qmp_handshake(QTestState *s, QList *capabilities)
@@ -593,7 +601,7 @@ QTestState *qtest_init_ext(const char *var, const char *extra_args,
                            QList *capabilities, bool do_connect)
 {
     QTestState *s = qtest_init_internal(qtest_qemu_binary(var), extra_args,
-                                        do_connect);
+                                        do_connect, qtest_spawn_qemu, NULL);
 
     if (do_connect) {
         qtest_qmp_handshake(s, capabilities);
-- 
1.8.3.1

[PATCH V1 05/11] tests/qtest: qtest_init_after_exec

[Steve Sistare]

Define a function to create a QTestState object representing the state
of QEMU after old QEMU exec's new QEMU.  This is needed for testing
the cpr-exec migration mode.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 tests/qtest/libqtest.h |  8 ++++++++
 tests/qtest/libqtest.c | 19 +++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index a164f58..ce6b9b0 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -57,6 +57,14 @@ QTestState *qtest_vinitf(const char *fmt, va_list ap) G_GNUC_PRINTF(1, 0);
 const char *qtest_qemu_binary(const char *var);
 
 /**
+ * qtest_init_after_exec:
+ * @from: the previous QEMU state
+ *
+ * Return a test state representing new QEMU after @from exec's it.
+ */
+QTestState *qtest_init_after_exec(QTestState *from);
+
+/**
  * qtest_qemu_args:
  * @extra_args: Other arguments to pass to QEMU.
  *
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index d97144e..3522d75 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -615,6 +615,25 @@ QTestState *qtest_init_ext(const char *var, const char *extra_args,
     return s;
 }
 
+static QTestState *qtest_attach_qemu(const char *qemu_bin,
+                                     const char *extra_args,
+                                     void *opaque)
+{
+    int pid = *(int *)opaque;
+    return qtest_create_test_state(pid);
+}
+
+QTestState *qtest_init_after_exec(QTestState *from)
+{
+    void *opaque = (void *)&from->qemu_pid;
+    QTestState *s;
+
+    s = qtest_init_internal(NULL, NULL, true, qtest_attach_qemu, opaque);
+    from->qemu_pid = -1;
+    qtest_qmp_handshake(s, NULL);
+    return s;
+}
+
 QTestState *qtest_init(const char *extra_args)
 {
     return qtest_init_ext(NULL, extra_args, NULL, true);
-- 
1.8.3.1

[PATCH V1 06/11] migration-test: only_source option

[Steve Sistare]

Add the only_source option, analogous to only_target.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 tests/qtest/migration/framework.h |  2 ++
 tests/qtest/migration/framework.c | 24 +++++++++++++++---------
 2 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h
index 01e425e..f1bb9d4 100644
--- a/tests/qtest/migration/framework.h
+++ b/tests/qtest/migration/framework.h
@@ -103,6 +103,8 @@ typedef struct {
      */
     bool hide_stderr;
     bool use_shmem;
+    /* only launch the source process */
+    bool only_source;
     /* only launch the target process */
     bool only_target;
     /* Use dirty ring if true; dirty logging otherwise */
diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
index 407c902..9564293 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -234,7 +234,7 @@ static void migrate_start_set_capabilities(QTestState *from, QTestState *to,
      * to mimic as closer as that.
      */
     migrate_set_capability(from, "events", true);
-    if (!args->defer_target_connect) {
+    if (!args->defer_target_connect && to) {
         migrate_set_capability(to, "events", true);
     }
 
@@ -246,8 +246,10 @@ static void migrate_start_set_capabilities(QTestState *from, QTestState *to,
     if (args->caps[MIGRATION_CAPABILITY_MULTIFD]) {
         migrate_set_parameter_int(from, "multifd-channels",
                                   MULTIFD_TEST_CHANNELS);
-        migrate_set_parameter_int(to, "multifd-channels",
-                                  MULTIFD_TEST_CHANNELS);
+        if (to) {
+            migrate_set_parameter_int(to, "multifd-channels",
+                                      MULTIFD_TEST_CHANNELS);
+        }
     }
 
     return;
@@ -410,11 +412,13 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
                                  shmem_opts ? shmem_opts : "",
                                  args->opts_target ? args->opts_target : "",
                                  ignore_stderr);
-    *to = qtest_init_ext(QEMU_ENV_DST, cmd_target, capabilities,
-                         !args->defer_target_connect);
-    qtest_qmp_set_event_callback(*to,
-                                 migrate_watch_for_events,
-                                 &dst_state);
+    if (!args->only_source) {
+        *to = qtest_init_ext(QEMU_ENV_DST, cmd_target, capabilities,
+                             !args->defer_target_connect);
+        qtest_qmp_set_event_callback(*to,
+                                     migrate_watch_for_events,
+                                     &dst_state);
+    }
 
     /*
      * Remove shmem file immediately to avoid memory leak in test failed case.
@@ -424,7 +428,9 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
         unlink(shmem_path);
     }
 
-    migrate_start_set_capabilities(*from, *to, args);
+    migrate_start_set_capabilities(*from,
+                                   args->only_source ? NULL : *to,
+                                   args);
 
     return 0;
 }
-- 
1.8.3.1

[PATCH V1 07/11] migration-test: shm path accessor

[Steve Sistare]

Define an accessor for the shm path.  It will be referenced from
multiple sites in a subsequent patch.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 tests/qtest/migration/framework.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
index 9564293..9d04f36 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -255,6 +255,11 @@ static void migrate_start_set_capabilities(QTestState *from, QTestState *to,
     return;
 }
 
+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)
 {
@@ -342,7 +347,7 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
     }
 
     if (args->use_shmem) {
-        shmem_path = g_strdup_printf("/dev/shm/qemu-%d", getpid());
+        shmem_path = test_shmem_path();
         shmem_opts = g_strdup_printf(
             "-object memory-backend-file,id=mem0,size=%s"
             ",mem-path=%s,share=on -numa node,memdev=mem0",
-- 
1.8.3.1

[PATCH V1 08/11] migration-test: misc exports

[Steve Sistare]

Export misc definitions needed by the cpr-exec test.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 tests/qtest/migration/bootfile.h  | 1 +
 tests/qtest/migration/framework.h | 4 ++++
 tests/qtest/migration/bootfile.c  | 5 +++++
 tests/qtest/migration/framework.c | 7 +++++--
 4 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/migration/bootfile.h b/tests/qtest/migration/bootfile.h
index 6d6a673..96e784b 100644
--- a/tests/qtest/migration/bootfile.h
+++ b/tests/qtest/migration/bootfile.h
@@ -35,5 +35,6 @@
 
 void bootfile_delete(void);
 char *bootfile_create(const char *arch, const char *dir, bool suspend_me);
+char *bootfile_get(void);
 
 #endif /* BOOTFILE_H */
diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h
index f1bb9d4..7ff3187 100644
--- a/tests/qtest/migration/framework.h
+++ b/tests/qtest/migration/framework.h
@@ -18,6 +18,9 @@
 #define FILE_TEST_OFFSET 0x1000
 #define FILE_TEST_MARKER 'X'
 
+#define QEMU_ENV_SRC "QTEST_QEMU_BINARY_SRC"
+#define QEMU_ENV_DST "QTEST_QEMU_BINARY_DST"
+
 typedef struct MigrationTestEnv {
     bool has_kvm;
     bool has_tcg;
@@ -237,6 +240,7 @@ void *migrate_hook_start_precopy_tcp_multifd_common(QTestState *from,
 
 typedef struct QTestMigrationState QTestMigrationState;
 QTestMigrationState *get_src(void);
+QTestMigrationState *get_dst(void);
 
 #ifdef CONFIG_GNUTLS
 void migration_test_add_tls(MigrationTestEnv *env);
diff --git a/tests/qtest/migration/bootfile.c b/tests/qtest/migration/bootfile.c
index fac059d..479c432 100644
--- a/tests/qtest/migration/bootfile.c
+++ b/tests/qtest/migration/bootfile.c
@@ -68,3 +68,8 @@ char *bootfile_create(const char *arch, const char *dir, bool suspend_me)
 
     return bootpath;
 }
+
+char *bootfile_get(void)
+{
+    return bootpath;
+}
diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
index 9d04f36..8f9e359 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -28,8 +28,6 @@
 
 
 #define QEMU_VM_FILE_MAGIC 0x5145564d
-#define QEMU_ENV_SRC "QTEST_QEMU_BINARY_SRC"
-#define QEMU_ENV_DST "QTEST_QEMU_BINARY_DST"
 #define MULTIFD_TEST_CHANNELS 4
 
 unsigned start_address;
@@ -1005,6 +1003,11 @@ QTestMigrationState *get_src(void)
     return &src_state;
 }
 
+QTestMigrationState *get_dst(void)
+{
+    return &dst_state;
+}
+
 MigrationTestEnv *migration_get_env(void)
 {
     static MigrationTestEnv *env;
-- 
1.8.3.1

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

[Steve Sistare]

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

[PATCH V1 10/11] migration-test: strv parameter

[Steve Sistare]

Define migrate_set_parameter_strv.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 tests/qtest/migration/migration-qmp.h |  2 ++
 tests/qtest/migration/migration-qmp.c | 16 ++++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/tests/qtest/migration/migration-qmp.h b/tests/qtest/migration/migration-qmp.h
index faa8181..44482d2 100644
--- a/tests/qtest/migration/migration-qmp.h
+++ b/tests/qtest/migration/migration-qmp.h
@@ -34,6 +34,8 @@ void read_blocktime(QTestState *who);
 void wait_for_migration_pass(QTestState *who, QTestMigrationState *src_state);
 void migrate_set_parameter_str(QTestState *who, const char *parameter,
                                const char *value);
+void migrate_set_parameter_strv(QTestState *who, const char *parameter,
+                                char **strv);
 void migrate_set_parameter_bool(QTestState *who, const char *parameter,
                                 int value);
 void migrate_ensure_non_converge(QTestState *who);
diff --git a/tests/qtest/migration/migration-qmp.c b/tests/qtest/migration/migration-qmp.c
index 66dd369..c803fce 100644
--- a/tests/qtest/migration/migration-qmp.c
+++ b/tests/qtest/migration/migration-qmp.c
@@ -442,6 +442,22 @@ void migrate_set_parameter_str(QTestState *who, const char *parameter,
     migrate_check_parameter_str(who, parameter, value);
 }
 
+void migrate_set_parameter_strv(QTestState *who, const char *parameter,
+                                char **strv)
+{
+    g_autofree char *args = g_strjoinv("\",\"", strv);
+    g_autoptr(GString) value = g_string_new("");
+    g_autofree char *command = NULL;
+
+    g_string_printf(value, "\"%s\"", args);
+
+    command = g_strdup_printf("{ 'execute': 'migrate-set-parameters',"
+                              "'arguments': { %%s: [ %s ]}}",
+                              value->str);
+
+    qtest_qmp_assert_success(who, command, parameter);
+}
+
 static long long migrate_get_parameter_bool(QTestState *who,
                                             const char *parameter)
 {
-- 
1.8.3.1

[PATCH V1 11/11] migration-test: test cpr-exec

[Steve Sistare]

Add a test for the cpr-exec migration mode.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 tests/qtest/migration/cpr-tests.c | 120 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)

diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c
index 5e764a6..f33af76 100644
--- a/tests/qtest/migration/cpr-tests.c
+++ b/tests/qtest/migration/cpr-tests.c
@@ -110,6 +110,125 @@ static void test_mode_transfer_defer(void)
     test_mode_transfer_common(true);
 }
 
+static void set_cpr_exec_args(QTestState *who, MigrateCommon *args)
+{
+    g_autofree char *qtest_from_args = NULL;
+    g_autofree char *from_args = NULL;
+    g_autofree char *to_args = NULL;
+    g_autofree char *exec_args = NULL;
+    g_auto(GStrv) argv = NULL;
+    char *from_str, *src, *dst;
+
+    args->start.hide_stderr = false;    /* omit redirection word from args */
+    migrate_args(&from_args, &to_args, args->listen_uri, &args->start);
+    qtest_from_args = qtest_qemu_args(from_args);
+
+    /* De-dup spaces so argv does not contain empty strings */
+    from_str = src = dst = g_strstrip(qtest_from_args);
+    do {
+        if (*src != ' ' || src[-1] != ' ') {
+            *dst++ = *src;
+        }
+    } while (*src++);
+
+    exec_args = g_strconcat(qtest_qemu_binary(QEMU_ENV_SRC),
+                            " -incoming defer ", from_str, NULL);
+    argv = g_strsplit(exec_args, " ", -1);
+    migrate_set_parameter_strv(who, "cpr-exec-command", argv);
+}
+
+static void wait_for_migration_event(QTestState *who, const char *waitfor)
+{
+    QDict *rsp, *data;
+    char *status;
+    bool done = false;
+
+    while (!done) {
+        rsp = qtest_qmp_eventwait_ref(who, "MIGRATION");
+        g_assert(qdict_haskey(rsp, "data"));
+        data = qdict_get_qdict(rsp, "data");
+        g_assert(qdict_haskey(data, "status"));
+        status = g_strdup(qdict_get_str(data, "status"));
+        g_assert(strcmp(status, "failed"));
+        done = !strcmp(status, waitfor);
+        qobject_unref(rsp);
+    }
+}
+
+static void test_cpr_exec(MigrateCommon *args)
+{
+    QTestState *from, *to;
+    void *data_hook = NULL;
+    g_autofree char *connect_uri = g_strdup(args->connect_uri);
+    g_autofree char *filename = g_strdup_printf("%s/%s", tmpfs,
+                                                FILE_TEST_FILENAME);
+
+
+    if (migrate_start(&from, NULL, args->listen_uri, &args->start)) {
+        return;
+    }
+
+    /* Source and dest never run concurrently */
+    g_assert_false(args->live);
+
+    if (args->start_hook) {
+        data_hook = args->start_hook(from, NULL);
+    }
+
+    wait_for_serial("src_serial");
+    set_cpr_exec_args(from, args);
+    migrate_set_capability(from, "events", true);
+    migrate_qmp(from, NULL, connect_uri, NULL, "{}");
+    wait_for_migration_event(from, "completed");
+
+    to = qtest_init_after_exec(from);
+
+    qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming',"
+                             "  'arguments': { "
+                             "      'channels': [ { 'channel-type': 'main',"
+                             "      'addr': { 'transport': 'file',"
+                             "                'filename': %s,"
+                             "                'offset': 0  } } ] } }",
+                             filename);
+    wait_for_migration_complete(to);
+
+    wait_for_resume(to, get_dst());
+    /* Device on target is still named src_serial because args do not change */
+    wait_for_serial("src_serial");
+
+    if (args->end_hook) {
+        args->end_hook(from, to, data_hook);
+    }
+
+    migrate_end(from, to, args->result == MIG_TEST_SUCCEED);
+}
+
+static void *test_mode_exec_start(QTestState *from, QTestState *to)
+{
+    assert(!to);
+    migrate_set_parameter_str(from, "mode", "cpr-exec");
+    return NULL;
+}
+
+static void test_mode_exec(void)
+{
+    g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
+                                           FILE_TEST_FILENAME);
+    g_autofree char *listen_uri = g_strdup_printf("defer");
+
+    MigrateCommon args = {
+        .start.only_source = true,
+        .start.opts_source = "-machine aux-ram-share=on -nodefaults",
+        .start.memory_backend = "-object memory-backend-memfd,id=pc.ram,size=%s"
+                                " -machine memory-backend=pc.ram",
+        .connect_uri = uri,
+        .listen_uri = listen_uri,
+        .start_hook = test_mode_exec_start,
+    };
+
+    test_cpr_exec(&args);
+}
+
 void migration_test_add_cpr(MigrationTestEnv *env)
 {
     tmpfs = env->tmpfs;
@@ -132,5 +251,6 @@ void migration_test_add_cpr(MigrationTestEnv *env)
         migration_test_add("/migration/mode/transfer", test_mode_transfer);
         migration_test_add("/migration/mode/transfer/defer",
                            test_mode_transfer_defer);
+        migration_test_add("/migration/mode/exec", test_mode_exec);
     }
 }
-- 
1.8.3.1

Re: [PATCH V1 01/11] tests/qtest: export qtest_qemu_binary

[Fabiano Rosas]

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

> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  tests/qtest/libqtest.h | 9 +++++++++
>  tests/qtest/libqtest.c | 2 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
> index b3f2e7f..6d3199f 100644
> --- a/tests/qtest/libqtest.h
> +++ b/tests/qtest/libqtest.h
> @@ -48,6 +48,15 @@ QTestState *qtest_initf(const char *fmt, ...) G_GNUC_PRINTF(1, 2);
>  QTestState *qtest_vinitf(const char *fmt, va_list ap) G_GNUC_PRINTF(1, 0);
>  
>  /**
> + * qtest_qemu_binary:
> + * @var: environment variable name
> + *
> + * Look up @var and return its value as the qemu binary path.
> + * If @var is NULL, look up  the default var name.
> + */
> +const char *qtest_qemu_binary(const char *var);
> +
> +/**
>   * qtest_init:
>   * @extra_args: other arguments to pass to QEMU.  CAUTION: these
>   * arguments are subject to word splitting and shell evaluation.
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index f3d4e08..6f76be1 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -357,7 +357,7 @@ void qtest_remove_abrt_handler(void *data)
>      }
>  }
>  
> -static const char *qtest_qemu_binary(const char *var)
> +const char *qtest_qemu_binary(const char *var)
>  {
>      const char *qemu_bin;

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH V1 02/11] tests/qtest: qtest_qemu_args

[Fabiano Rosas]

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

> Define an accessor that returns all the arguments used to exec QEMU.
> Collect the arguments that were passed to qtest_spawn_qemu, plus the trace
> arguments that were composed inside qtest_spawn_qemu, and move them to a
> new function qtest_qemu_args.
>
> This will be needed to test the cpr-exec migration mode.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH V1 03/11] tests/qtest: qtest_create_test_state

[Fabiano Rosas]

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

> Refactor qtest_spawn_qemu and create a subroutine to create a QTestState
> object, to be used in a subsequent patch.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  tests/qtest/libqtest.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index 551bc8c..3fa9317 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -409,22 +409,29 @@ static pid_t qtest_create_process(char *cmd)
>  }
>  #endif /* _WIN32 */
>  
> -static QTestState *qtest_spawn_qemu(const char *qemu_bin, const char *args)
> +static QTestState *qtest_create_test_state(int pid)
>  {
>      QTestState *s = g_new0(QTestState, 1);
> +
> +    s->qemu_pid = pid;
> +    qtest_add_abrt_handler(kill_qemu_hook_func, s);
> +    return s;
> +}
> +
> +static QTestState *qtest_spawn_qemu(const char *qemu_bin, const char *args)
> +{
> +    int pid;
>      g_autoptr(GString) command = g_string_new("");
>  
>      g_string_printf(command, CMD_EXEC "%s %s", qemu_bin, args);
>  
> -    qtest_add_abrt_handler(kill_qemu_hook_func, s);
> -
>      if (!silence_spawn_log) {
>          g_test_message("starting QEMU: %s", command->str);
>      }
>  
>  #ifndef _WIN32
> -    s->qemu_pid = fork();
> -    if (s->qemu_pid == 0) {
> +    pid = fork();
> +    if (pid == 0) {
>  #ifdef __linux__
>          /*
>           * Although we register a ABRT handler to kill off QEMU
> @@ -447,10 +454,10 @@ static QTestState *qtest_spawn_qemu(const char *qemu_bin, const char *args)
>          exit(1);
>      }
>  #else
> -    s->qemu_pid = qtest_create_process(command->str);
> +    pid = qtest_create_process(command->str);
>  #endif /* _WIN32 */
>  
> -    return s;
> +    return qtest_create_test_state(pid);
>  }
>  
>  static char *qtest_socket_path(const char *suffix)

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH V1 04/11] tests/qtest: qtest_qemu_spawn_func

[Fabiano Rosas]

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

> Allow the qtest_qemu_spawn caller to pass the function to be called
> to perform the spawn.  The opaque argument is needed by a new spawn
> function in a subsequent patch.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH V1 05/11] tests/qtest: qtest_init_after_exec

[Fabiano Rosas]

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

> Define a function to create a QTestState object representing the state
> of QEMU after old QEMU exec's new QEMU.  This is needed for testing
> the cpr-exec migration mode.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

> ---
>  tests/qtest/libqtest.h |  8 ++++++++
>  tests/qtest/libqtest.c | 19 +++++++++++++++++++
>  2 files changed, 27 insertions(+)
>
> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
> index a164f58..ce6b9b0 100644
> --- a/tests/qtest/libqtest.h
> +++ b/tests/qtest/libqtest.h
> @@ -57,6 +57,14 @@ QTestState *qtest_vinitf(const char *fmt, va_list ap) G_GNUC_PRINTF(1, 0);
>  const char *qtest_qemu_binary(const char *var);
>  
>  /**
> + * qtest_init_after_exec:
> + * @from: the previous QEMU state

qts is the term used in libqtest. I'll amend it myself.

> + *
> + * Return a test state representing new QEMU after @from exec's it.
> + */
> +QTestState *qtest_init_after_exec(QTestState *from);
> +
> +/**
>   * qtest_qemu_args:
>   * @extra_args: Other arguments to pass to QEMU.
>   *
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index d97144e..3522d75 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -615,6 +615,25 @@ QTestState *qtest_init_ext(const char *var, const char *extra_args,
>      return s;
>  }
>  
> +static QTestState *qtest_attach_qemu(const char *qemu_bin,
> +                                     const char *extra_args,
> +                                     void *opaque)
> +{
> +    int pid = *(int *)opaque;
> +    return qtest_create_test_state(pid);
> +}
> +
> +QTestState *qtest_init_after_exec(QTestState *from)
> +{
> +    void *opaque = (void *)&from->qemu_pid;
> +    QTestState *s;
> +
> +    s = qtest_init_internal(NULL, NULL, true, qtest_attach_qemu, opaque);
> +    from->qemu_pid = -1;
> +    qtest_qmp_handshake(s, NULL);
> +    return s;
> +}
> +
>  QTestState *qtest_init(const char *extra_args)
>  {
>      return qtest_init_ext(NULL, extra_args, NULL, true);

Re: [PATCH V1 06/11] migration-test: only_source option

[Fabiano Rosas]

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

> Add the only_source option, analogous to only_target.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH V1 07/11] migration-test: shm path accessor

[Fabiano Rosas]

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

> Define an accessor for the shm path.  It will be referenced from
> multiple sites in a subsequent patch.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

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

[Fabiano Rosas]

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

I've been thinking of moving QTestState inside QTestMigrationState, so
these variables are not loosely kept without a accompanying
QTestState. When implementing ping-pong tests the current code got a bit
out of hand. Also the "src_serial" problem later on would be resolved by
a new qtms->serial.

What's stopping me at the moment is the amount of churn. If you spot a
better solution let be know.

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

Reviewed-by: Fabiano Rosas <farosas@suse.de>

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

[Steven Sistare]

On 9/29/2025 4:32 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;
>>       }
>>   
>>       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;
> 
> I've been thinking of moving QTestState inside QTestMigrationState, so
> these variables are not loosely kept without a accompanying
> QTestState. When implementing ping-pong tests the current code got a bit
> out of hand. Also the "src_serial" problem later on would be resolved by
> a new qtms->serial.
> 
> What's stopping me at the moment is the amount of churn. If you spot a
> better solution let be know.

I had the same thought, or having QTestMigrationState and QTestState point to
each other.  Perhaps the latter would cause less churn.

Thanks very much for reviewing my patches!

- Steve

> 
>> +
>> +    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);
>>       }
> 
> Reviewed-by: Fabiano Rosas <farosas@suse.de>

Re: [PATCH V1 00/11] cpr-exec test

[Fabiano Rosas]

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

> Add a migration test for cpr-exec mode.
> Depends on the patch series "Live update: cpr-exec".
>
> Steve Sistare (11):
>   tests/qtest: export qtest_qemu_binary
>   tests/qtest: qtest_qemu_args
>   tests/qtest: qtest_create_test_state
>   tests/qtest: qtest_qemu_spawn_func
>   tests/qtest: qtest_init_after_exec
>   migration-test: only_source option
>   migration-test: shm path accessor
>   migration-test: misc exports
>   migration-test: migrate_args
>   migration-test: strv parameter
>   migration-test: test cpr-exec
>
>  tests/qtest/libqtest.h                |  25 +++++++
>  tests/qtest/migration/bootfile.h      |   1 +
>  tests/qtest/migration/framework.h     |   8 +++
>  tests/qtest/migration/migration-qmp.h |   2 +
>  tests/qtest/libqtest.c                | 108 ++++++++++++++++++++----------
>  tests/qtest/migration/bootfile.c      |   5 ++
>  tests/qtest/migration/cpr-tests.c     | 120 ++++++++++++++++++++++++++++++++++
>  tests/qtest/migration/framework.c     | 102 ++++++++++++++++++-----------
>  tests/qtest/migration/migration-qmp.c |  16 +++++
>  9 files changed, 317 insertions(+), 70 deletions(-)

Queued up to patch 10 to qtest-next.

Re: [PATCH V1 11/11] migration-test: test cpr-exec

[Peter Xu]

On Fri, Sep 19, 2025 at 07:12:33AM -0700, Steve Sistare wrote:
> Add a test for the cpr-exec migration mode.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Looks good, only some nitpicks or pure questions below.

> ---
>  tests/qtest/migration/cpr-tests.c | 120 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 120 insertions(+)
> 
> diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c
> index 5e764a6..f33af76 100644
> --- a/tests/qtest/migration/cpr-tests.c
> +++ b/tests/qtest/migration/cpr-tests.c
> @@ -110,6 +110,125 @@ static void test_mode_transfer_defer(void)
>      test_mode_transfer_common(true);
>  }
>  
> +static void set_cpr_exec_args(QTestState *who, MigrateCommon *args)
> +{
> +    g_autofree char *qtest_from_args = NULL;
> +    g_autofree char *from_args = NULL;
> +    g_autofree char *to_args = NULL;
> +    g_autofree char *exec_args = NULL;
> +    g_auto(GStrv) argv = NULL;
> +    char *from_str, *src, *dst;
> +
> +    args->start.hide_stderr = false;    /* omit redirection word from args */

It's default off, right?  Could I request for some more explanations?

Could we also set it in test_mode_exec() directly if needed?

> +    migrate_args(&from_args, &to_args, args->listen_uri, &args->start);
> +    qtest_from_args = qtest_qemu_args(from_args);
> +
> +    /* De-dup spaces so argv does not contain empty strings */
> +    from_str = src = dst = g_strstrip(qtest_from_args);
> +    do {
> +        if (*src != ' ' || src[-1] != ' ') {
> +            *dst++ = *src;
> +        }
> +    } while (*src++);

Pure ask.. when will empty string be present?

> +
> +    exec_args = g_strconcat(qtest_qemu_binary(QEMU_ENV_SRC),

Should this be QEMU_ENV_DST?

OTOH, we can use migration_get_env()->qemu_dst to reduce referencing global
vars.

> +                            " -incoming defer ", from_str, NULL);
> +    argv = g_strsplit(exec_args, " ", -1);
> +    migrate_set_parameter_strv(who, "cpr-exec-command", argv);
> +}
> +
> +static void wait_for_migration_event(QTestState *who, const char *waitfor)
> +{
> +    QDict *rsp, *data;
> +    char *status;
> +    bool done = false;
> +
> +    while (!done) {
> +        rsp = qtest_qmp_eventwait_ref(who, "MIGRATION");
> +        g_assert(qdict_haskey(rsp, "data"));
> +        data = qdict_get_qdict(rsp, "data");
> +        g_assert(qdict_haskey(data, "status"));
> +        status = g_strdup(qdict_get_str(data, "status"));
> +        g_assert(strcmp(status, "failed"));
> +        done = !strcmp(status, waitfor);
> +        qobject_unref(rsp);
> +    }
> +}
> +
> +static void test_cpr_exec(MigrateCommon *args)
> +{
> +    QTestState *from, *to;
> +    void *data_hook = NULL;
> +    g_autofree char *connect_uri = g_strdup(args->connect_uri);
> +    g_autofree char *filename = g_strdup_printf("%s/%s", tmpfs,
> +                                                FILE_TEST_FILENAME);
> +
> +

Newline can be dropped.

> +    if (migrate_start(&from, NULL, args->listen_uri, &args->start)) {
> +        return;
> +    }
> +
> +    /* Source and dest never run concurrently */
> +    g_assert_false(args->live);
> +
> +    if (args->start_hook) {
> +        data_hook = args->start_hook(from, NULL);
> +    }
> +
> +    wait_for_serial("src_serial");
> +    set_cpr_exec_args(from, args);
> +    migrate_set_capability(from, "events", true);
> +    migrate_qmp(from, NULL, connect_uri, NULL, "{}");
> +    wait_for_migration_event(from, "completed");
> +
> +    to = qtest_init_after_exec(from);
> +
> +    qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming',"
> +                             "  'arguments': { "
> +                             "      'channels': [ { 'channel-type': 'main',"
> +                             "      'addr': { 'transport': 'file',"
> +                             "                'filename': %s,"
> +                             "                'offset': 0  } } ] } }",
> +                             filename);
> +    wait_for_migration_complete(to);
> +
> +    wait_for_resume(to, get_dst());
> +    /* Device on target is still named src_serial because args do not change */
> +    wait_for_serial("src_serial");
> +
> +    if (args->end_hook) {
> +        args->end_hook(from, to, data_hook);
> +    }
> +
> +    migrate_end(from, to, args->result == MIG_TEST_SUCCEED);
> +}
> +
> +static void *test_mode_exec_start(QTestState *from, QTestState *to)
> +{
> +    assert(!to);
> +    migrate_set_parameter_str(from, "mode", "cpr-exec");
> +    return NULL;
> +}
> +
> +static void test_mode_exec(void)
> +{
> +    g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
> +                                           FILE_TEST_FILENAME);
> +    g_autofree char *listen_uri = g_strdup_printf("defer");
> +
> +    MigrateCommon args = {
> +        .start.only_source = true,
> +        .start.opts_source = "-machine aux-ram-share=on -nodefaults",
> +        .start.memory_backend = "-object memory-backend-memfd,id=pc.ram,size=%s"
> +                                " -machine memory-backend=pc.ram",
> +        .connect_uri = uri,
> +        .listen_uri = listen_uri,
> +        .start_hook = test_mode_exec_start,
> +    };
> +
> +    test_cpr_exec(&args);
> +}
> +
>  void migration_test_add_cpr(MigrationTestEnv *env)
>  {
>      tmpfs = env->tmpfs;
> @@ -132,5 +251,6 @@ void migration_test_add_cpr(MigrationTestEnv *env)
>          migration_test_add("/migration/mode/transfer", test_mode_transfer);
>          migration_test_add("/migration/mode/transfer/defer",
>                             test_mode_transfer_defer);
> +        migration_test_add("/migration/mode/exec", test_mode_exec);
>      }
>  }
> -- 
> 1.8.3.1
> 

-- 
Peter Xu

Re: [PATCH V1 11/11] migration-test: test cpr-exec

[Steven Sistare]

On 9/30/2025 1:08 PM, Peter Xu wrote:
> On Fri, Sep 19, 2025 at 07:12:33AM -0700, Steve Sistare wrote:
>> Add a test for the cpr-exec migration mode.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> 
> Looks good, only some nitpicks or pure questions below.
> 
>> ---
>>   tests/qtest/migration/cpr-tests.c | 120 ++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 120 insertions(+)
>>
>> diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c
>> index 5e764a6..f33af76 100644
>> --- a/tests/qtest/migration/cpr-tests.c
>> +++ b/tests/qtest/migration/cpr-tests.c
>> @@ -110,6 +110,125 @@ static void test_mode_transfer_defer(void)
>>       test_mode_transfer_common(true);
>>   }
>>   
>> +static void set_cpr_exec_args(QTestState *who, MigrateCommon *args)
>> +{
>> +    g_autofree char *qtest_from_args = NULL;
>> +    g_autofree char *from_args = NULL;
>> +    g_autofree char *to_args = NULL;
>> +    g_autofree char *exec_args = NULL;
>> +    g_auto(GStrv) argv = NULL;
>> +    char *from_str, *src, *dst;
>> +
>> +    args->start.hide_stderr = false;    /* omit redirection word from args */
> 
> It's default off, right?  Could I request for some more explanations?

Yes, the default is false, so I will omit this line.  I will change it to
an assertion. (IIRC when I first wrote this code 1-2 years ago, the cpr-exec
test was a derivative of a precopy common test that set hide_stderr=true).

hide_stderr must be false when deriving cpr-exec arguments because of
this code in framework.c:

     if (!getenv("QTEST_LOG") && args->hide_stderr) {
         ignore_stderr = "2>/dev/null";

ignore_stderr is appended to the command line.  For cpr-exec the command line
may not include redirection, because we pass it to execv(), not to the shell.

> Could we also set it in test_mode_exec() directly if needed?

Yes, one can set hide_stderr when launching the source VM.

>> +    migrate_args(&from_args, &to_args, args->listen_uri, &args->start);
>> +    qtest_from_args = qtest_qemu_args(from_args);
>> +
>> +    /* De-dup spaces so argv does not contain empty strings */
>> +    from_str = src = dst = g_strstrip(qtest_from_args);
>> +    do {
>> +        if (*src != ' ' || src[-1] != ' ') {
>> +            *dst++ = *src;
>> +        }
>> +    } while (*src++);
> 
> Pure ask.. when will empty string be present?

migrate_args() format strings "%s %s %s" produce "   " when the arguments
are empty strings.  Then g_strsplit("   ") would produce an array of 3
empty strings.

>> +
>> +    exec_args = g_strconcat(qtest_qemu_binary(QEMU_ENV_SRC),
> 
> Should this be QEMU_ENV_DST?

Good catch, thanks, will fix.

> OTOH, we can use migration_get_env()->qemu_dst to reduce referencing global
> vars.

OK, that works.

>> +                            " -incoming defer ", from_str, NULL);
>> +    argv = g_strsplit(exec_args, " ", -1);
>> +    migrate_set_parameter_strv(who, "cpr-exec-command", argv);
>> +}
>> +
>> +static void wait_for_migration_event(QTestState *who, const char *waitfor)
>> +{
>> +    QDict *rsp, *data;
>> +    char *status;
>> +    bool done = false;
>> +
>> +    while (!done) {
>> +        rsp = qtest_qmp_eventwait_ref(who, "MIGRATION");
>> +        g_assert(qdict_haskey(rsp, "data"));
>> +        data = qdict_get_qdict(rsp, "data");
>> +        g_assert(qdict_haskey(data, "status"));
>> +        status = g_strdup(qdict_get_str(data, "status"));
>> +        g_assert(strcmp(status, "failed"));
>> +        done = !strcmp(status, waitfor);
>> +        qobject_unref(rsp);
>> +    }
>> +}
>> +
>> +static void test_cpr_exec(MigrateCommon *args)
>> +{
>> +    QTestState *from, *to;
>> +    void *data_hook = NULL;
>> +    g_autofree char *connect_uri = g_strdup(args->connect_uri);
>> +    g_autofree char *filename = g_strdup_printf("%s/%s", tmpfs,
>> +                                                FILE_TEST_FILENAME);
>> +
>> +
> 
> Newline can be dropped.

OK.

- Steve

>> +    if (migrate_start(&from, NULL, args->listen_uri, &args->start)) {
>> +        return;
>> +    }
>> +
>> +    /* Source and dest never run concurrently */
>> +    g_assert_false(args->live);
>> +
>> +    if (args->start_hook) {
>> +        data_hook = args->start_hook(from, NULL);
>> +    }
>> +
>> +    wait_for_serial("src_serial");
>> +    set_cpr_exec_args(from, args);
>> +    migrate_set_capability(from, "events", true);
>> +    migrate_qmp(from, NULL, connect_uri, NULL, "{}");
>> +    wait_for_migration_event(from, "completed");
>> +
>> +    to = qtest_init_after_exec(from);
>> +
>> +    qtest_qmp_assert_success(to, "{ 'execute': 'migrate-incoming',"
>> +                             "  'arguments': { "
>> +                             "      'channels': [ { 'channel-type': 'main',"
>> +                             "      'addr': { 'transport': 'file',"
>> +                             "                'filename': %s,"
>> +                             "                'offset': 0  } } ] } }",
>> +                             filename);
>> +    wait_for_migration_complete(to);
>> +
>> +    wait_for_resume(to, get_dst());
>> +    /* Device on target is still named src_serial because args do not change */
>> +    wait_for_serial("src_serial");
>> +
>> +    if (args->end_hook) {
>> +        args->end_hook(from, to, data_hook);
>> +    }
>> +
>> +    migrate_end(from, to, args->result == MIG_TEST_SUCCEED);
>> +}
>> +
>> +static void *test_mode_exec_start(QTestState *from, QTestState *to)
>> +{
>> +    assert(!to);
>> +    migrate_set_parameter_str(from, "mode", "cpr-exec");
>> +    return NULL;
>> +}
>> +
>> +static void test_mode_exec(void)
>> +{
>> +    g_autofree char *uri = g_strdup_printf("file:%s/%s", tmpfs,
>> +                                           FILE_TEST_FILENAME);
>> +    g_autofree char *listen_uri = g_strdup_printf("defer");
>> +
>> +    MigrateCommon args = {
>> +        .start.only_source = true,
>> +        .start.opts_source = "-machine aux-ram-share=on -nodefaults",
>> +        .start.memory_backend = "-object memory-backend-memfd,id=pc.ram,size=%s"
>> +                                " -machine memory-backend=pc.ram",
>> +        .connect_uri = uri,
>> +        .listen_uri = listen_uri,
>> +        .start_hook = test_mode_exec_start,
>> +    };
>> +
>> +    test_cpr_exec(&args);
>> +}
>> +
>>   void migration_test_add_cpr(MigrationTestEnv *env)
>>   {
>>       tmpfs = env->tmpfs;
>> @@ -132,5 +251,6 @@ void migration_test_add_cpr(MigrationTestEnv *env)
>>           migration_test_add("/migration/mode/transfer", test_mode_transfer);
>>           migration_test_add("/migration/mode/transfer/defer",
>>                              test_mode_transfer_defer);
>> +        migration_test_add("/migration/mode/exec", test_mode_exec);
>>       }
>>   }
>> -- 
>> 1.8.3.1
>>
> 

Re: [PATCH V1 11/11] migration-test: test cpr-exec

[Peter Xu]

On Tue, Sep 30, 2025 at 02:23:49PM -0400, Steven Sistare wrote:
> On 9/30/2025 1:08 PM, Peter Xu wrote:
> > On Fri, Sep 19, 2025 at 07:12:33AM -0700, Steve Sistare wrote:
> > > Add a test for the cpr-exec migration mode.
> > > 
> > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> > 
> > Looks good, only some nitpicks or pure questions below.
> > 
> > > ---
> > >   tests/qtest/migration/cpr-tests.c | 120 ++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 120 insertions(+)
> > > 
> > > diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c
> > > index 5e764a6..f33af76 100644
> > > --- a/tests/qtest/migration/cpr-tests.c
> > > +++ b/tests/qtest/migration/cpr-tests.c
> > > @@ -110,6 +110,125 @@ static void test_mode_transfer_defer(void)
> > >       test_mode_transfer_common(true);
> > >   }
> > > +static void set_cpr_exec_args(QTestState *who, MigrateCommon *args)
> > > +{
> > > +    g_autofree char *qtest_from_args = NULL;
> > > +    g_autofree char *from_args = NULL;
> > > +    g_autofree char *to_args = NULL;
> > > +    g_autofree char *exec_args = NULL;
> > > +    g_auto(GStrv) argv = NULL;
> > > +    char *from_str, *src, *dst;
> > > +
> > > +    args->start.hide_stderr = false;    /* omit redirection word from args */
> > 
> > It's default off, right?  Could I request for some more explanations?
> 
> Yes, the default is false, so I will omit this line.  I will change it to
> an assertion. (IIRC when I first wrote this code 1-2 years ago, the cpr-exec
> test was a derivative of a precopy common test that set hide_stderr=true).
> 
> hide_stderr must be false when deriving cpr-exec arguments because of
> this code in framework.c:
> 
>     if (!getenv("QTEST_LOG") && args->hide_stderr) {
>         ignore_stderr = "2>/dev/null";
> 
> ignore_stderr is appended to the command line.  For cpr-exec the command line
> may not include redirection, because we pass it to execv(), not to the shell.

Please kindly add this rich comment above the assertion..

> 
> > Could we also set it in test_mode_exec() directly if needed?
> 
> Yes, one can set hide_stderr when launching the source VM.
> 
> > > +    migrate_args(&from_args, &to_args, args->listen_uri, &args->start);
> > > +    qtest_from_args = qtest_qemu_args(from_args);
> > > +
> > > +    /* De-dup spaces so argv does not contain empty strings */
> > > +    from_str = src = dst = g_strstrip(qtest_from_args);
> > > +    do {
> > > +        if (*src != ' ' || src[-1] != ' ') {
> > > +            *dst++ = *src;
> > > +        }
> > > +    } while (*src++);
> > 
> > Pure ask.. when will empty string be present?
> 
> migrate_args() format strings "%s %s %s" produce "   " when the arguments
> are empty strings.  Then g_strsplit("   ") would produce an array of 3
> empty strings.

... and here too.

Thanks,

-- 
Peter Xu

Re: [PATCH V1 11/11] migration-test: test cpr-exec

[Steven Sistare]

On 9/30/2025 3:02 PM, Peter Xu wrote:
> On Tue, Sep 30, 2025 at 02:23:49PM -0400, Steven Sistare wrote:
>> On 9/30/2025 1:08 PM, Peter Xu wrote:
>>> On Fri, Sep 19, 2025 at 07:12:33AM -0700, Steve Sistare wrote:
>>>> Add a test for the cpr-exec migration mode.
>>>>
>>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>>
>>> Looks good, only some nitpicks or pure questions below.
>>>
>>>> ---
>>>>    tests/qtest/migration/cpr-tests.c | 120 ++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 120 insertions(+)
>>>>
>>>> diff --git a/tests/qtest/migration/cpr-tests.c b/tests/qtest/migration/cpr-tests.c
>>>> index 5e764a6..f33af76 100644
>>>> --- a/tests/qtest/migration/cpr-tests.c
>>>> +++ b/tests/qtest/migration/cpr-tests.c
>>>> @@ -110,6 +110,125 @@ static void test_mode_transfer_defer(void)
>>>>        test_mode_transfer_common(true);
>>>>    }
>>>> +static void set_cpr_exec_args(QTestState *who, MigrateCommon *args)
>>>> +{
>>>> +    g_autofree char *qtest_from_args = NULL;
>>>> +    g_autofree char *from_args = NULL;
>>>> +    g_autofree char *to_args = NULL;
>>>> +    g_autofree char *exec_args = NULL;
>>>> +    g_auto(GStrv) argv = NULL;
>>>> +    char *from_str, *src, *dst;
>>>> +
>>>> +    args->start.hide_stderr = false;    /* omit redirection word from args */
>>>
>>> It's default off, right?  Could I request for some more explanations?
>>
>> Yes, the default is false, so I will omit this line.  I will change it to
>> an assertion. (IIRC when I first wrote this code 1-2 years ago, the cpr-exec
>> test was a derivative of a precopy common test that set hide_stderr=true).
>>
>> hide_stderr must be false when deriving cpr-exec arguments because of
>> this code in framework.c:
>>
>>      if (!getenv("QTEST_LOG") && args->hide_stderr) {
>>          ignore_stderr = "2>/dev/null";
>>
>> ignore_stderr is appended to the command line.  For cpr-exec the command line
>> may not include redirection, because we pass it to execv(), not to the shell.
> 
> Please kindly add this rich comment above the assertion..

will do.

> 
>>
>>> Could we also set it in test_mode_exec() directly if needed?
>>
>> Yes, one can set hide_stderr when launching the source VM.
>>
>>>> +    migrate_args(&from_args, &to_args, args->listen_uri, &args->start);
>>>> +    qtest_from_args = qtest_qemu_args(from_args);
>>>> +
>>>> +    /* De-dup spaces so argv does not contain empty strings */
>>>> +    from_str = src = dst = g_strstrip(qtest_from_args);
>>>> +    do {
>>>> +        if (*src != ' ' || src[-1] != ' ') {
>>>> +            *dst++ = *src;
>>>> +        }
>>>> +    } while (*src++);
>>>
>>> Pure ask.. when will empty string be present?
>>
>> migrate_args() format strings "%s %s %s" produce "   " when the arguments
>> are empty strings.  Then g_strsplit("   ") would produce an array of 3
>> empty strings.
> 
> ... and here too.

will do.

- Steve

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

[Fabiano Rosas]

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

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

[Steven Sistare]

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

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

[Steven Sistare]

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

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

[Fabiano Rosas]

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

> 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?
>

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