[PATCH v3 00/11] tests/migration-test: Allow testing older machine types

[PATCH v3 00/11] tests/migration-test: Allow testing older machine types

[Fabiano Rosas]

This adds support for running migration-test with two different QEMU
versions to test migration compatibility. The tests automatically
choose the latest machine type supported by both QEMU versions.

changes:

- added a cleanup function for the allocated strings in the machine
  list. The suggestion of making a copy of the list won't work because
  keeping the structure around to avoid freeing memory, and caching
  the list are two separate things that only make sense together at
  qtest_get_machines(). Outside of it, code can just free memory
  normally and caching doesn't make sense because there's no QMP call;

- kept strdups for the reasons mentioned above;

- added a message when we're unable to find a common machine type
  (should never happen);

- changed the default x86_64 machine to q35;

- new patch to fix booting with the a-b bootsector on q35;

- added machine_opts for the options after the comma;

- used machine_alias and machine for the variable names;

CI run: https://gitlab.com/farosas/qemu/-/pipelines/1041238172

v2:
https://lore.kernel.org/r/20231006123910.17759-1-farosas@suse.de

changes:

- introduce *_with_env variants of the relevant functions [Daniel, Juan]

- keep the requirement for having the QTEST_QEMU_BINARY always
  present. qtest_get_arch() is used extensively in the qtest_add*
  functions. It would be too much churn to pass a different binary
  into it.

- with this^ we also need to keep the requirement for using only one
  of SRC|DST. Otherwise it would be confusing to have three binaries
  listed.

- query the alias to find out the machine types [Daniel]

I haven't looked into the docker part for now. I think Daniel's
suggestion of QTEST_QEMU_BINARY_SRC='podman run ... qemu-system-foo'
looks interesting. Do we have the latest release already built in the
registry at any given point?

Thanks

v1:
https://lore.kernel.org/r/20231003141932.2367-1-farosas@suse.de

Hi, I had this WIP patch laying around that seems to fit Juan's vision
about testing older machine types. It is a very rough draft for now,
but it may be useful for kickstarting the discussion.

With this we can give the tests two different QEMU versions. The test
picks the older machine type between the two and runs the whole
migration-test suite.

We'd just need a way to provide the older build. Currently I'm doing
this by hand.

sample output:
 # Using two different QEMU binaries. Common machine type: pc-i440fx-8.1
 ...
 # Using ./qemu-system-x86_64 (v8.1.0-952-g8a940312a2-dirty) as migration source
 ...
 # Using ../build-8.1.0/qemu-system-x86_64 (v8.1.0-dirty) as migration destination

Let me know what you think.

Fabiano Rosas (11):
  tests/qtest: Allow qtest_qemu_binary to use a custom environment
    variable
  tests/qtest: Introduce qtest_init_with_env
  tests/qtest: Allow qtest_get_machines to use an alternate QEMU binary
  tests/qtest: Introduce qtest_has_machine_with_env
  tests/qtest: Introduce qtest_resolve_machine_alias
  tests/qtest/migration: Introduce find_common_machine_version
  tests/qtest/migration: Define a machine for all architectures
  tests/qtest/migration: Specify the geometry of the bootsector
  tests/qtest/migration: Set q35 as the default machine for x86_86
  tests/qtest/migration: Support more than one QEMU binary
  tests/qtest: Don't print messages from query instances

 tests/qtest/libqtest.c          | 86 +++++++++++++++++++++++++++------
 tests/qtest/libqtest.h          | 32 ++++++++++++
 tests/qtest/migration-helpers.c | 24 +++++++++
 tests/qtest/migration-helpers.h |  2 +
 tests/qtest/migration-test.c    | 44 +++++++++++++++--
 5 files changed, 169 insertions(+), 19 deletions(-)

-- 
2.35.3

[PATCH v3 01/11] tests/qtest: Allow qtest_qemu_binary to use a custom environment variable

[Fabiano Rosas]

We're adding support for testing migration using two different QEMU
binaries. We'll provide the second binary in a new environment
variable.

Allow qtest_qemu_binary() to receive the name of the new variable. If
the new environment variable is not set, that's not an error, we use
QTEST_QEMU_BINARY as a fallback.

Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/libqtest.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index dc7a55634c..03fa644663 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -336,10 +336,17 @@ void qtest_remove_abrt_handler(void *data)
     }
 }
 
-static const char *qtest_qemu_binary(void)
+static const char *qtest_qemu_binary(const char *var)
 {
     const char *qemu_bin;
 
+    if (var) {
+        qemu_bin = getenv(var);
+        if (qemu_bin) {
+            return qemu_bin;
+        }
+    }
+
     qemu_bin = getenv("QTEST_QEMU_BINARY");
     if (!qemu_bin) {
         fprintf(stderr, "Environment variable QTEST_QEMU_BINARY required\n");
@@ -392,7 +399,7 @@ static QTestState *G_GNUC_PRINTF(1, 2) qtest_spawn_qemu(const char *fmt, ...)
 
     va_start(ap, fmt);
     g_string_append_printf(command, CMD_EXEC "%s %s",
-                           qtest_qemu_binary(), tracearg);
+                           qtest_qemu_binary(NULL), tracearg);
     g_string_append_vprintf(command, fmt, ap);
     va_end(ap);
 
@@ -905,7 +912,7 @@ char *qtest_hmp(QTestState *s, const char *fmt, ...)
 
 const char *qtest_get_arch(void)
 {
-    const char *qemu = qtest_qemu_binary();
+    const char *qemu = qtest_qemu_binary(NULL);
     const char *end = strrchr(qemu, '-');
 
     if (!end) {
-- 
2.35.3

[PATCH v3 02/11] tests/qtest: Introduce qtest_init_with_env

[Fabiano Rosas]

Add a version of qtest_init() that takes an environment variable
containing the path of the QEMU binary. This allows tests to use more
than one QEMU binary.

If no variable is provided or the environment variable does not exist,
that is not an error. Fallback to using QTEST_QEMU_BINARY.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/libqtest.c | 26 +++++++++++++++++++-------
 tests/qtest/libqtest.h | 13 +++++++++++++
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 03fa644663..9eebba8767 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -388,7 +388,8 @@ static pid_t qtest_create_process(char *cmd)
 }
 #endif /* _WIN32 */
 
-static QTestState *G_GNUC_PRINTF(1, 2) qtest_spawn_qemu(const char *fmt, ...)
+static QTestState *G_GNUC_PRINTF(2, 3) qtest_spawn_qemu(const char *qemu_bin,
+                                                        const char *fmt, ...)
 {
     va_list ap;
     QTestState *s = g_new0(QTestState, 1);
@@ -398,8 +399,7 @@ static QTestState *G_GNUC_PRINTF(1, 2) qtest_spawn_qemu(const char *fmt, ...)
     g_autoptr(GString) command = g_string_new("");
 
     va_start(ap, fmt);
-    g_string_append_printf(command, CMD_EXEC "%s %s",
-                           qtest_qemu_binary(NULL), tracearg);
+    g_string_append_printf(command, CMD_EXEC "%s %s", qemu_bin, tracearg);
     g_string_append_vprintf(command, fmt, ap);
     va_end(ap);
 
@@ -438,7 +438,8 @@ static QTestState *G_GNUC_PRINTF(1, 2) qtest_spawn_qemu(const char *fmt, ...)
     return s;
 }
 
-QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
+static QTestState *qtest_init_internal(const char *qemu_bin,
+                                       const char *extra_args)
 {
     QTestState *s;
     int sock, qmpsock, i;
@@ -463,7 +464,8 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     sock = init_socket(socket_path);
     qmpsock = init_socket(qmp_socket_path);
 
-    s = qtest_spawn_qemu("-qtest unix:%s "
+    s = qtest_spawn_qemu(qemu_bin,
+                         "-qtest unix:%s "
                          "-qtest-log %s "
                          "-chardev socket,path=%s,id=char0 "
                          "-mon chardev=char0,mode=control "
@@ -516,9 +518,14 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
     return s;
 }
 
-QTestState *qtest_init(const char *extra_args)
+QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
 {
-    QTestState *s = qtest_init_without_qmp_handshake(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 *s = qtest_init_internal(qtest_qemu_binary(var), extra_args);
     QDict *greeting;
 
     /* Read the QMP greeting and then do the handshake */
@@ -529,6 +536,11 @@ QTestState *qtest_init(const char *extra_args)
     return s;
 }
 
+QTestState *qtest_init(const char *extra_args)
+{
+    return qtest_init_with_env(NULL, extra_args);
+}
+
 QTestState *qtest_vinitf(const char *fmt, va_list ap)
 {
     char *args = g_strdup_vprintf(fmt, ap);
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 5fe3d13466..76fc195f1c 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -55,6 +55,19 @@ QTestState *qtest_vinitf(const char *fmt, va_list ap) G_GNUC_PRINTF(1, 0);
  */
 QTestState *qtest_init(const char *extra_args);
 
+/**
+ * qtest_init_with_env:
+ * @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.
+ *
+ * Like qtest_init(), but use a different environment variable for the
+ * QEMU binary.
+ *
+ * Returns: #QTestState instance.
+ */
+QTestState *qtest_init_with_env(const char *var, const char *extra_args);
+
 /**
  * qtest_init_without_qmp_handshake:
  * @extra_args: other arguments to pass to QEMU.  CAUTION: these
-- 
2.35.3

[PATCH v3 03/11] tests/qtest: Allow qtest_get_machines to use an alternate QEMU binary

[Fabiano Rosas]

We're adding support for using more than one QEMU binary in
tests. Modify qtest_get_machines() to take an environment variable
that contains the QEMU binary path.

Since the function keeps a cache of the machines list in the form of a
static variable, refresh it any time the environment variable changes.

Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/libqtest.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 9eebba8767..c52fab373f 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1472,9 +1472,10 @@ struct MachInfo {
  * Returns an array with pointers to the available machine names.
  * The terminating entry has the name set to NULL.
  */
-static struct MachInfo *qtest_get_machines(void)
+static struct MachInfo *qtest_get_machines(const char *var)
 {
     static struct MachInfo *machines;
+    static char *qemu_var;
     QDict *response, *minfo;
     QList *list;
     const QListEntry *p;
@@ -1483,11 +1484,19 @@ static struct MachInfo *qtest_get_machines(void)
     QTestState *qts;
     int idx;
 
+    if (g_strcmp0(qemu_var, var)) {
+        qemu_var = g_strdup(var);
+
+        /* new qemu, clear the cache */
+        g_free(machines);
+        machines = NULL;
+    }
+
     if (machines) {
         return machines;
     }
 
-    qts = qtest_init("-machine none");
+    qts = qtest_init_with_env(qemu_var, "-machine none");
     response = qtest_qmp(qts, "{ 'execute': 'query-machines' }");
     g_assert(response);
     list = qdict_get_qlist(response, "return");
@@ -1528,7 +1537,7 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine),
     struct MachInfo *machines;
     int i;
 
-    machines = qtest_get_machines();
+    machines = qtest_get_machines(NULL);
 
     for (i = 0; machines[i].name != NULL; i++) {
         /* Ignore machines that cannot be used for qtests */
@@ -1549,7 +1558,7 @@ bool qtest_has_machine(const char *machine)
     struct MachInfo *machines;
     int i;
 
-    machines = qtest_get_machines();
+    machines = qtest_get_machines(NULL);
 
     for (i = 0; machines[i].name != NULL; i++) {
         if (g_str_equal(machine, machines[i].name) ||
-- 
2.35.3

[PATCH v3 04/11] tests/qtest: Introduce qtest_has_machine_with_env

[Fabiano Rosas]

Add a variant of qtest_has_machine() that receives an environment
variable containing an alternate QEMU binary path.

Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/libqtest.c | 9 +++++++--
 tests/qtest/libqtest.h | 9 +++++++++
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index c52fab373f..7c5628e0dc 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1553,12 +1553,12 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine),
     }
 }
 
-bool qtest_has_machine(const char *machine)
+bool qtest_has_machine_with_env(const char *var, const char *machine)
 {
     struct MachInfo *machines;
     int i;
 
-    machines = qtest_get_machines(NULL);
+    machines = qtest_get_machines(var);
 
     for (i = 0; machines[i].name != NULL; i++) {
         if (g_str_equal(machine, machines[i].name) ||
@@ -1570,6 +1570,11 @@ bool qtest_has_machine(const char *machine)
     return false;
 }
 
+bool qtest_has_machine(const char *machine)
+{
+    return qtest_has_machine_with_env(NULL, machine);
+}
+
 bool qtest_has_device(const char *device)
 {
     static QList *list;
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 76fc195f1c..d16deb9891 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -930,6 +930,15 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine),
  */
 bool qtest_has_machine(const char *machine);
 
+/**
+ * qtest_has_machine_with_env:
+ * @var: Environment variable from where to take the QEMU binary
+ * @machine: The machine to look for
+ *
+ * Returns: true if the machine is available in the specified binary.
+ */
+bool qtest_has_machine_with_env(const char *var, const char *machine);
+
 /**
  * qtest_has_device:
  * @device: The device to look for
-- 
2.35.3

[PATCH v3 05/11] tests/qtest: Introduce qtest_resolve_machine_alias

[Fabiano Rosas]

The migration tests are being enhanced to test migration between
different QEMU versions. A requirement of migration is that the
machine type between source and destination matches, including the
version.

We cannot hardcode machine types in the tests because those change
with each release. QEMU provides a machine type alias that has a fixed
name, but points to the latest machine type at each release.

Add a helper to resolve the alias into the exact machine
type. E.g. "-machine pc" resolves to "pc-i440fx-8.2"

Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/libqtest.c | 16 ++++++++++++++++
 tests/qtest/libqtest.h | 10 ++++++++++
 2 files changed, 26 insertions(+)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 7c5628e0dc..fda163836c 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1553,6 +1553,22 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine),
     }
 }
 
+char *qtest_resolve_machine_alias(const char *var, const char *alias)
+{
+    struct MachInfo *machines;
+    int i;
+
+    machines = qtest_get_machines(var);
+
+    for (i = 0; machines[i].name != NULL; i++) {
+        if (machines[i].alias && g_str_equal(alias, machines[i].alias)) {
+            return g_strdup(machines[i].name);
+        }
+    }
+
+    return NULL;
+}
+
 bool qtest_has_machine_with_env(const char *var, const char *machine)
 {
     struct MachInfo *machines;
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index d16deb9891..6e3d3525bf 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -922,6 +922,16 @@ void qtest_qmp_fds_assert_success(QTestState *qts, int *fds, size_t nfds,
 void qtest_cb_for_every_machine(void (*cb)(const char *machine),
                                 bool skip_old_versioned);
 
+/**
+ * qtest_resolve_machine_alias:
+ * @var: Environment variable from where to take the QEMU binary
+ * @alias: The alias to resolve
+ *
+ * Returns: the machine type corresponding to the alias if any,
+ * otherwise NULL.
+ */
+char *qtest_resolve_machine_alias(const char *var, const char *alias);
+
 /**
  * qtest_has_machine:
  * @machine: The machine to look for
-- 
2.35.3

[PATCH v3 06/11] tests/qtest/migration: Introduce find_common_machine_version

[Fabiano Rosas]

When using two different QEMU binaries for migration testing, we'll
need to find what is the machine version that will work with both
binaries. Add a helper for that.

Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration-helpers.c | 24 ++++++++++++++++++++++++
 tests/qtest/migration-helpers.h |  2 ++
 2 files changed, 26 insertions(+)

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index 0c185db450..1fb0362af7 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -240,3 +240,27 @@ void wait_for_migration_fail(QTestState *from, bool allow_active)
     g_assert(qdict_get_bool(rsp_return, "running"));
     qobject_unref(rsp_return);
 }
+
+char *find_common_machine_version(const char *mtype, const char *var1,
+                                  const char *var2)
+{
+    g_autofree char *type1 = qtest_resolve_machine_alias(var1, mtype);
+    g_autofree char *type2 = qtest_resolve_machine_alias(var2, mtype);
+
+    g_assert(type1 && type2);
+
+    if (g_str_equal(type1, type2)) {
+        /* either can be used */
+        return g_strdup(type1);
+    }
+
+    if (qtest_has_machine_with_env(var2, type1)) {
+        return g_strdup(type1);
+    }
+
+    if (qtest_has_machine_with_env(var1, type2)) {
+        return g_strdup(type2);
+    }
+
+    g_assert_not_reached();
+}
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index 4f51d0f8bc..d1c2351d33 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -43,4 +43,6 @@ void wait_for_migration_complete(QTestState *who);
 
 void wait_for_migration_fail(QTestState *from, bool allow_active);
 
+char *find_common_machine_version(const char *mtype, const char *var1,
+                                  const char *var2);
 #endif /* MIGRATION_HELPERS_H */
-- 
2.35.3

[PATCH v3 07/11] tests/qtest/migration: Define a machine for all architectures

[Fabiano Rosas]

Stop relying on defaults and select a machine explicitly for every
architecture.

This is a prerequisite for being able to select machine types for
migration using different QEMU binaries for source and destination.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration-test.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index e1c110537b..40de320fcc 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -743,6 +743,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
     const char *kvm_opts = NULL;
     const char *arch = qtest_get_arch();
     const char *memory_size;
+    const char *machine;
 
     if (args->use_shmem) {
         if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) {
@@ -755,11 +756,13 @@ static int test_migrate_start(QTestState **from, QTestState **to,
     got_dst_resume = false;
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         memory_size = "150M";
+        machine = "pc";
         arch_opts = g_strdup_printf("-drive file=%s,format=raw", bootpath);
         start_address = X86_TEST_MEM_START;
         end_address = X86_TEST_MEM_END;
     } else if (g_str_equal(arch, "s390x")) {
         memory_size = "128M";
+        machine = "s390-ccw-virtio";
         arch_opts = g_strdup_printf("-bios %s", bootpath);
         start_address = S390_TEST_MEM_START;
         end_address = S390_TEST_MEM_END;
@@ -771,10 +774,12 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                       "'nvramrc=hex .\" _\" begin %x %x "
                                       "do i c@ 1 + i c! 1000 +loop .\" B\" 0 "
                                       "until'", end_address, start_address);
+        machine = "pseries";
         arch_opts = g_strdup("-nodefaults -machine vsmt=8");
     } else if (strcmp(arch, "aarch64") == 0) {
         memory_size = "150M";
-        arch_opts = g_strdup_printf("-machine virt,gic-version=max -cpu max "
+        machine = "virt";
+        arch_opts = g_strdup_printf("-machine gic-version=max -cpu max "
                                     "-kernel %s", bootpath);
         start_address = ARM_TEST_MEM_START;
         end_address = ARM_TEST_MEM_END;
@@ -810,11 +815,13 @@ static int test_migrate_start(QTestState **from, QTestState **to,
     }
 
     cmd_source = g_strdup_printf("-accel kvm%s -accel tcg "
+                                 "-machine %s "
                                  "-name source,debug-threads=on "
                                  "-m %s "
                                  "-serial file:%s/src_serial "
                                  "%s %s %s %s %s",
                                  kvm_opts ? kvm_opts : "",
+                                 machine,
                                  memory_size, tmpfs,
                                  arch_opts ? arch_opts : "",
                                  arch_source ? arch_source : "",
@@ -829,12 +836,14 @@ static int test_migrate_start(QTestState **from, QTestState **to,
     }
 
     cmd_target = g_strdup_printf("-accel kvm%s -accel tcg "
+                                 "-machine %s "
                                  "-name target,debug-threads=on "
                                  "-m %s "
                                  "-serial file:%s/dest_serial "
                                  "-incoming %s "
                                  "%s %s %s %s %s",
                                  kvm_opts ? kvm_opts : "",
+                                 machine,
                                  memory_size, tmpfs, uri,
                                  arch_opts ? arch_opts : "",
                                  arch_target ? arch_target : "",
-- 
2.35.3

[PATCH v3 08/11] tests/qtest/migration: Specify the geometry of the bootsector

[Fabiano Rosas]

We're about to enable the x86_64 tests to run with the q35 machine,
but that machine does not work with the program we use to dirty the
memory for the tests.

The issue is that QEMU needs to guess the geometry of the "disk" we
give to it and the guessed geometry doesn't pass the sanity checks
done by SeaBIOS. This causes SeaBIOS to interpret the geometry as if
needing a translation from LBA to CHS and SeaBIOS ends up miscomputing
the number of cylinders and aborting due to that.

The reason things work with the "pc" machine is that is uses ATA
instead of AHCI like q35 and SeaBIOS has an exception for ATA that
ends up skipping the sanity checks and ignoring translation
altogether.

Workaround this situation by specifying a geometry in the command
line.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration-test.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 40de320fcc..74f3b5f772 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -757,7 +757,9 @@ static int test_migrate_start(QTestState **from, QTestState **to,
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         memory_size = "150M";
         machine = "pc";
-        arch_opts = g_strdup_printf("-drive file=%s,format=raw", bootpath);
+        arch_opts = g_strdup_printf(
+            "-drive if=none,id=d0,file=%s,format=raw "
+            "-device ide-hd,drive=d0,secs=1,cyls=1,heads=1", bootpath);
         start_address = X86_TEST_MEM_START;
         end_address = X86_TEST_MEM_END;
     } else if (g_str_equal(arch, "s390x")) {
-- 
2.35.3

[PATCH v3 09/11] tests/qtest/migration: Set q35 as the default machine for x86_86

[Fabiano Rosas]

Change the x86_64 to use the q35 machines in tests from now on. Keep
testing the pc macine on 32bit.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
this could affect bisecting, so I put it in separate patch to be
easier to revert
---
 tests/qtest/migration-test.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 74f3b5f772..209e611061 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -756,7 +756,12 @@ static int test_migrate_start(QTestState **from, QTestState **to,
     got_dst_resume = false;
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         memory_size = "150M";
-        machine = "pc";
+
+        if (g_str_equal(arch, "i386")) {
+            machine = "pc";
+        } else {
+            machine = "q35";
+        }
         arch_opts = g_strdup_printf(
             "-drive if=none,id=d0,file=%s,format=raw "
             "-device ide-hd,drive=d0,secs=1,cyls=1,heads=1", bootpath);
-- 
2.35.3

[PATCH v3 10/11] tests/qtest/migration: Support more than one QEMU binary

[Fabiano Rosas]

We have strict rules around migration compatibility between different
QEMU versions but no test to validate the migration state between
different binaries.

Add infrastructure to allow running the migration tests with two
different QEMU binaries as migration source and destination.

The code now recognizes two new environment variables
QTEST_QEMU_BINARY_SRC and QTEST_QEMU_BINARY_DST. In the absence of
either of them, the test will use the QTEST_QEMU_BINARY variable. If
both are missing then the tests are run with single binary as
previously.

The machine type is selected automatically as the latest machine type
version that works with both binaries.

Usage:
QTEST_QEMU_BINARY_SRC=../build-8.2.0/qemu-system-x86_64 \
QTEST_QEMU_BINARY_DST=../build-8.1.0/qemu-system-x86_64 \
./tests/qtest/migration-test

Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration-test.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 209e611061..11ac414e56 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -71,6 +71,8 @@ static bool got_dst_resume;
 #define QEMU_VM_FILE_MAGIC 0x5145564d
 #define FILE_TEST_FILENAME "migfile"
 #define FILE_TEST_OFFSET 0x1000
+#define QEMU_ENV_SRC "QTEST_QEMU_BINARY_SRC"
+#define QEMU_ENV_DST "QTEST_QEMU_BINARY_DST"
 
 #if defined(__linux__)
 #include <sys/syscall.h>
@@ -744,6 +746,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
     const char *arch = qtest_get_arch();
     const char *memory_size;
     const char *machine;
+    g_autofree char *machine_type = NULL;
 
     if (args->use_shmem) {
         if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) {
@@ -821,6 +824,10 @@ static int test_migrate_start(QTestState **from, QTestState **to,
         kvm_opts = ",dirty-ring-size=4096";
     }
 
+    machine_type = find_common_machine_version(machine, QEMU_ENV_SRC,
+                                               QEMU_ENV_DST);
+    g_test_message("Using machine type: %s", machine_type);
+
     cmd_source = g_strdup_printf("-accel kvm%s -accel tcg "
                                  "-machine %s "
                                  "-name source,debug-threads=on "
@@ -828,7 +835,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                  "-serial file:%s/src_serial "
                                  "%s %s %s %s %s",
                                  kvm_opts ? kvm_opts : "",
-                                 machine,
+                                 machine_type,
                                  memory_size, tmpfs,
                                  arch_opts ? arch_opts : "",
                                  arch_source ? arch_source : "",
@@ -836,7 +843,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                  args->opts_source ? args->opts_source : "",
                                  ignore_stderr);
     if (!args->only_target) {
-        *from = qtest_init(cmd_source);
+        *from = qtest_init_with_env(QEMU_ENV_SRC, cmd_source);
         qtest_qmp_set_event_callback(*from,
                                      migrate_watch_for_stop,
                                      &got_src_stop);
@@ -850,14 +857,14 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                  "-incoming %s "
                                  "%s %s %s %s %s",
                                  kvm_opts ? kvm_opts : "",
-                                 machine,
+                                 machine_type,
                                  memory_size, tmpfs, uri,
                                  arch_opts ? arch_opts : "",
                                  arch_target ? arch_target : "",
                                  shmem_opts ? shmem_opts : "",
                                  args->opts_target ? args->opts_target : "",
                                  ignore_stderr);
-    *to = qtest_init(cmd_target);
+    *to = qtest_init_with_env(QEMU_ENV_DST, cmd_target);
     qtest_qmp_set_event_callback(*to,
                                  migrate_watch_for_resume,
                                  &got_dst_resume);
@@ -2988,10 +2995,23 @@ int main(int argc, char **argv)
     bool has_uffd;
     const char *arch;
     g_autoptr(GError) err = NULL;
+    const char *qemu_src = getenv(QEMU_ENV_SRC);
+    const char *qemu_dst = getenv(QEMU_ENV_DST);
     int ret;
 
     g_test_init(&argc, &argv, NULL);
 
+    /*
+     * The default QTEST_QEMU_BINARY must always be provided because
+     * that is what helpers use to query the accel type and
+     * architecture.
+     */
+    if (qemu_src && qemu_dst) {
+        g_test_message("Only one of %s, %s is allowed",
+                       QEMU_ENV_SRC, QEMU_ENV_DST);
+        exit(1);
+    }
+
     has_kvm = qtest_has_accel("kvm");
     has_tcg = qtest_has_accel("tcg");
 
-- 
2.35.3

[PATCH v3 11/11] tests/qtest: Don't print messages from query instances

[Fabiano Rosas]

Now that we can query more than one binary, the "starting QEMU..."
message can get a little noisy. Mute those messages unless we're
running with --verbose.

Only affects qtest_init() calls from within libqtest. The tests
continue to output as usual.

Reviewed-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/libqtest.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index fda163836c..1ac4f7d36a 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -91,6 +91,7 @@ struct QTestState
 
 static GHookList abrt_hooks;
 static void (*sighandler_old)(int);
+static bool silence_spawn_log;
 
 static int qtest_query_target_endianness(QTestState *s);
 
@@ -405,7 +406,9 @@ static QTestState *G_GNUC_PRINTF(2, 3) qtest_spawn_qemu(const char *qemu_bin,
 
     qtest_add_abrt_handler(kill_qemu_hook_func, s);
 
-    g_test_message("starting QEMU: %s", command->str);
+    if (!silence_spawn_log) {
+        g_test_message("starting QEMU: %s", command->str);
+    }
 
 #ifndef _WIN32
     s->qemu_pid = fork();
@@ -1496,6 +1499,8 @@ static struct MachInfo *qtest_get_machines(const char *var)
         return machines;
     }
 
+    silence_spawn_log = !g_test_verbose();
+
     qts = qtest_init_with_env(qemu_var, "-machine none");
     response = qtest_qmp(qts, "{ 'execute': 'query-machines' }");
     g_assert(response);
@@ -1527,6 +1532,8 @@ static struct MachInfo *qtest_get_machines(const char *var)
     qtest_quit(qts);
     qobject_unref(response);
 
+    silence_spawn_log = false;
+
     memset(&machines[idx], 0, sizeof(struct MachInfo)); /* Terminating entry */
     return machines;
 }
-- 
2.35.3

Re: [PATCH v3 02/11] tests/qtest: Introduce qtest_init_with_env

[Juan Quintela]

Fabiano Rosas <farosas@suse.de> wrote:
> Add a version of qtest_init() that takes an environment variable
> containing the path of the QEMU binary. This allows tests to use more
> than one QEMU binary.
>
> If no variable is provided or the environment variable does not exist,
> that is not an error. Fallback to using QTEST_QEMU_BINARY.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

Reviewed-by: Juan Quintela <quintela@redhat.com>

But I need that thomas or someone else also agrees.

Re: [PATCH v3 00/11] tests/migration-test: Allow testing older machine types

[Fabiano Rosas]

Fabiano Rosas <farosas@suse.de> writes:

> This adds support for running migration-test with two different QEMU
> versions to test migration compatibility. The tests automatically
> choose the latest machine type supported by both QEMU versions.

Let me point out two situations that came up while developing this:

1) There was a generic command line change in QEMU (-audio option)
   introduced this cycle that was ported to the command line used by
   libqtest.c. This means that when using the same command line to
   start both guests, the older one will not recognize the option and
   will break.

   Ideally, we would keep command lines out of the library and require
   each test to provide their own. Then maybe the migration code could
   cope with the change somehow.

2) When fixing an issue during a release cycle, that issue can still
   show up on the next one when using this series to test
   compatibility. There's probably not much we can do here, but
   remember the issues that were already fixed and ignore them when
   they appear in these compatibiliy tests.

I don't see how to avoid the situations above, but I think their
existence implies we cannot enable by default any sort of migration
compatibility testing that uses a live guest.

Re: [PATCH v3 10/11] tests/qtest/migration: Support more than one QEMU binary

[Juan Quintela]

Fabiano Rosas <farosas@suse.de> wrote:
D> We have strict rules around migration compatibility between different
> QEMU versions but no test to validate the migration state between
> different binaries.
>
> Add infrastructure to allow running the migration tests with two
> different QEMU binaries as migration source and destination.
>
> The code now recognizes two new environment variables
> QTEST_QEMU_BINARY_SRC and QTEST_QEMU_BINARY_DST. In the absence of
> either of them, the test will use the QTEST_QEMU_BINARY variable. If
> both are missing then the tests are run with single binary as
> previously.
>
> The machine type is selected automatically as the latest machine type
> version that works with both binaries.
>
> Usage:
> QTEST_QEMU_BINARY_SRC=../build-8.2.0/qemu-system-x86_64 \
> QTEST_QEMU_BINARY_DST=../build-8.1.0/qemu-system-x86_64 \
> ./tests/qtest/migration-test
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

The test works for me.  But I would really like to be able to specify
the machine type for which I have to test.  I.e. right now, we can test:

qemu-8.2 <-> qemu-8.1

and it is going to use q35-8.1

But in the case that I want to test that two binaries with q35-8.0,
there is no way to setup that.

So basically what I need is

QTEST_QEMU_MACHINE_TYPE var, and if that exist, just use that instead of
the value of "machine"

What do you think?

Later, Juan.

Re: [PATCH v3 00/11] tests/migration-test: Allow testing older machine types

[Daniel P. Berrangé]

On Wed, Oct 18, 2023 at 11:19:01AM -0300, Fabiano Rosas wrote:
> Fabiano Rosas <farosas@suse.de> writes:
> 
> > This adds support for running migration-test with two different QEMU
> > versions to test migration compatibility. The tests automatically
> > choose the latest machine type supported by both QEMU versions.
> 
> Let me point out two situations that came up while developing this:
> 
> 1) There was a generic command line change in QEMU (-audio option)
>    introduced this cycle that was ported to the command line used by
>    libqtest.c. This means that when using the same command line to
>    start both guests, the older one will not recognize the option and
>    will break.
> 
>    Ideally, we would keep command lines out of the library and require
>    each test to provide their own. Then maybe the migration code could
>    cope with the change somehow.

Right the problem is that QEMU doesn't provide a long term stable
API, but has finite sliding windows where QEMU versions can be
compatible in their CLI, determined by our minimal deprecation
period of 2 releases.

eg you can come up with a CLI that is compatible for all of 7.1, 7.2
and 8.0 but you can't neccessarily come up with a CLI that is compatible
for all of 7.1, 7.2, 8.0 and 8.1 because the lifespan of that set of
versions is greater than our deprecation period.

If we did't use -audio in libqtest, we could be compatible with current
QEMU and some old QEMUs, but not future QEMU.

If we do use -audio in libqtest, we could be compatible with current
QEMU and some future QEMUs, but not old QEMUs.

This is a no win scenario.

Now, libvirt provides a long term stable configuration for QEMU that
transcends changes in QEMU CLI.

So we could come up with a live migration test between QEMU's of an
arbitrary age difference, if we used libvirt to drive the process.
That wouldn't really be qtest though. Could perhaps we done in the
context of avocado, if we used the libvirt-python API binding, but
this would be different from other avocado tests by virtue of not
using the QEMU python classes for launch.

> 2) When fixing an issue during a release cycle, that issue can still
>    show up on the next one when using this series to test
>    compatibility. There's probably not much we can do here, but
>    remember the issues that were already fixed and ignore them when
>    they appear in these compatibiliy tests.
> 
> I don't see how to avoid the situations above, but I think their
> existence implies we cannot enable by default any sort of migration
> compatibility testing that uses a live guest.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3 10/11] tests/qtest/migration: Support more than one QEMU binary

[Fabiano Rosas]

Juan Quintela <quintela@redhat.com> writes:

> Fabiano Rosas <farosas@suse.de> wrote:
> D> We have strict rules around migration compatibility between different
>> QEMU versions but no test to validate the migration state between
>> different binaries.
>>
>> Add infrastructure to allow running the migration tests with two
>> different QEMU binaries as migration source and destination.
>>
>> The code now recognizes two new environment variables
>> QTEST_QEMU_BINARY_SRC and QTEST_QEMU_BINARY_DST. In the absence of
>> either of them, the test will use the QTEST_QEMU_BINARY variable. If
>> both are missing then the tests are run with single binary as
>> previously.
>>
>> The machine type is selected automatically as the latest machine type
>> version that works with both binaries.
>>
>> Usage:
>> QTEST_QEMU_BINARY_SRC=../build-8.2.0/qemu-system-x86_64 \
>> QTEST_QEMU_BINARY_DST=../build-8.1.0/qemu-system-x86_64 \
>> ./tests/qtest/migration-test
>>
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>
> The test works for me.  But I would really like to be able to specify
> the machine type for which I have to test.  I.e. right now, we can test:
>
> qemu-8.2 <-> qemu-8.1
>
> and it is going to use q35-8.1
>
> But in the case that I want to test that two binaries with q35-8.0,
> there is no way to setup that.

Sorry, I didn't catch that you wanted to test versions other than the
latest when you mentioned it the first time.

> So basically what I need is
>
> QTEST_QEMU_MACHINE_TYPE var, and if that exist, just use that instead of
> the value of "machine"
>
> What do you think?

I don't see why not.

I have to resend this series because I sent the wrong version by
accident. I'll add the new var then.

Re: [PATCH v3 10/11] tests/qtest/migration: Support more than one QEMU binary

[Juan Quintela]

Fabiano Rosas <farosas@suse.de> wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
>> Fabiano Rosas <farosas@suse.de> wrote:
>> D> We have strict rules around migration compatibility between different
>>> QEMU versions but no test to validate the migration state between
>>> different binaries.
>>>
>>> Add infrastructure to allow running the migration tests with two
>>> different QEMU binaries as migration source and destination.
>>>
>>> The code now recognizes two new environment variables
>>> QTEST_QEMU_BINARY_SRC and QTEST_QEMU_BINARY_DST. In the absence of
>>> either of them, the test will use the QTEST_QEMU_BINARY variable. If
>>> both are missing then the tests are run with single binary as
>>> previously.
>>>
>>> The machine type is selected automatically as the latest machine type
>>> version that works with both binaries.
>>>
>>> Usage:
>>> QTEST_QEMU_BINARY_SRC=../build-8.2.0/qemu-system-x86_64 \
>>> QTEST_QEMU_BINARY_DST=../build-8.1.0/qemu-system-x86_64 \
>>> ./tests/qtest/migration-test
>>>
>>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>>
>> The test works for me.  But I would really like to be able to specify
>> the machine type for which I have to test.  I.e. right now, we can test:
>>
>> qemu-8.2 <-> qemu-8.1
>>
>> and it is going to use q35-8.1
>>
>> But in the case that I want to test that two binaries with q35-8.0,
>> there is no way to setup that.
>
> Sorry, I didn't catch that you wanted to test versions other than the
> latest when you mentioned it the first time.
>
>> So basically what I need is
>>
>> QTEST_QEMU_MACHINE_TYPE var, and if that exist, just use that instead of
>> the value of "machine"
>>
>> What do you think?
>
> I don't see why not.
>
> I have to resend this series because I sent the wrong version by
> accident. I'll add the new var then.

Thanks.

Re: [PATCH v3 10/11] tests/qtest/migration: Support more than one QEMU binary

[Peter Xu]

On Wed, Oct 18, 2023 at 11:07:35AM -0300, Fabiano Rosas wrote:
> Usage:
> QTEST_QEMU_BINARY_SRC=../build-8.2.0/qemu-system-x86_64 \
> QTEST_QEMU_BINARY_DST=../build-8.1.0/qemu-system-x86_64 \
> ./tests/qtest/migration-test

Please also touch up this example when repost; I think it's not allowed
now..

-- 
Peter Xu

Re: [PATCH v3 00/11] tests/migration-test: Allow testing older machine types

[Peter Xu]

On Wed, Oct 18, 2023 at 11:19:01AM -0300, Fabiano Rosas wrote:
> Let me point out two situations that came up while developing this:
> 
> 1) There was a generic command line change in QEMU (-audio option)
>    introduced this cycle that was ported to the command line used by
>    libqtest.c. This means that when using the same command line to
>    start both guests, the older one will not recognize the option and
>    will break.
> 
>    Ideally, we would keep command lines out of the library and require
>    each test to provide their own. Then maybe the migration code could
>    cope with the change somehow.

Yeah this is unfortunate, and I hit it as soon as trying this out with an
old binary.

One other thing on top of this: the cross binary tests for different
devices, where we used to hit issues when some specific pci device or a
pcie root port failed to be compatible with old binaries, iirc.

I'm still not sure what's the best way to do that, though, especially if
the migration failure can only trigger when a device driver setup the
device first.  Maybe we can start with migrating a device with the reset
status between binaries.  Having this series is definitely better than
nothing already.

Thanks,

-- 
Peter Xu