[RFC PATCH 0/1] tests/migration-test: Allow testing older machine types

[RFC PATCH 0/1] tests/migration-test: Allow testing older machine types

[Fabiano Rosas]

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 (1):
  qtest/migration: Support more than one QEMU binary

 tests/qtest/migration-helpers.c | 168 ++++++++++++++++++++++++++++++++
 tests/qtest/migration-helpers.h |   3 +
 tests/qtest/migration-test.c    |  52 ++++++++--
 3 files changed, 216 insertions(+), 7 deletions(-)

-- 
2.35.3

[RFC PATCH 1/1] qtest/migration: Support more than one QEMU binary

[Fabiano Rosas]

We have strict rules around migration compatibility between different
QEMU versions but not a single test that validates the migration state
between different binaries.

Add some 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_SRC
and QTEST_QEMU_DST. Only one of the two is expected to be used along
with the existing QTEST_QEMU_BINARY, which will automatically be used
for the other side of migration.

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

This code also works for when debugging with GDB to pass the same
binary, but different GDB options for src and dst.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 tests/qtest/migration-helpers.c | 168 ++++++++++++++++++++++++++++++++
 tests/qtest/migration-helpers.h |   3 +
 tests/qtest/migration-test.c    |  52 ++++++++--
 3 files changed, 216 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
index be00c52d00..e84360c3b3 100644
--- a/tests/qtest/migration-helpers.c
+++ b/tests/qtest/migration-helpers.c
@@ -12,6 +12,8 @@
 
 #include "qemu/osdep.h"
 #include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qstring.h"
 
 #include "migration-helpers.h"
 
@@ -180,3 +182,169 @@ void wait_for_migration_fail(QTestState *from, bool allow_active)
     g_assert(qdict_get_bool(rsp_return, "running"));
     qobject_unref(rsp_return);
 }
+
+static char *query_pkg_version(QTestState *who)
+{
+    QDict *rsp;
+    char *pkg;
+
+    rsp = qtest_qmp_assert_success_ref(who, "{ 'execute': 'query-version' }");
+    g_assert(rsp);
+
+    pkg = g_strdup(qdict_get_str(rsp, "package"));
+    qobject_unref(rsp);
+
+    return pkg;
+}
+
+static QList *query_machines(void)
+{
+    QDict *response;
+    QList *list;
+    QTestState *qts;
+
+    qts = qtest_init("-machine none");
+    response = qtest_qmp(qts, "{ 'execute': 'query-machines' }");
+    g_assert(response);
+    list = qdict_get_qlist(response, "return");
+    g_assert(list);
+
+    qtest_quit(qts);
+    return list;
+}
+
+static char *get_default_machine(QList *list)
+{
+    QDict *info;
+    QListEntry *entry;
+    QString *qstr;
+    char *name = NULL;
+
+    QLIST_FOREACH_ENTRY(list, entry) {
+        info = qobject_to(QDict, qlist_entry_obj(entry));
+        g_assert(info);
+
+        if (qdict_get(info, "is-default")) {
+            qstr = qobject_to(QString, qdict_get(info, "name"));
+            g_assert(qstr);
+            name = g_strdup(qstring_get_str(qstr));
+            break;
+        }
+    }
+
+    g_assert(name);
+    return name;
+}
+
+static bool search_default_machine(QList *list, const char *theirs)
+{
+    QDict *info;
+    QListEntry *entry;
+    QString *qstr;
+
+    if (!theirs) {
+        return false;
+    }
+
+    QLIST_FOREACH_ENTRY(list, entry) {
+        info = qobject_to(QDict, qlist_entry_obj(entry));
+        g_assert(info);
+
+        qstr = qobject_to(QString, qdict_get(info, "name"));
+        g_assert(qstr);
+
+        if (g_str_equal(qstring_get_str(qstr), theirs)) {
+            return true;
+        }
+    }
+    return false;
+}
+
+/*
+ * We need to ensure that both QEMU instances set via the QTEST_QEMU_*
+ * vars will use the same machine type. Use a custom query_machines
+ * function because the generic one in libqtest has a cache that would
+ * return the same machines for both binaries.
+ */
+char *find_common_machine_type(const char *bin)
+{
+    QList *m1, *m2;
+    g_autofree char *def1 = NULL;
+    g_autofree char *def2 = NULL;
+    const char *qemu_bin = getenv("QTEST_QEMU_BINARY");
+
+    m1 = query_machines();
+
+    g_setenv("QTEST_QEMU_BINARY", bin, true);
+    m2 = query_machines();
+    g_setenv("QTEST_QEMU_BINARY", qemu_bin, true);
+
+    def1 = get_default_machine(m1);
+    def2 = get_default_machine(m2);
+
+    if (g_str_equal(def1, def2)) {
+        /* either can be used */
+        return g_strdup(def1);
+    }
+
+    if (search_default_machine(m1, def2)) {
+        return g_strdup(def2);
+    }
+
+    if (search_default_machine(m2, def1)) {
+        return g_strdup(def1);
+    }
+
+    g_assert_not_reached();
+}
+
+/*
+ * Init a guest for migration tests using an alternate QEMU binary for
+ * either the source or destination, depending on @var. The other
+ * binary should be set as usual via QTEST_QEMU_BINARY.
+ *
+ * Expected values:
+ *   QTEST_QEMU_SRC
+ *   QTEST_QEMU_DST
+ *
+ * Warning: The generic parts of qtest could be using
+ * QTEST_QEMU_BINARY to query for properties before we reach the
+ * migration code. If the alternate binary is too dissimilar that
+ * could cause issues.
+ */
+static QTestState *init_vm(const char *extra_args, const char *var)
+{
+    const char *alt_bin = getenv(var);
+    const char *qemu_bin = getenv("QTEST_QEMU_BINARY");
+    g_autofree char *pkg = NULL;
+    bool src = !!strstr(var, "SRC");
+    QTestState *qts;
+
+    if (alt_bin) {
+        g_setenv("QTEST_QEMU_BINARY", alt_bin, true);
+    }
+
+    qts = qtest_init(extra_args);
+    pkg = query_pkg_version(qts);
+
+    g_test_message("Using %s (%s) as migration %s",
+                   alt_bin ? alt_bin : qemu_bin,
+                   pkg,
+                   src ? "source" : "destination");
+
+    if (alt_bin) {
+        /* restore the original */
+        g_setenv("QTEST_QEMU_BINARY", qemu_bin, true);
+    }
+    return qts;
+}
+
+QTestState *mig_init_src(const char *extra_args)
+{
+    return init_vm(extra_args, "QTEST_QEMU_SRC");
+}
+
+QTestState *mig_init_dst(const char *extra_args)
+{
+    return init_vm(extra_args, "QTEST_QEMU_DST");
+}
diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
index 009e250e90..aabdbc7507 100644
--- a/tests/qtest/migration-helpers.h
+++ b/tests/qtest/migration-helpers.h
@@ -33,4 +33,7 @@ void wait_for_migration_complete(QTestState *who);
 
 void wait_for_migration_fail(QTestState *from, bool allow_active);
 
+QTestState *mig_init_src(const char *extra_args);
+QTestState *mig_init_dst(const char *extra_args);
+char *find_common_machine_type(const char *bin);
 #endif /* MIGRATION_HELPERS_H */
diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 1b43df5ca7..60f0b15417 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -45,6 +45,7 @@ unsigned end_address;
 static bool uffd_feature_thread_id;
 static bool got_src_stop;
 static bool got_dst_resume;
+static char *common_machine_type;
 
 /*
  * An initial 3 MB offset is used as that corresponds
@@ -712,6 +713,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
     g_autofree char *shmem_path = NULL;
     const char *arch = qtest_get_arch();
     const char *memory_size;
+    g_autofree char *machine = NULL;
 
     if (args->use_shmem) {
         if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) {
@@ -723,18 +725,30 @@ static int test_migrate_start(QTestState **from, QTestState **to,
     got_src_stop = false;
     got_dst_resume = false;
     bootpath = g_strdup_printf("%s/bootsect", tmpfs);
+
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
         /* the assembled x86 boot sector should be exactly one sector large */
         assert(sizeof(x86_bootsect) == 512);
         init_bootfile(bootpath, x86_bootsect, sizeof(x86_bootsect));
         memory_size = "150M";
-        arch_opts = g_strdup_printf("-drive file=%s,format=raw", bootpath);
+
+        if (common_machine_type) {
+            machine = g_strdup_printf("-machine %s", common_machine_type);
+        }
+
+        arch_opts = g_strdup_printf("%s -drive file=%s,format=raw",
+                                    machine, bootpath);
         start_address = X86_TEST_MEM_START;
         end_address = X86_TEST_MEM_END;
     } else if (g_str_equal(arch, "s390x")) {
         init_bootfile(bootpath, s390x_elf, sizeof(s390x_elf));
         memory_size = "128M";
-        arch_opts = g_strdup_printf("-bios %s", bootpath);
+
+        if (common_machine_type) {
+            machine = g_strdup_printf("-machine %s", common_machine_type);
+        }
+
+        arch_opts = g_strdup_printf("%s -bios %s", machine, bootpath);
         start_address = S390_TEST_MEM_START;
         end_address = S390_TEST_MEM_END;
     } else if (strcmp(arch, "ppc64") == 0) {
@@ -745,12 +759,24 @@ 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);
-        arch_opts = g_strdup("-nodefaults -machine vsmt=8");
+
+        if (common_machine_type) {
+            machine = g_strdup_printf("%s,", common_machine_type);
+        }
+
+        arch_opts = g_strdup_printf("-nodefaults -machine %svsmt=8", machine);
     } else if (strcmp(arch, "aarch64") == 0) {
         init_bootfile(bootpath, aarch64_kernel, sizeof(aarch64_kernel));
         memory_size = "150M";
-        arch_opts = g_strdup_printf("-machine virt,gic-version=max -cpu max "
-                                    "-kernel %s", bootpath);
+
+        if (common_machine_type) {
+            machine = g_strdup_printf("%s", common_machine_type);
+        } else {
+            machine = g_strdup("virt");
+        }
+
+        arch_opts = g_strdup_printf("-machine %s,gic-version=max -cpu max "
+                                    "-kernel %s", machine, bootpath);
         start_address = ARM_TEST_MEM_START;
         end_address = ARM_TEST_MEM_END;
 
@@ -799,7 +825,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 = mig_init_src(cmd_source);
         qtest_qmp_set_event_callback(*from,
                                      migrate_watch_for_stop,
                                      &got_src_stop);
@@ -819,7 +845,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
                                  shmem_opts,
                                  args->opts_target ? args->opts_target : "",
                                  ignore_stderr);
-    *to = qtest_init(cmd_target);
+    *to = mig_init_dst(cmd_target);
     qtest_qmp_set_event_callback(*to,
                                  migrate_watch_for_resume,
                                  &got_dst_resume);
@@ -2769,6 +2795,8 @@ int main(int argc, char **argv)
     const char *arch;
     g_autoptr(GError) err = NULL;
     int ret;
+    const char *qemu_src = getenv("QTEST_QEMU_SRC");
+    const char *qemu_dst = getenv("QTEST_QEMU_DST");
 
     g_test_init(&argc, &argv, NULL);
 
@@ -2780,6 +2808,16 @@ int main(int argc, char **argv)
         return 0;
     }
 
+    if (qemu_src || qemu_dst) {
+        if (qemu_src && qemu_dst) {
+            g_test_message("Only one of QTEST_QEMU_SRC, QTEST_QEMU_DST is allowed.");
+            exit(1);
+        }
+        common_machine_type = find_common_machine_type(qemu_src ? qemu_src : qemu_dst);
+        g_test_message("Using two different QEMU binaries. Common machine type: %s",
+                       common_machine_type);
+    }
+
     has_uffd = ufd_version_check();
     arch = qtest_get_arch();
 
-- 
2.35.3

Re: [RFC PATCH 1/1] qtest/migration: Support more than one QEMU binary

[Philippe Mathieu-Daudé]

Hi Fabiano,

[+Alex & Daniel]

On 3/10/23 16:19, Fabiano Rosas wrote:
> We have strict rules around migration compatibility between different
> QEMU versions but not a single test that validates the migration state
> between different binaries.
> 
> Add some 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_SRC
> and QTEST_QEMU_DST. Only one of the two is expected to be used along
> with the existing QTEST_QEMU_BINARY, which will automatically be used
> for the other side of migration.
> 
> Usage:
> QTEST_QEMU_DST=./build-8.2.0/qemu-system-x86_64 \
> QTEST_QEMU_BINARY=../build-8.1.0/qemu-system-x86_64 \
> ./tests/qtest/migration-test

I like it as a first step, but I'd rather run $QTEST_QEMU_SRC
directly from a docker image, i.e.:

$ docker run -it opensuse/leap
# zypper update -y && zypper install -y qemu-x86
$ docker run opensuse/leap qemu-system-x86_64 ...

> This code also works for when debugging with GDB to pass the same
> binary, but different GDB options for src and dst.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>   tests/qtest/migration-helpers.c | 168 ++++++++++++++++++++++++++++++++
>   tests/qtest/migration-helpers.h |   3 +
>   tests/qtest/migration-test.c    |  52 ++++++++--
>   3 files changed, 216 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> index be00c52d00..e84360c3b3 100644
> --- a/tests/qtest/migration-helpers.c
> +++ b/tests/qtest/migration-helpers.c
> @@ -12,6 +12,8 @@
>   
>   #include "qemu/osdep.h"
>   #include "qapi/qmp/qjson.h"
> +#include "qapi/qmp/qlist.h"
> +#include "qapi/qmp/qstring.h"
>   
>   #include "migration-helpers.h"
>   
> @@ -180,3 +182,169 @@ void wait_for_migration_fail(QTestState *from, bool allow_active)
>       g_assert(qdict_get_bool(rsp_return, "running"));
>       qobject_unref(rsp_return);
>   }
> +
> +static char *query_pkg_version(QTestState *who)
> +{
> +    QDict *rsp;
> +    char *pkg;
> +
> +    rsp = qtest_qmp_assert_success_ref(who, "{ 'execute': 'query-version' }");
> +    g_assert(rsp);
> +
> +    pkg = g_strdup(qdict_get_str(rsp, "package"));
> +    qobject_unref(rsp);
> +
> +    return pkg;
> +}
> +
> +static QList *query_machines(void)
> +{
> +    QDict *response;
> +    QList *list;
> +    QTestState *qts;
> +
> +    qts = qtest_init("-machine none");
> +    response = qtest_qmp(qts, "{ 'execute': 'query-machines' }");
> +    g_assert(response);
> +    list = qdict_get_qlist(response, "return");
> +    g_assert(list);
> +
> +    qtest_quit(qts);
> +    return list;
> +}
> +
> +static char *get_default_machine(QList *list)
> +{
> +    QDict *info;
> +    QListEntry *entry;
> +    QString *qstr;
> +    char *name = NULL;
> +
> +    QLIST_FOREACH_ENTRY(list, entry) {
> +        info = qobject_to(QDict, qlist_entry_obj(entry));
> +        g_assert(info);
> +
> +        if (qdict_get(info, "is-default")) {
> +            qstr = qobject_to(QString, qdict_get(info, "name"));
> +            g_assert(qstr);
> +            name = g_strdup(qstring_get_str(qstr));
> +            break;
> +        }
> +    }
> +
> +    g_assert(name);
> +    return name;
> +}
> +
> +static bool search_default_machine(QList *list, const char *theirs)
> +{
> +    QDict *info;
> +    QListEntry *entry;
> +    QString *qstr;
> +
> +    if (!theirs) {
> +        return false;
> +    }
> +
> +    QLIST_FOREACH_ENTRY(list, entry) {
> +        info = qobject_to(QDict, qlist_entry_obj(entry));
> +        g_assert(info);
> +
> +        qstr = qobject_to(QString, qdict_get(info, "name"));
> +        g_assert(qstr);
> +
> +        if (g_str_equal(qstring_get_str(qstr), theirs)) {
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
> +/*
> + * We need to ensure that both QEMU instances set via the QTEST_QEMU_*
> + * vars will use the same machine type. Use a custom query_machines
> + * function because the generic one in libqtest has a cache that would
> + * return the same machines for both binaries.
> + */
> +char *find_common_machine_type(const char *bin)
> +{
> +    QList *m1, *m2;
> +    g_autofree char *def1 = NULL;
> +    g_autofree char *def2 = NULL;
> +    const char *qemu_bin = getenv("QTEST_QEMU_BINARY");
> +
> +    m1 = query_machines();
> +
> +    g_setenv("QTEST_QEMU_BINARY", bin, true);
> +    m2 = query_machines();
> +    g_setenv("QTEST_QEMU_BINARY", qemu_bin, true);
> +
> +    def1 = get_default_machine(m1);
> +    def2 = get_default_machine(m2);
> +
> +    if (g_str_equal(def1, def2)) {
> +        /* either can be used */
> +        return g_strdup(def1);
> +    }
> +
> +    if (search_default_machine(m1, def2)) {
> +        return g_strdup(def2);
> +    }
> +
> +    if (search_default_machine(m2, def1)) {
> +        return g_strdup(def1);
> +    }
> +
> +    g_assert_not_reached();
> +}
> +
> +/*
> + * Init a guest for migration tests using an alternate QEMU binary for
> + * either the source or destination, depending on @var. The other
> + * binary should be set as usual via QTEST_QEMU_BINARY.
> + *
> + * Expected values:
> + *   QTEST_QEMU_SRC
> + *   QTEST_QEMU_DST
> + *
> + * Warning: The generic parts of qtest could be using
> + * QTEST_QEMU_BINARY to query for properties before we reach the
> + * migration code. If the alternate binary is too dissimilar that
> + * could cause issues.
> + */
> +static QTestState *init_vm(const char *extra_args, const char *var)
> +{
> +    const char *alt_bin = getenv(var);
> +    const char *qemu_bin = getenv("QTEST_QEMU_BINARY");
> +    g_autofree char *pkg = NULL;
> +    bool src = !!strstr(var, "SRC");
> +    QTestState *qts;
> +
> +    if (alt_bin) {
> +        g_setenv("QTEST_QEMU_BINARY", alt_bin, true);
> +    }
> +
> +    qts = qtest_init(extra_args);
> +    pkg = query_pkg_version(qts);
> +
> +    g_test_message("Using %s (%s) as migration %s",
> +                   alt_bin ? alt_bin : qemu_bin,
> +                   pkg,
> +                   src ? "source" : "destination");
> +
> +    if (alt_bin) {
> +        /* restore the original */
> +        g_setenv("QTEST_QEMU_BINARY", qemu_bin, true);
> +    }
> +    return qts;
> +}
> +
> +QTestState *mig_init_src(const char *extra_args)
> +{
> +    return init_vm(extra_args, "QTEST_QEMU_SRC");
> +}
> +
> +QTestState *mig_init_dst(const char *extra_args)
> +{
> +    return init_vm(extra_args, "QTEST_QEMU_DST");
> +}
> diff --git a/tests/qtest/migration-helpers.h b/tests/qtest/migration-helpers.h
> index 009e250e90..aabdbc7507 100644
> --- a/tests/qtest/migration-helpers.h
> +++ b/tests/qtest/migration-helpers.h
> @@ -33,4 +33,7 @@ void wait_for_migration_complete(QTestState *who);
>   
>   void wait_for_migration_fail(QTestState *from, bool allow_active);
>   
> +QTestState *mig_init_src(const char *extra_args);
> +QTestState *mig_init_dst(const char *extra_args);
> +char *find_common_machine_type(const char *bin);
>   #endif /* MIGRATION_HELPERS_H */
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 1b43df5ca7..60f0b15417 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -45,6 +45,7 @@ unsigned end_address;
>   static bool uffd_feature_thread_id;
>   static bool got_src_stop;
>   static bool got_dst_resume;
> +static char *common_machine_type;
>   
>   /*
>    * An initial 3 MB offset is used as that corresponds
> @@ -712,6 +713,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>       g_autofree char *shmem_path = NULL;
>       const char *arch = qtest_get_arch();
>       const char *memory_size;
> +    g_autofree char *machine = NULL;
>   
>       if (args->use_shmem) {
>           if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) {
> @@ -723,18 +725,30 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>       got_src_stop = false;
>       got_dst_resume = false;
>       bootpath = g_strdup_printf("%s/bootsect", tmpfs);
> +
>       if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
>           /* the assembled x86 boot sector should be exactly one sector large */
>           assert(sizeof(x86_bootsect) == 512);
>           init_bootfile(bootpath, x86_bootsect, sizeof(x86_bootsect));
>           memory_size = "150M";
> -        arch_opts = g_strdup_printf("-drive file=%s,format=raw", bootpath);
> +
> +        if (common_machine_type) {
> +            machine = g_strdup_printf("-machine %s", common_machine_type);
> +        }
> +
> +        arch_opts = g_strdup_printf("%s -drive file=%s,format=raw",
> +                                    machine, bootpath);
>           start_address = X86_TEST_MEM_START;
>           end_address = X86_TEST_MEM_END;
>       } else if (g_str_equal(arch, "s390x")) {
>           init_bootfile(bootpath, s390x_elf, sizeof(s390x_elf));
>           memory_size = "128M";
> -        arch_opts = g_strdup_printf("-bios %s", bootpath);
> +
> +        if (common_machine_type) {
> +            machine = g_strdup_printf("-machine %s", common_machine_type);
> +        }
> +
> +        arch_opts = g_strdup_printf("%s -bios %s", machine, bootpath);
>           start_address = S390_TEST_MEM_START;
>           end_address = S390_TEST_MEM_END;
>       } else if (strcmp(arch, "ppc64") == 0) {
> @@ -745,12 +759,24 @@ 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);
> -        arch_opts = g_strdup("-nodefaults -machine vsmt=8");
> +
> +        if (common_machine_type) {
> +            machine = g_strdup_printf("%s,", common_machine_type);
> +        }
> +
> +        arch_opts = g_strdup_printf("-nodefaults -machine %svsmt=8", machine);
>       } else if (strcmp(arch, "aarch64") == 0) {
>           init_bootfile(bootpath, aarch64_kernel, sizeof(aarch64_kernel));
>           memory_size = "150M";
> -        arch_opts = g_strdup_printf("-machine virt,gic-version=max -cpu max "
> -                                    "-kernel %s", bootpath);
> +
> +        if (common_machine_type) {
> +            machine = g_strdup_printf("%s", common_machine_type);
> +        } else {
> +            machine = g_strdup("virt");
> +        }
> +
> +        arch_opts = g_strdup_printf("-machine %s,gic-version=max -cpu max "
> +                                    "-kernel %s", machine, bootpath);
>           start_address = ARM_TEST_MEM_START;
>           end_address = ARM_TEST_MEM_END;
>   
> @@ -799,7 +825,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 = mig_init_src(cmd_source);
>           qtest_qmp_set_event_callback(*from,
>                                        migrate_watch_for_stop,
>                                        &got_src_stop);
> @@ -819,7 +845,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>                                    shmem_opts,
>                                    args->opts_target ? args->opts_target : "",
>                                    ignore_stderr);
> -    *to = qtest_init(cmd_target);
> +    *to = mig_init_dst(cmd_target);
>       qtest_qmp_set_event_callback(*to,
>                                    migrate_watch_for_resume,
>                                    &got_dst_resume);
> @@ -2769,6 +2795,8 @@ int main(int argc, char **argv)
>       const char *arch;
>       g_autoptr(GError) err = NULL;
>       int ret;
> +    const char *qemu_src = getenv("QTEST_QEMU_SRC");
> +    const char *qemu_dst = getenv("QTEST_QEMU_DST");
>   
>       g_test_init(&argc, &argv, NULL);
>   
> @@ -2780,6 +2808,16 @@ int main(int argc, char **argv)
>           return 0;
>       }
>   
> +    if (qemu_src || qemu_dst) {
> +        if (qemu_src && qemu_dst) {
> +            g_test_message("Only one of QTEST_QEMU_SRC, QTEST_QEMU_DST is allowed.");
> +            exit(1);
> +        }
> +        common_machine_type = find_common_machine_type(qemu_src ? qemu_src : qemu_dst);
> +        g_test_message("Using two different QEMU binaries. Common machine type: %s",
> +                       common_machine_type);
> +    }
> +
>       has_uffd = ufd_version_check();
>       arch = qtest_get_arch();
>   

Re: [RFC PATCH 1/1] qtest/migration: Support more than one QEMU binary

[Daniel P. Berrangé]

On Tue, Oct 03, 2023 at 05:24:50PM +0200, Philippe Mathieu-Daudé wrote:
> Hi Fabiano,
> 
> [+Alex & Daniel]
> 
> On 3/10/23 16:19, Fabiano Rosas wrote:
> > We have strict rules around migration compatibility between different
> > QEMU versions but not a single test that validates the migration state
> > between different binaries.
> > 
> > Add some 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_SRC
> > and QTEST_QEMU_DST. Only one of the two is expected to be used along
> > with the existing QTEST_QEMU_BINARY, which will automatically be used
> > for the other side of migration.
> > 
> > Usage:
> > QTEST_QEMU_DST=./build-8.2.0/qemu-system-x86_64 \
> > QTEST_QEMU_BINARY=../build-8.1.0/qemu-system-x86_64 \
> > ./tests/qtest/migration-test
> 
> I like it as a first step, but I'd rather run $QTEST_QEMU_SRC
> directly from a docker image, i.e.:
> 
> $ docker run -it opensuse/leap
> # zypper update -y && zypper install -y qemu-x86
> $ docker run opensuse/leap qemu-system-x86_64 ...

In theory this does not need any support in libqtest at all

$ cat myqemu.dkr 
FROM fedora:38

RUN dnf -y install qemu-kvm

$ podman build -f myqemu.dkr --tag myqemu .

$ cat > myqemu <<EOF
#!/bin/sh
exec podman run --volume /tmp=/tmp --security-opt label=disable myqemu qemu-system-x86_64 "$@"

$ chmod +x myqemu

$ QTEST_QEMU_BINARY=./myqemu.sh  ./build/tests/qtest/rtc-test


Except we fail on the last step, because bind mounts don't make UNIX domain
sockets accessible. So we can see the /tmp/qtest-$PID.sock in the container,
but it can't be used.

UNIX domain sockets in the filesystem are tied to the mount namespace, and
podman/docker inherantly creates a new mount namespace making the UNIX
domani socket inaccessible.

UNIX domain sockets in the abstract namespace, however, are tied to the
network namespace, so if you used podman --network host, they should be
accessible.

libqtest could be changed to use abstract UNIX domain sockets on Linux
only, and likely unlock the use of podman for QEMU.

> 
> > This code also works for when debugging with GDB to pass the same
> > binary, but different GDB options for src and dst.
> > 
> > Signed-off-by: Fabiano Rosas <farosas@suse.de>
> > ---
> >   tests/qtest/migration-helpers.c | 168 ++++++++++++++++++++++++++++++++
> >   tests/qtest/migration-helpers.h |   3 +
> >   tests/qtest/migration-test.c    |  52 ++++++++--
> >   3 files changed, 216 insertions(+), 7 deletions(-)
> > 
> > diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
> > index be00c52d00..e84360c3b3 100644
> > --- a/tests/qtest/migration-helpers.c
> > +++ b/tests/qtest/migration-helpers.c
> > @@ -12,6 +12,8 @@
> >   #include "qemu/osdep.h"
> >   #include "qapi/qmp/qjson.h"
> > +#include "qapi/qmp/qlist.h"
> > +#include "qapi/qmp/qstring.h"
> >   #include "migration-helpers.h"
> > @@ -180,3 +182,169 @@ void wait_for_migration_fail(QTestState *from, bool allow_active)
> >       g_assert(qdict_get_bool(rsp_return, "running"));
> >       qobject_unref(rsp_return);
> >   }
> > +
> > +static char *query_pkg_version(QTestState *who)
> > +{
> > +    QDict *rsp;
> > +    char *pkg;
> > +
> > +    rsp = qtest_qmp_assert_success_ref(who, "{ 'execute': 'query-version' }");
> > +    g_assert(rsp);
> > +
> > +    pkg = g_strdup(qdict_get_str(rsp, "package"));
> > +    qobject_unref(rsp);
> > +
> > +    return pkg;
> > +}
> > +
> > +static QList *query_machines(void)
> > +{
> > +    QDict *response;
> > +    QList *list;
> > +    QTestState *qts;
> > +
> > +    qts = qtest_init("-machine none");
> > +    response = qtest_qmp(qts, "{ 'execute': 'query-machines' }");
> > +    g_assert(response);
> > +    list = qdict_get_qlist(response, "return");
> > +    g_assert(list);
> > +
> > +    qtest_quit(qts);
> > +    return list;
> > +}
> > +
> > +static char *get_default_machine(QList *list)
> > +{
> > +    QDict *info;
> > +    QListEntry *entry;
> > +    QString *qstr;
> > +    char *name = NULL;
> > +
> > +    QLIST_FOREACH_ENTRY(list, entry) {
> > +        info = qobject_to(QDict, qlist_entry_obj(entry));
> > +        g_assert(info);
> > +
> > +        if (qdict_get(info, "is-default")) {
> > +            qstr = qobject_to(QString, qdict_get(info, "name"));
> > +            g_assert(qstr);
> > +            name = g_strdup(qstring_get_str(qstr));
> > +            break;
> > +        }
> > +    }
> > +
> > +    g_assert(name);
> > +    return name;
> > +}
> > +
> > +static bool search_default_machine(QList *list, const char *theirs)
> > +{
> > +    QDict *info;
> > +    QListEntry *entry;
> > +    QString *qstr;
> > +
> > +    if (!theirs) {
> > +        return false;
> > +    }
> > +
> > +    QLIST_FOREACH_ENTRY(list, entry) {
> > +        info = qobject_to(QDict, qlist_entry_obj(entry));
> > +        g_assert(info);
> > +
> > +        qstr = qobject_to(QString, qdict_get(info, "name"));
> > +        g_assert(qstr);
> > +
> > +        if (g_str_equal(qstring_get_str(qstr), theirs)) {
> > +            return true;
> > +        }
> > +    }
> > +    return false;
> > +}
> > +
> > +/*
> > + * We need to ensure that both QEMU instances set via the QTEST_QEMU_*
> > + * vars will use the same machine type. Use a custom query_machines
> > + * function because the generic one in libqtest has a cache that would
> > + * return the same machines for both binaries.
> > + */
> > +char *find_common_machine_type(const char *bin)
> > +{
> > +    QList *m1, *m2;
> > +    g_autofree char *def1 = NULL;
> > +    g_autofree char *def2 = NULL;
> > +    const char *qemu_bin = getenv("QTEST_QEMU_BINARY");
> > +
> > +    m1 = query_machines();
> > +
> > +    g_setenv("QTEST_QEMU_BINARY", bin, true);
> > +    m2 = query_machines();
> > +    g_setenv("QTEST_QEMU_BINARY", qemu_bin, true);
> > +
> > +    def1 = get_default_machine(m1);
> > +    def2 = get_default_machine(m2);
> > +
> > +    if (g_str_equal(def1, def2)) {
> > +        /* either can be used */
> > +        return g_strdup(def1);
> > +    }
> > +
> > +    if (search_default_machine(m1, def2)) {
> > +        return g_strdup(def2);
> > +    }
> > +
> > +    if (search_default_machine(m2, def1)) {
> > +        return g_strdup(def1);
> > +    }
> > +
> > +    g_assert_not_reached();
> > +}
> > +
> > +/*
> > + * Init a guest for migration tests using an alternate QEMU binary for
> > + * either the source or destination, depending on @var. The other
> > + * binary should be set as usual via QTEST_QEMU_BINARY.
> > + *
> > + * Expected values:
> > + *   QTEST_QEMU_SRC
> > + *   QTEST_QEMU_DST
> > + *
> > + * Warning: The generic parts of qtest could be using
> > + * QTEST_QEMU_BINARY to query for properties before we reach the
> > + * migration code. If the alternate binary is too dissimilar that
> > + * could cause issues.
> > + */
> > +static QTestState *init_vm(const char *extra_args, const char *var)
> > +{
> > +    const char *alt_bin = getenv(var);
> > +    const char *qemu_bin = getenv("QTEST_QEMU_BINARY");
> > +    g_autofree char *pkg = NULL;
> > +    bool src = !!strstr(var, "SRC");
> > +    QTestState *qts;
> > +
> > +    if (alt_bin) {
> > +        g_setenv("QTEST_QEMU_BINARY", alt_bin, true);
> > +    }
> > +
> > +    qts = qtest_init(extra_args);
> > +    pkg = query_pkg_version(qts);
> > +
> > +    g_test_message("Using %s (%s) as migration %s",
> > +                   alt_bin ? alt_bin : qemu_bin,
> > +                   pkg,
> > +                   src ? "source" : "destination");
> > +
> > +    if (alt_bin) {
> > +        /* restore the original */
> > +        g_setenv("QTEST_QEMU_BINARY", qemu_bin, true);
> > +    }
> > +    return qts;
> > +}

IMHO, we should just create a new qtest_init_env variant, that is the
same as qtest_init, but accepts an environment variable name to use as
an override.

eg

   qtest_init_env("QTEST_QEMU_BINARY_SRC", extra_args)

it would look for $QTEST_QEMU_BINARY_SRC and if not found automatically
fallback to $QTEST_QEMU_BINARY.

I don't think there's any need to explicitly forbid setting both
QTEST_QEMU_BINARY_SRC and QTEST_QEMU_BINARY_DST at the same time.


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: [RFC PATCH 1/1] qtest/migration: Support more than one QEMU binary

[Fabiano Rosas]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Oct 03, 2023 at 05:24:50PM +0200, Philippe Mathieu-Daudé wrote:
>> Hi Fabiano,
>> 
>> [+Alex & Daniel]
>> 
>> On 3/10/23 16:19, Fabiano Rosas wrote:
>> > We have strict rules around migration compatibility between different
>> > QEMU versions but not a single test that validates the migration state
>> > between different binaries.
>> > 
>> > Add some 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_SRC
>> > and QTEST_QEMU_DST. Only one of the two is expected to be used along
>> > with the existing QTEST_QEMU_BINARY, which will automatically be used
>> > for the other side of migration.
>> > 
>> > Usage:
>> > QTEST_QEMU_DST=./build-8.2.0/qemu-system-x86_64 \
>> > QTEST_QEMU_BINARY=../build-8.1.0/qemu-system-x86_64 \
>> > ./tests/qtest/migration-test
>> 
>> I like it as a first step, but I'd rather run $QTEST_QEMU_SRC
>> directly from a docker image, i.e.:
>> 
>> $ docker run -it opensuse/leap
>> # zypper update -y && zypper install -y qemu-x86
>> $ docker run opensuse/leap qemu-system-x86_64 ...
>
> In theory this does not need any support in libqtest at all
>
> $ cat myqemu.dkr 
> FROM fedora:38
>
> RUN dnf -y install qemu-kvm
>
> $ podman build -f myqemu.dkr --tag myqemu .
>
> $ cat > myqemu <<EOF
> #!/bin/sh
> exec podman run --volume /tmp=/tmp --security-opt label=disable myqemu qemu-system-x86_64 "$@"
>
> $ chmod +x myqemu
>
> $ QTEST_QEMU_BINARY=./myqemu.sh  ./build/tests/qtest/rtc-test

I'm favor of this. I usually set that variable to something like 'gdb
--args ...' and it works just fine.

> Except we fail on the last step, because bind mounts don't make UNIX domain
> sockets accessible. So we can see the /tmp/qtest-$PID.sock in the container,
> but it can't be used.
>
> UNIX domain sockets in the filesystem are tied to the mount namespace, and
> podman/docker inherantly creates a new mount namespace making the UNIX
> domani socket inaccessible.
>
> UNIX domain sockets in the abstract namespace, however, are tied to the
> network namespace, so if you used podman --network host, they should be
> accessible.
>
> libqtest could be changed to use abstract UNIX domain sockets on Linux
> only, and likely unlock the use of podman for QEMU.
>
>> 
>> > This code also works for when debugging with GDB to pass the same
>> > binary, but different GDB options for src and dst.
>> > 
>> > Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> > ---
>> >   tests/qtest/migration-helpers.c | 168 ++++++++++++++++++++++++++++++++
>> >   tests/qtest/migration-helpers.h |   3 +
>> >   tests/qtest/migration-test.c    |  52 ++++++++--
>> >   3 files changed, 216 insertions(+), 7 deletions(-)
>> > 
>> > diff --git a/tests/qtest/migration-helpers.c b/tests/qtest/migration-helpers.c
>> > index be00c52d00..e84360c3b3 100644
>> > --- a/tests/qtest/migration-helpers.c
>> > +++ b/tests/qtest/migration-helpers.c
>> > @@ -12,6 +12,8 @@
>> >   #include "qemu/osdep.h"
>> >   #include "qapi/qmp/qjson.h"
>> > +#include "qapi/qmp/qlist.h"
>> > +#include "qapi/qmp/qstring.h"
>> >   #include "migration-helpers.h"
>> > @@ -180,3 +182,169 @@ void wait_for_migration_fail(QTestState *from, bool allow_active)
>> >       g_assert(qdict_get_bool(rsp_return, "running"));
>> >       qobject_unref(rsp_return);
>> >   }
>> > +
>> > +static char *query_pkg_version(QTestState *who)
>> > +{
>> > +    QDict *rsp;
>> > +    char *pkg;
>> > +
>> > +    rsp = qtest_qmp_assert_success_ref(who, "{ 'execute': 'query-version' }");
>> > +    g_assert(rsp);
>> > +
>> > +    pkg = g_strdup(qdict_get_str(rsp, "package"));
>> > +    qobject_unref(rsp);
>> > +
>> > +    return pkg;
>> > +}
>> > +
>> > +static QList *query_machines(void)
>> > +{
>> > +    QDict *response;
>> > +    QList *list;
>> > +    QTestState *qts;
>> > +
>> > +    qts = qtest_init("-machine none");
>> > +    response = qtest_qmp(qts, "{ 'execute': 'query-machines' }");
>> > +    g_assert(response);
>> > +    list = qdict_get_qlist(response, "return");
>> > +    g_assert(list);
>> > +
>> > +    qtest_quit(qts);
>> > +    return list;
>> > +}
>> > +
>> > +static char *get_default_machine(QList *list)
>> > +{
>> > +    QDict *info;
>> > +    QListEntry *entry;
>> > +    QString *qstr;
>> > +    char *name = NULL;
>> > +
>> > +    QLIST_FOREACH_ENTRY(list, entry) {
>> > +        info = qobject_to(QDict, qlist_entry_obj(entry));
>> > +        g_assert(info);
>> > +
>> > +        if (qdict_get(info, "is-default")) {
>> > +            qstr = qobject_to(QString, qdict_get(info, "name"));
>> > +            g_assert(qstr);
>> > +            name = g_strdup(qstring_get_str(qstr));
>> > +            break;
>> > +        }
>> > +    }
>> > +
>> > +    g_assert(name);
>> > +    return name;
>> > +}
>> > +
>> > +static bool search_default_machine(QList *list, const char *theirs)
>> > +{
>> > +    QDict *info;
>> > +    QListEntry *entry;
>> > +    QString *qstr;
>> > +
>> > +    if (!theirs) {
>> > +        return false;
>> > +    }
>> > +
>> > +    QLIST_FOREACH_ENTRY(list, entry) {
>> > +        info = qobject_to(QDict, qlist_entry_obj(entry));
>> > +        g_assert(info);
>> > +
>> > +        qstr = qobject_to(QString, qdict_get(info, "name"));
>> > +        g_assert(qstr);
>> > +
>> > +        if (g_str_equal(qstring_get_str(qstr), theirs)) {
>> > +            return true;
>> > +        }
>> > +    }
>> > +    return false;
>> > +}
>> > +
>> > +/*
>> > + * We need to ensure that both QEMU instances set via the QTEST_QEMU_*
>> > + * vars will use the same machine type. Use a custom query_machines
>> > + * function because the generic one in libqtest has a cache that would
>> > + * return the same machines for both binaries.
>> > + */
>> > +char *find_common_machine_type(const char *bin)
>> > +{
>> > +    QList *m1, *m2;
>> > +    g_autofree char *def1 = NULL;
>> > +    g_autofree char *def2 = NULL;
>> > +    const char *qemu_bin = getenv("QTEST_QEMU_BINARY");
>> > +
>> > +    m1 = query_machines();
>> > +
>> > +    g_setenv("QTEST_QEMU_BINARY", bin, true);
>> > +    m2 = query_machines();
>> > +    g_setenv("QTEST_QEMU_BINARY", qemu_bin, true);
>> > +
>> > +    def1 = get_default_machine(m1);
>> > +    def2 = get_default_machine(m2);
>> > +
>> > +    if (g_str_equal(def1, def2)) {
>> > +        /* either can be used */
>> > +        return g_strdup(def1);
>> > +    }
>> > +
>> > +    if (search_default_machine(m1, def2)) {
>> > +        return g_strdup(def2);
>> > +    }
>> > +
>> > +    if (search_default_machine(m2, def1)) {
>> > +        return g_strdup(def1);
>> > +    }
>> > +
>> > +    g_assert_not_reached();
>> > +}
>> > +
>> > +/*
>> > + * Init a guest for migration tests using an alternate QEMU binary for
>> > + * either the source or destination, depending on @var. The other
>> > + * binary should be set as usual via QTEST_QEMU_BINARY.
>> > + *
>> > + * Expected values:
>> > + *   QTEST_QEMU_SRC
>> > + *   QTEST_QEMU_DST
>> > + *
>> > + * Warning: The generic parts of qtest could be using
>> > + * QTEST_QEMU_BINARY to query for properties before we reach the
>> > + * migration code. If the alternate binary is too dissimilar that
>> > + * could cause issues.
>> > + */
>> > +static QTestState *init_vm(const char *extra_args, const char *var)
>> > +{
>> > +    const char *alt_bin = getenv(var);
>> > +    const char *qemu_bin = getenv("QTEST_QEMU_BINARY");
>> > +    g_autofree char *pkg = NULL;
>> > +    bool src = !!strstr(var, "SRC");
>> > +    QTestState *qts;
>> > +
>> > +    if (alt_bin) {
>> > +        g_setenv("QTEST_QEMU_BINARY", alt_bin, true);
>> > +    }
>> > +
>> > +    qts = qtest_init(extra_args);
>> > +    pkg = query_pkg_version(qts);
>> > +
>> > +    g_test_message("Using %s (%s) as migration %s",
>> > +                   alt_bin ? alt_bin : qemu_bin,
>> > +                   pkg,
>> > +                   src ? "source" : "destination");
>> > +
>> > +    if (alt_bin) {
>> > +        /* restore the original */
>> > +        g_setenv("QTEST_QEMU_BINARY", qemu_bin, true);
>> > +    }
>> > +    return qts;
>> > +}
>
> IMHO, we should just create a new qtest_init_env variant, that is the
> same as qtest_init, but accepts an environment variable name to use as
> an override.
>
> eg
>
>    qtest_init_env("QTEST_QEMU_BINARY_SRC", extra_args)
>
> it would look for $QTEST_QEMU_BINARY_SRC and if not found automatically
> fallback to $QTEST_QEMU_BINARY.
>

This was initially intended to be an off-tree patch that I could use to
test migration compatibility, so I avoided touching libqtest. Now that I
learned this might be of interest we can make it less hackish.

> I don't think there's any need to explicitly forbid setting both
> QTEST_QEMU_BINARY_SRC and QTEST_QEMU_BINARY_DST at the same time.
>

This is a little biased on my usage of migration-test from the command
line. Adding one of SRC|DST is easier when coming/going from a test that
already uses QTEST_QEMU_BINARY. No big deal, of course.

We'll just have to do something about qtest_get_machines(),
qtest_has_device() and qtest_get_arch(), which expect QTEST_QEMU_BINARY
to be present.

Re: [RFC PATCH 1/1] qtest/migration: Support more than one QEMU binary

[Juan Quintela]

Fabiano Rosas <farosas@suse.de> wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
>> On Tue, Oct 03, 2023 at 05:24:50PM +0200, Philippe Mathieu-Daudé wrote:
[...]

>> $ cat myqemu.dkr 
>> FROM fedora:38
>>
>> RUN dnf -y install qemu-kvm
>>
>> $ podman build -f myqemu.dkr --tag myqemu .
>>
>> $ cat > myqemu <<EOF
>> #!/bin/sh
>> exec podman run --volume /tmp=/tmp --security-opt label=disable myqemu qemu-system-x86_64 "$@"
>>
>> $ chmod +x myqemu
>>
>> $ QTEST_QEMU_BINARY=./myqemu.sh  ./build/tests/qtest/rtc-test
>
> I'm favor of this. I usually set that variable to something like 'gdb
> --args ...' and it works just fine.
>
>> Except we fail on the last step, because bind mounts don't make UNIX domain
>> sockets accessible. So we can see the /tmp/qtest-$PID.sock in the container,
>> but it can't be used.
>>
>> UNIX domain sockets in the filesystem are tied to the mount namespace, and
>> podman/docker inherantly creates a new mount namespace making the UNIX
>> domani socket inaccessible.
>>
>> UNIX domain sockets in the abstract namespace, however, are tied to the
>> network namespace, so if you used podman --network host, they should be
>> accessible.
>>
>> libqtest could be changed to use abstract UNIX domain sockets on Linux
>> only, and likely unlock the use of podman for QEMU.

That is one idea, but why can't we convince a container to compile
_both_ qemus?

I am not familiar with containers, but:
- We already know how to compile a qemu inside a container
- We can teach it to compile $HEAD and v8.0.0 (or whatever)

And do the test inside, right?

On the other hand, your approach has the advantage that one can test
opensuse qemu against fedora qemu, and similar.  Not sure how useful is
that, though.

[lots of code to find common machine types]

I think that it is just easier to pass the machine type we want to test
to whatever script we have.  Specially where [sane] architectures like
arm don't have a default machine type (no, I haven't double checked if
that has changed lately).

>> IMHO, we should just create a new qtest_init_env variant, that is the
>> same as qtest_init, but accepts an environment variable name to use as
>> an override.
>>
>> eg
>>
>>    qtest_init_env("QTEST_QEMU_BINARY_SRC", extra_args)

That was going to be my suggestion.

>> it would look for $QTEST_QEMU_BINARY_SRC and if not found automatically
>> fallback to $QTEST_QEMU_BINARY.
>>
>
> This was initially intended to be an off-tree patch that I could use to
> test migration compatibility, so I avoided touching libqtest. Now that I
> learned this might be of interest we can make it less hackish.
>
>> I don't think there's any need to explicitly forbid setting both
>> QTEST_QEMU_BINARY_SRC and QTEST_QEMU_BINARY_DST at the same time.
>>
>
> This is a little biased on my usage of migration-test from the command
> line. Adding one of SRC|DST is easier when coming/going from a test that
> already uses QTEST_QEMU_BINARY. No big deal, of course.
>
> We'll just have to do something about qtest_get_machines(),
> qtest_has_device() and qtest_get_arch(), which expect QTEST_QEMU_BINARY
> to be present.

I think we can do the same trick here:

qtest_has_device_env("VARIABLE") and let qemu_has_device() just call it
with QTEST_QEMU_BINARY.  Same for the others.

It is more, if we can do it easy, we can do that qtest_init_env() checks
in the case that variable is not QTEST_QEMU_BINARY that this one is also
set, and assume in the rest of the code that options are compatible
between whatever we passed an QTEST_QEMU_BINARY, that way we don't need
to change anything else.  Well, just put a big warning in a comment
saying that using qtest_init_env() means that you know what you are
doing.

Later, Juan.

Re: [RFC PATCH 1/1] qtest/migration: Support more than one QEMU binary

[Fabiano Rosas]

Juan Quintela <quintela@redhat.com> writes:

> Fabiano Rosas <farosas@suse.de> wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>>
>>> On Tue, Oct 03, 2023 at 05:24:50PM +0200, Philippe Mathieu-Daudé wrote:
> [...]
>
>>> $ cat myqemu.dkr 
>>> FROM fedora:38
>>>
>>> RUN dnf -y install qemu-kvm
>>>
>>> $ podman build -f myqemu.dkr --tag myqemu .
>>>
>>> $ cat > myqemu <<EOF
>>> #!/bin/sh
>>> exec podman run --volume /tmp=/tmp --security-opt label=disable myqemu qemu-system-x86_64 "$@"
>>>
>>> $ chmod +x myqemu
>>>
>>> $ QTEST_QEMU_BINARY=./myqemu.sh  ./build/tests/qtest/rtc-test
>>
>> I'm favor of this. I usually set that variable to something like 'gdb
>> --args ...' and it works just fine.
>>
>>> Except we fail on the last step, because bind mounts don't make UNIX domain
>>> sockets accessible. So we can see the /tmp/qtest-$PID.sock in the container,
>>> but it can't be used.
>>>
>>> UNIX domain sockets in the filesystem are tied to the mount namespace, and
>>> podman/docker inherantly creates a new mount namespace making the UNIX
>>> domani socket inaccessible.
>>>
>>> UNIX domain sockets in the abstract namespace, however, are tied to the
>>> network namespace, so if you used podman --network host, they should be
>>> accessible.
>>>
>>> libqtest could be changed to use abstract UNIX domain sockets on Linux
>>> only, and likely unlock the use of podman for QEMU.
>
> That is one idea, but why can't we convince a container to compile
> _both_ qemus?
>
> I am not familiar with containers, but:
> - We already know how to compile a qemu inside a container
> - We can teach it to compile $HEAD and v8.0.0 (or whatever)
>
> And do the test inside, right?
>
> On the other hand, your approach has the advantage that one can test
> opensuse qemu against fedora qemu, and similar.  Not sure how useful is
> that, though.
>
> [lots of code to find common machine types]

I'm working on a cleanup of this patch to make it more integrated with
libqtest. If we teach qtest_get_machines() to sometimes refresh the list
of machines then it becomes way less code.

> I think that it is just easier to pass the machine type we want to test
> to whatever script we have.  Specially where [sane] architectures like
> arm don't have a default machine type (no, I haven't double checked if
> that has changed lately).

We still need to enforce the same machine type for both binaries and a
sane range of QEMU versions. I think our docs state that we only support
migration from QEMU n->n+1 and vice versa? If the test will know what
combinations are allowed, it could just go ahead and use those.

>>> IMHO, we should just create a new qtest_init_env variant, that is the
>>> same as qtest_init, but accepts an environment variable name to use as
>>> an override.
>>>
>>> eg
>>>
>>>    qtest_init_env("QTEST_QEMU_BINARY_SRC", extra_args)
>
> That was going to be my suggestion.
>
>>> it would look for $QTEST_QEMU_BINARY_SRC and if not found automatically
>>> fallback to $QTEST_QEMU_BINARY.
>>>
>>
>> This was initially intended to be an off-tree patch that I could use to
>> test migration compatibility, so I avoided touching libqtest. Now that I
>> learned this might be of interest we can make it less hackish.
>>
>>> I don't think there's any need to explicitly forbid setting both
>>> QTEST_QEMU_BINARY_SRC and QTEST_QEMU_BINARY_DST at the same time.
>>>
>>
>> This is a little biased on my usage of migration-test from the command
>> line. Adding one of SRC|DST is easier when coming/going from a test that
>> already uses QTEST_QEMU_BINARY. No big deal, of course.
>>
>> We'll just have to do something about qtest_get_machines(),
>> qtest_has_device() and qtest_get_arch(), which expect QTEST_QEMU_BINARY
>> to be present.
>
> I think we can do the same trick here:
>
> qtest_has_device_env("VARIABLE") and let qemu_has_device() just call it
> with QTEST_QEMU_BINARY.  Same for the others.
>
> It is more, if we can do it easy, we can do that qtest_init_env() checks
> in the case that variable is not QTEST_QEMU_BINARY that this one is also
> set, and assume in the rest of the code that options are compatible
> between whatever we passed an QTEST_QEMU_BINARY, that way we don't need
> to change anything else.  Well, just put a big warning in a comment
> saying that using qtest_init_env() means that you know what you are
> doing.
>
> Later, Juan.

Re: [RFC PATCH 1/1] qtest/migration: Support more than one QEMU binary

[Daniel P. Berrangé]

