[PATCH 0/2] test/qtest/migration: Use out-of-band execution for migrate-recover

[PATCH 0/2] test/qtest/migration: Use out-of-band execution for migrate-recover

[Juraj Marcin]

In real use cases, the migrate-recover command requires out-of-band
execution, because the main thread processing normal commands is blocked
by a page fault in the guest memory. Tests, however, do not reflect this
which might result in some bugs not being caught with tests.

The first patch in the series, adds a new qtest initialization function
which accepts capabilities the test wants to enable. This allows a test
to enable the oob capability.

The second patch then enables the oob capability in migration tests and
changes the execution of the migrate-recover command.

Juraj Marcin (2):
  tests/qtest: Introduce qtest_init_with_env_and_capabilities()
  tests/qtest/migration: Use out-of-band execution for migrate-recover

 tests/qtest/libqtest.c                | 18 ++++++++++++++++--
 tests/qtest/libqtest.h                | 17 +++++++++++++++++
 tests/qtest/migration/framework.c     | 23 +++++++++++++++++++++--
 tests/qtest/migration/framework.h     |  2 ++
 tests/qtest/migration/migration-qmp.c |  2 +-
 5 files changed, 57 insertions(+), 5 deletions(-)

-- 
2.47.1

[PATCH 1/2] tests/qtest: Introduce qtest_init_with_env_and_capabilities()

[Juraj Marcin]

This patch adds a new version of qtest_init_with_env() that allows
specifying QMP capabilities that should be enabled during handshake.
This is useful for example if a test needs out-of-band execution of QMP
commands, it can initialize with the oob capability.

Signed-off-by: Juraj Marcin <jmarcin@redhat.com>
---
 tests/qtest/libqtest.c | 18 ++++++++++++++++--
 tests/qtest/libqtest.h | 17 +++++++++++++++++
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 8de5f1fde3..00dc01a851 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -543,7 +543,9 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     return qtest_init_internal(qtest_qemu_binary(NULL), extra_args);
 }
 
-QTestState *qtest_init_with_env(const char *var, const char *extra_args)
+QTestState *qtest_init_with_env_and_capabilities(const char *var,
+                                                 const char *extra_args,
+                                                 QList *capabilities)
 {
     QTestState *s = qtest_init_internal(qtest_qemu_binary(var), extra_args);
     QDict *greeting;
@@ -551,11 +553,23 @@ QTestState *qtest_init_with_env(const char *var, const char *extra_args)
     /* Read the QMP greeting and then do the handshake */
     greeting = qtest_qmp_receive(s);
     qobject_unref(greeting);
-    qobject_unref(qtest_qmp(s, "{ 'execute': 'qmp_capabilities' }"));
+    if (capabilities) {
+        qtest_qmp_assert_success(s,
+                                 "{ 'execute': 'qmp_capabilities', "
+                                 "'arguments': { 'enable': %p } }",
+                                 qobject_ref(capabilities));
+    } else {
+        qtest_qmp_assert_success(s, "{ 'execute': 'qmp_capabilities' }");
+    }
 
     return s;
 }
 