On Wed, Oct 04, 2023 at 12:59:49PM -0300, Fabiano Rosas wrote:
> Juan Quintela <quintela@redhat.com> writes:
> 
> > Fabiano Rosas <farosas@suse.de> wrote:
> >> Daniel P. Berrangé <berrange@redhat.com> writes:
> >>
> >>> On Tue, Oct 03, 2023 at 05:24:50PM +0200, Philippe Mathieu-Daudé wrote:
> > [...]
> >
> >>> $ cat myqemu.dkr 
> >>> FROM fedora:38
> >>>
> >>> RUN dnf -y install qemu-kvm
> >>>
> >>> $ podman build -f myqemu.dkr --tag myqemu .
> >>>
> >>> $ cat > myqemu <<EOF
> >>> #!/bin/sh
> >>> exec podman run --volume /tmp=/tmp --security-opt label=disable myqemu qemu-system-x86_64 "$@"
> >>>
> >>> $ chmod +x myqemu
> >>>
> >>> $ QTEST_QEMU_BINARY=./myqemu.sh  ./build/tests/qtest/rtc-test
> >>
> >> I'm favor of this. I usually set that variable to something like 'gdb
> >> --args ...' and it works just fine.
> >>
> >>> Except we fail on the last step, because bind mounts don't make UNIX domain
> >>> sockets accessible. So we can see the /tmp/qtest-$PID.sock in the container,
> >>> but it can't be used.
> >>>
> >>> UNIX domain sockets in the filesystem are tied to the mount namespace, and
> >>> podman/docker inherantly creates a new mount namespace making the UNIX
> >>> domani socket inaccessible.
> >>>
> >>> UNIX domain sockets in the abstract namespace, however, are tied to the
> >>> network namespace, so if you used podman --network host, they should be
> >>> accessible.
> >>>
> >>> libqtest could be changed to use abstract UNIX domain sockets on Linux
> >>> only, and likely unlock the use of podman for QEMU.
> >
> > That is one idea, but why can't we convince a container to compile
> > _both_ qemus?
> >
> > I am not familiar with containers, but:
> > - We already know how to compile a qemu inside a container
> > - We can teach it to compile $HEAD and v8.0.0 (or whatever)
> >
> > And do the test inside, right?
> >
> > On the other hand, your approach has the advantage that one can test
> > opensuse qemu against fedora qemu, and similar.  Not sure how useful is
> > that, though.
> >
> > [lots of code to find common machine types]
> 
> I'm working on a cleanup of this patch to make it more integrated with
> libqtest. If we teach qtest_get_machines() to sometimes refresh the list
> of machines then it becomes way less code.
> 
> > I think that it is just easier to pass the machine type we want to test
> > to whatever script we have.  Specially where [sane] architectures like
> > arm don't have a default machine type (no, I haven't double checked if
> > that has changed lately).
> 
> We still need to enforce the same machine type for both binaries and a
> sane range of QEMU versions. I think our docs state that we only support
> migration from QEMU n->n+1 and vice versa? If the test will know what
> combinations are allowed, it could just go ahead and use those.

Query the 'pc' (or 'q35' as appropriate) alias on both QEMU versions,
to resolve them into versioned machines.

Then find which resolved machine version(s) exist in both QEMUs, and
prefer the src machine if multiple matches exist.

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: [RFC PATCH 1/1] qtest/migration: Support more than one QEMU binary

[Juan Quintela]

Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Wed, Oct 04, 2023 at 12:59:49PM -0300, Fabiano Rosas wrote:
>> Juan Quintela <quintela@redhat.com> writes:
>> 
>> > Fabiano Rosas <farosas@suse.de> wrote:
>> >> Daniel P. Berrangé <berrange@redhat.com> writes:
>> >>
>> >>> On Tue, Oct 03, 2023 at 05:24:50PM +0200, Philippe Mathieu-Daudé wrote:
>> > [...]

>> I'm working on a cleanup of this patch to make it more integrated with
>> libqtest. If we teach qtest_get_machines() to sometimes refresh the list
>> of machines then it becomes way less code.
>> 
>> > I think that it is just easier to pass the machine type we want to test
>> > to whatever script we have.  Specially where [sane] architectures like
>> > arm don't have a default machine type (no, I haven't double checked if
>> > that has changed lately).
>> 
>> We still need to enforce the same machine type for both binaries and a
>> sane range of QEMU versions. I think our docs state that we only support
>> migration from QEMU n->n+1 and vice versa? If the test will know what
>> combinations are allowed, it could just go ahead and use those.
>
> Query the 'pc' (or 'q35' as appropriate) alias on both QEMU versions,
> to resolve them into versioned machines.
>
> Then find which resolved machine version(s) exist in both QEMUs, and
> prefer the src machine if multiple matches exist.

We only change Machine Type with each qemu version, so having to change
it by hand don't look so complicated.

Let's assume for a moment that "pc" and "q35" machine types don't exist
(rest of architectures needs to do a similar thing)

latest qemu has:
pc-i440fx-8.2        Standard PC (i440FX + PIIX, 1996) (default)
pc-i440fx-8.1        Standard PC (i440FX + PIIX, 1996) (default)
pc-i440fx-8.0        Standard PC (i440FX + PIIX, 1996) (default)
pc-i440fx-7.2        Standard PC (i440FX + PIIX, 1996) (default)

Previous version one has everything except 8.2

We want to test:

(this is what we do now)
qemu-8.2 -M pc-i440fx-8.2  -> qemu-8.2 -M pc-i440fx-8.2

And we want to test additionally:

qemu-8.1 -M pc-i440fx-8.1  -> qemu-8.2 -M pc-i440fx-8.1
qemu-8.2 -M pc-i440fx-8.1  -> qemu-8.1 -M pc-i440fx-8.1

And that is it.

So the thing that we need is a sane way to get qtest_init() to use the
right machine type without inventing what machine type they want.  Not
having a default machine type has other advantages, but that is a
different discussion.

Later, Juan.

Re: [RFC PATCH 1/1] qtest/migration: Support more than one QEMU binary

[Thomas Huth]

On 04/10/2023 20.09, Juan Quintela wrote:
> Daniel P. Berrangé <berrange@redhat.com> wrote:
>> On Wed, Oct 04, 2023 at 12:59:49PM -0300, Fabiano Rosas wrote:
>>> Juan Quintela <quintela@redhat.com> writes:
>>>
>>>> Fabiano Rosas <farosas@suse.de> wrote:
>>>>> Daniel P. Berrangé <berrange@redhat.com> writes:
>>>>>
>>>>>> On Tue, Oct 03, 2023 at 05:24:50PM +0200, Philippe Mathieu-Daudé wrote:
>>>> [...]
> 
>>> I'm working on a cleanup of this patch to make it more integrated with
>>> libqtest. If we teach qtest_get_machines() to sometimes refresh the list
>>> of machines then it becomes way less code.
>>>
>>>> I think that it is just easier to pass the machine type we want to test
>>>> to whatever script we have.  Specially where [sane] architectures like
>>>> arm don't have a default machine type (no, I haven't double checked if
>>>> that has changed lately).
>>>
>>> We still need to enforce the same machine type for both binaries and a
>>> sane range of QEMU versions. I think our docs state that we only support
>>> migration from QEMU n->n+1 and vice versa? If the test will know what
>>> combinations are allowed, it could just go ahead and use those.
>>
>> Query the 'pc' (or 'q35' as appropriate) alias on both QEMU versions,
>> to resolve them into versioned machines.
>>
>> Then find which resolved machine version(s) exist in both QEMUs, and
>> prefer the src machine if multiple matches exist.
> 
> We only change Machine Type with each qemu version, so having to change
> it by hand don't look so complicated.
> 
> Let's assume for a moment that "pc" and "q35" machine types don't exist
> (rest of architectures needs to do a similar thing)
> 
> latest qemu has:
> pc-i440fx-8.2        Standard PC (i440FX + PIIX, 1996) (default)
> pc-i440fx-8.1        Standard PC (i440FX + PIIX, 1996) (default)
> pc-i440fx-8.0        Standard PC (i440FX + PIIX, 1996) (default)
> pc-i440fx-7.2        Standard PC (i440FX + PIIX, 1996) (default)
> 
> Previous version one has everything except 8.2
> 
> We want to test:
> 
> (this is what we do now)
> qemu-8.2 -M pc-i440fx-8.2  -> qemu-8.2 -M pc-i440fx-8.2
> 
> And we want to test additionally:
> 
> qemu-8.1 -M pc-i440fx-8.1  -> qemu-8.2 -M pc-i440fx-8.1
> qemu-8.2 -M pc-i440fx-8.1  -> qemu-8.1 -M pc-i440fx-8.1
> 
> And that is it.
> 
> So the thing that we need is a sane way to get qtest_init() to use the
> right machine type without inventing what machine type they want.  Not
> having a default machine type has other advantages, but that is a
> different discussion.

Not sure whether it's useful for you, but have a look at 
qtest_cb_for_every_machine() and qtest_is_old_versioned_machine() ... there 
is already some logic in there to find out the latest machine version, maybe 
you can re-use some of the code in there.

  Thomas