+QTestState *qtest_init_with_env(const char *var, const char *extra_args)
+{
+    return qtest_init_with_env_and_capabilities(var, extra_args, NULL);
+}
+
 QTestState *qtest_init(const char *extra_args)
 {
     return qtest_init_with_env(NULL, extra_args);
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index f23d80e9e5..1d0d5e7c29 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -19,6 +19,7 @@
 
 #include "qapi/qmp/qobject.h"
 #include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qlist.h"
 #include "libqmp.h"
 
 typedef struct QTestState QTestState;
@@ -68,6 +69,22 @@ QTestState *qtest_init(const char *extra_args);
  */
 QTestState *qtest_init_with_env(const char *var, const char *extra_args);
 
+/**
+ * qtest_init_with_env_and_capabilities:
+ * @var: Environment variable from where to take the QEMU binary
+ * @extra_args: Other arguments to pass to QEMU.  CAUTION: these
+ * arguments are subject to word splitting and shell evaluation.
+ * @capabilities: list of QMP capabilities (strings) to enable
+ *
+ * Like qtest_init_with_env(), but enable specified capabilities during
+ * hadshake.
+ *
+ * Returns: #QTestState instance.
+ */
+QTestState *qtest_init_with_env_and_capabilities(const char *var,
+                                                 const char *extra_args,
+                                                 QList *capabilities);
+
 /**
  * qtest_init_without_qmp_handshake:
  * @extra_args: other arguments to pass to QEMU.  CAUTION: these
-- 
2.47.1

[PATCH 2/2] tests/qtest/migration: Use out-of-band execution for migrate-recover

[Juraj Marcin]

In real use cases, the migrate-recover command requires out-of-band
execution, because the thread processing normal commands is blocked by a
page fault in the guest memory. With this change, the tests will be
closer to real use cases and could help detect regressions and other
bugs in migration recovery.

Signed-off-by: Juraj Marcin <jmarcin@redhat.com>
---
 tests/qtest/migration/framework.c     | 23 +++++++++++++++++++++--
 tests/qtest/migration/framework.h     |  2 ++
 tests/qtest/migration/migration-qmp.c |  2 +-
 3 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/migration/framework.c b/tests/qtest/migration/framework.c
index 47ce07856e..4550cda129 100644
--- a/tests/qtest/migration/framework.c
+++ b/tests/qtest/migration/framework.c
@@ -194,6 +194,16 @@ static void cleanup(const char *filename)
     unlink(path);
 }
 
+static QList *migrate_start_get_qmp_capabilities(const MigrateStart *args)
+{
+    QList *capabilities = qlist_new();
+
+    if (args->oob) {
+        qlist_append_str(capabilities, "oob");
+    }
+    return capabilities;
+}
+
 int migrate_start(QTestState **from, QTestState **to, const char *uri,
                   MigrateStart *args)
 {
@@ -210,6 +220,7 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
     const char *machine_alias, *machine_opts = "";
     g_autofree char *machine = NULL;
     const char *bootpath;
+    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)) {
@@ -314,7 +325,8 @@ int migrate_start(QTestState **from, QTestState **to, const char *uri,
                                  args->opts_source ? args->opts_source : "",
                                  ignore_stderr);
     if (!args->only_target) {
-        *from = qtest_init_with_env(QEMU_ENV_SRC, cmd_source);
+        *from = qtest_init_with_env_and_capabilities(QEMU_ENV_SRC, cmd_source,
+                                                     capabilities);
         qtest_qmp_set_event_callback(*from,
                                      migrate_watch_for_events,
                                      &src_state);
@@ -334,7 +346,8 @@ 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_with_env(QEMU_ENV_DST, cmd_target);
+    *to = qtest_init_with_env_and_capabilities(QEMU_ENV_DST, cmd_target,
+                                               capabilities);
     qtest_qmp_set_event_callback(*to,
                                  migrate_watch_for_events,
                                  &dst_state);
@@ -601,6 +614,12 @@ void test_postcopy_recovery_common(MigrateCommon *args)
     QTestState *from, *to;
     g_autofree char *uri = NULL;
 
+    /*
+     * Always enable OOB QMP capability for recovery tests, migrate-recover is
+     * executed out-of-band
+     */
+    args->start.oob = true;
+
     /* Always hide errors for postcopy recover tests since they're expected */
     args->start.hide_stderr = true;
 
diff --git a/tests/qtest/migration/framework.h b/tests/qtest/migration/framework.h
index e9fc4ec363..7991ee56b6 100644
--- a/tests/qtest/migration/framework.h
+++ b/tests/qtest/migration/framework.h
@@ -109,6 +109,8 @@ typedef struct {
     const char *opts_target;
     /* suspend the src before migrating to dest. */
     bool suspend_me;
+    /* enable OOB QMP capability */
+    bool oob;
 } MigrateStart;
 
 typedef enum PostcopyRecoveryFailStage {
diff --git a/tests/qtest/migration/migration-qmp.c b/tests/qtest/migration/migration-qmp.c
index 71b14b51b2..9431d2beda 100644
--- a/tests/qtest/migration/migration-qmp.c
+++ b/tests/qtest/migration/migration-qmp.c
@@ -464,7 +464,7 @@ void migrate_continue(QTestState *who, const char *state)
 void migrate_recover(QTestState *who, const char *uri)
 {
     qtest_qmp_assert_success(who,
-                             "{ 'execute': 'migrate-recover', "
+                             "{ 'exec-oob': 'migrate-recover', "
                              "  'id': 'recover-cmd', "
                              "  'arguments': { 'uri': %s } }",
                              uri);
-- 
2.47.1

Re: [PATCH 0/2] test/qtest/migration: Use out-of-band execution for migrate-recover

[Peter Xu]

On Tue, Jan 07, 2025 at 05:31:52PM +0100, Juraj Marcin wrote:
> In real use cases, the migrate-recover command requires out-of-band
> execution, because the main thread processing normal commands is blocked
> by a page fault in the guest memory. Tests, however, do not reflect this
> which might result in some bugs not being caught with tests.
> 
> The first patch in the series, adds a new qtest initialization function
> which accepts capabilities the test wants to enable. This allows a test
> to enable the oob capability.
> 
> The second patch then enables the oob capability in migration tests and
> changes the execution of the migrate-recover command.
> 
> Juraj Marcin (2):
>   tests/qtest: Introduce qtest_init_with_env_and_capabilities()
>   tests/qtest/migration: Use out-of-band execution for migrate-recover
> 
>  tests/qtest/libqtest.c                | 18 ++++++++++++++++--
>  tests/qtest/libqtest.h                | 17 +++++++++++++++++
>  tests/qtest/migration/framework.c     | 23 +++++++++++++++++++++--
>  tests/qtest/migration/framework.h     |  2 ++
>  tests/qtest/migration/migration-qmp.c |  2 +-
>  5 files changed, 57 insertions(+), 5 deletions(-)

Looks all good, thanks!

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu