[PATCH 0/6] tests/qtest/readconfig: Test configs in docs/config/

[PATCH 0/6] tests/qtest/readconfig: Test configs in docs/config/

[Thomas Huth]

We've got a bunch of config files in our docs/config/ directory
which only get occasional manual test coverage. And we've got a
"readconfig" qtest which does not check real config files yet.
Let's bring those two areas together and check the real config
files in the readconfig qtest!

I started with ich9-ehci-uhci.cfg which is still quite easy to
test, and then went on with q35-emulated.cfg that needs a little
bit of tweaking (since it hard-codes disk image names) ... so I'd
like to get information on this approach first before I continue
with the other remaining config files in the docs/config/ directory.

Thomas Huth (6):
  tests/qtest/readconfig: Rework test_object_rng_resp into a generic
    function
  tests/qtest/readconfig: Test docs/config/ich9-ehci-uhci.cfg
  docs/config: Set the "kvm" accelerator via "[accel]" section
  tests/qtest/readconfig-test: Allow testing for arbitrary memory sizes
  tests/qtest: Move mkimg() and have_qemu_img() from libqos to libqtest
  tests/qtest/readconfig: Test docs/config/q35-emulated.cfg

 docs/config/mach-virt-graphical.cfg  |   4 +-
 docs/config/mach-virt-serial.cfg     |   4 +-
 docs/config/q35-emulated.cfg         |   2 +
 docs/config/q35-virtio-graphical.cfg |   2 +
 docs/config/q35-virtio-serial.cfg    |   2 +
 configure                            |   1 +
 tests/qtest/libqos/libqos.h          |   2 -
 tests/qtest/libqtest.h               |  21 ++++
 tests/qtest/libqos/libqos.c          |  49 +-------
 tests/qtest/libqtest.c               |  52 ++++++++
 tests/qtest/readconfig-test.c        | 175 +++++++++++++++++++++++++--
 11 files changed, 251 insertions(+), 63 deletions(-)

-- 
2.31.1

[PATCH 1/6] tests/qtest/readconfig: Rework test_object_rng_resp into a generic function

[Thomas Huth]

test_object_rng_resp() can be reworked quite easily to allow
testing for arbitrary objects in the qom-list response.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/readconfig-test.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c
index 9ef870643d..26d79b5e4b 100644
--- a/tests/qtest/readconfig-test.c
+++ b/tests/qtest/readconfig-test.c
@@ -124,13 +124,15 @@ static void test_spice(void)
 }
 #endif
 
-static void test_object_rng_resp(QObject *res)
+static void test_object_available(QObject *res, const char *name,
+                                  const char *type)
 {
     Visitor *v;
     g_autoptr(ObjectPropertyInfoList) objs = NULL;
     ObjectPropertyInfoList *tmp;
     ObjectPropertyInfo *obj;
-    bool seen_rng = false;
+    bool object_available = false;
+    g_autofree char *childtype = g_strdup_printf("child<%s>", type);
 
     g_assert(res);
     v = qobject_input_visitor_new(res);
@@ -142,16 +144,15 @@ static void test_object_rng_resp(QObject *res)
         g_assert(tmp->value);
 
         obj = tmp->value;
-        if (g_str_equal(obj->name, "rng0") &&
-            g_str_equal(obj->type, "child<rng-builtin>")) {
-            seen_rng = true;
+        if (g_str_equal(obj->name, name) && g_str_equal(obj->type, childtype)) {
+            object_available = true;
             break;
         }
 
         tmp = tmp->next;
     }
 
-    g_assert(seen_rng);
+    g_assert(object_available);
 
     visit_free(v);
 }
@@ -170,7 +171,7 @@ static void test_object_rng(void)
     resp = qtest_qmp(qts,
                      "{ 'execute': 'qom-list',"
                      "  'arguments': {'path': '/objects' }}");
-    test_object_rng_resp(qdict_get(resp, "return"));
+    test_object_available(qdict_get(resp, "return"), "rng0", "rng-builtin");
     qobject_unref(resp);
 
     qtest_quit(qts);
-- 
2.31.1

[PATCH 2/6] tests/qtest/readconfig: Test docs/config/ich9-ehci-uhci.cfg

[Thomas Huth]

We've got some sample config files in docs/config/ but no means
of regression checking them. Thus let's test them in our readconfig
qtest, starting with ich9-ehci-uhci.cfg. Note: To enable the test
to read the config files from the build folder, we have to install
a symlink for docs/config in the build directory.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 configure                     |  1 +
 tests/qtest/readconfig-test.c | 21 +++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/configure b/configure
index dccb5d4f96..4331fead78 100755
--- a/configure
+++ b/configure
@@ -2215,6 +2215,7 @@ fi
 # tests might fail. Prefer to keep the relevant files in their own
 # directory and symlink the directory instead.
 LINKS="Makefile"
+LINKS="$LINKS docs/config"
 LINKS="$LINKS pc-bios/optionrom/Makefile"
 LINKS="$LINKS pc-bios/s390-ccw/Makefile"
 LINKS="$LINKS pc-bios/vof/Makefile"
diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c
index 26d79b5e4b..2160603880 100644
--- a/tests/qtest/readconfig-test.c
+++ b/tests/qtest/readconfig-test.c
@@ -177,6 +177,26 @@ static void test_object_rng(void)
     qtest_quit(qts);
 }
 
+static void test_docs_config_ich9(void)
+{
+    QTestState *qts;
+    QDict *resp;
+    QObject *qobj;
+
+    qts = qtest_initf("-nodefaults -readconfig docs/config/ich9-ehci-uhci.cfg");
+
+    resp = qtest_qmp(qts, "{ 'execute': 'qom-list',"
+                          "  'arguments': {'path': '/machine/peripheral' }}");
+    qobj = qdict_get(resp, "return");
+    test_object_available(qobj, "ehci", "ich9-usb-ehci1");
+    test_object_available(qobj, "uhci-1", "ich9-usb-uhci1");
+    test_object_available(qobj, "uhci-2", "ich9-usb-uhci2");
+    test_object_available(qobj, "uhci-3", "ich9-usb-uhci3");
+    qobject_unref(resp);
+
+    qtest_quit(qts);
+}
+
 int main(int argc, char *argv[])
 {
     const char *arch;
@@ -187,6 +207,7 @@ int main(int argc, char *argv[])
     if (g_str_equal(arch, "i386") ||
         g_str_equal(arch, "x86_64")) {
         qtest_add_func("readconfig/x86/memdev", test_x86_memdev);
+        qtest_add_func("readconfig/x86/ich9-ehci-uhci", test_docs_config_ich9);
     }
 #ifdef CONFIG_SPICE
     qtest_add_func("readconfig/spice", test_spice);
-- 
2.31.1

[PATCH 3/6] docs/config: Set the "kvm" accelerator via "[accel]" section

[Thomas Huth]

Configuring the accelerator should nowadays be done via the "-accel"
command line parameter, and thus via the "[accel]" section in config
files. We also need this change for the upcoming qtests that will
use these config files, since the qtests are already using "-accel"
for setting the "qtest" accelerator and QEMU does not like mixing
"-accel ..." and "-machine accel=...".

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 docs/config/mach-virt-graphical.cfg  | 4 +++-
 docs/config/mach-virt-serial.cfg     | 4 +++-
 docs/config/q35-emulated.cfg         | 2 ++
 docs/config/q35-virtio-graphical.cfg | 2 ++
 docs/config/q35-virtio-serial.cfg    | 2 ++
 5 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/docs/config/mach-virt-graphical.cfg b/docs/config/mach-virt-graphical.cfg
index d6d31b17f5..eba76eb198 100644
--- a/docs/config/mach-virt-graphical.cfg
+++ b/docs/config/mach-virt-graphical.cfg
@@ -56,9 +56,11 @@
 
 [machine]
   type = "virt"
-  accel = "kvm"
   gic-version = "host"
 
+[accel]
+  accel = "kvm"
+
 [memory]
   size = "1024"
 
diff --git a/docs/config/mach-virt-serial.cfg b/docs/config/mach-virt-serial.cfg
index 18a7c83731..324b0542ff 100644
--- a/docs/config/mach-virt-serial.cfg
+++ b/docs/config/mach-virt-serial.cfg
@@ -62,9 +62,11 @@
 
 [machine]
   type = "virt"
-  accel = "kvm"
   gic-version = "host"
 
+[accel]
+  accel = "kvm"
+
 [memory]
   size = "1024"
 
diff --git a/docs/config/q35-emulated.cfg b/docs/config/q35-emulated.cfg
index 99ac918e78..c8806e6d36 100644
--- a/docs/config/q35-emulated.cfg
+++ b/docs/config/q35-emulated.cfg
@@ -61,6 +61,8 @@
 
 [machine]
   type = "q35"
+
+[accel]
   accel = "kvm"
 
 [memory]
diff --git a/docs/config/q35-virtio-graphical.cfg b/docs/config/q35-virtio-graphical.cfg
index 4207f11e4f..148b5d2c5e 100644
--- a/docs/config/q35-virtio-graphical.cfg
+++ b/docs/config/q35-virtio-graphical.cfg
@@ -55,6 +55,8 @@
 
 [machine]
   type = "q35"
+
+[accel]
   accel = "kvm"
 
 [memory]
diff --git a/docs/config/q35-virtio-serial.cfg b/docs/config/q35-virtio-serial.cfg
index d2830aec5e..023291390e 100644
--- a/docs/config/q35-virtio-serial.cfg
+++ b/docs/config/q35-virtio-serial.cfg
@@ -60,6 +60,8 @@
 
 [machine]
   type = "q35"
+
+[accel]
   accel = "kvm"
 
 [memory]
-- 
2.31.1

[PATCH 4/6] tests/qtest/readconfig-test: Allow testing for arbitrary memory sizes

[Thomas Huth]

Make test_x86_memdev_resp() more flexible by allowing arbitrary
memory sizes as parameter here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/readconfig-test.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c
index 2160603880..533623f638 100644
--- a/tests/qtest/readconfig-test.c
+++ b/tests/qtest/readconfig-test.c
@@ -48,7 +48,7 @@ static QTestState *qtest_init_with_config(const char *cfgdata)
     return qts;
 }
 
-static void test_x86_memdev_resp(QObject *res)
+static void test_x86_memdev_resp(QObject *res, const char *mem_id, int size)
 {
     Visitor *v;
     g_autoptr(MemdevList) memdevs = NULL;
@@ -63,8 +63,8 @@ static void test_x86_memdev_resp(QObject *res)
     g_assert(!memdevs->next);
 
     memdev = memdevs->value;
-    g_assert_cmpstr(memdev->id, ==, "ram");
-    g_assert_cmpint(memdev->size, ==, 200 * MiB);
+    g_assert_cmpstr(memdev->id, ==, mem_id);
+    g_assert_cmpint(memdev->size, ==, size * MiB);
 
     visit_free(v);
 }
@@ -80,7 +80,7 @@ static void test_x86_memdev(void)
     qts = qtest_init_with_config(cfgdata);
     /* Test valid command */
     resp = qtest_qmp(qts, "{ 'execute': 'query-memdev' }");
-    test_x86_memdev_resp(qdict_get(resp, "return"));
+    test_x86_memdev_resp(qdict_get(resp, "return"), "ram", 200);
     qobject_unref(resp);
 
     qtest_quit(qts);
-- 
2.31.1

[PATCH 5/6] tests/qtest: Move mkimg() and have_qemu_img() from libqos to libqtest

[Thomas Huth]

These two functions can be useful for other qtests beside the
qos-test, too, so move them to libqtest instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/libqos/libqos.h |  2 --
 tests/qtest/libqtest.h      | 21 +++++++++++++++
 tests/qtest/libqos/libqos.c | 49 +---------------------------------
 tests/qtest/libqtest.c      | 52 +++++++++++++++++++++++++++++++++++++
 4 files changed, 74 insertions(+), 50 deletions(-)

diff --git a/tests/qtest/libqos/libqos.h b/tests/qtest/libqos/libqos.h
index 12d05b2365..c04950e2b1 100644
--- a/tests/qtest/libqos/libqos.h
+++ b/tests/qtest/libqos/libqos.h
@@ -27,8 +27,6 @@ QOSState *qtest_boot(QOSOps *ops, const char *cmdline_fmt, ...)
     G_GNUC_PRINTF(2, 3);
 void qtest_common_shutdown(QOSState *qs);
 void qtest_shutdown(QOSState *qs);
-bool have_qemu_img(void);
-void mkimg(const char *file, const char *fmt, unsigned size_mb);
 void mkqcow2(const char *file, unsigned size_mb);
 void migrate(QOSState *from, QOSState *to, const char *uri);
 void prepare_blkdebug_script(const char *debug_fn, const char *event);
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index fcf1c3c3b3..3380cc1f54 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -832,4 +832,25 @@ void qtest_qom_set_bool(QTestState *s, const char *path, const char *property,
  * Returns: Value retrieved from property.
  */
 bool qtest_qom_get_bool(QTestState *s, const char *path, const char *property);
+
+/**
+ * have_qemu_img:
+ *
+ * Returns: true if "qemu-img" is available.
+ */
+bool have_qemu_img(void);
+
+/**
+ * mkimg:
+ * @file: File name of the image that should be created
+ * @fmt: Format, e.g. "qcow2" or "raw"
+ * @size_mb: Size of the image in megabytes
+ *
+ * Create a disk image with qemu-img. Note that the QTEST_QEMU_IMG
+ * environment variable must point to the qemu-img file.
+ *
+ * Returns: true if the image has been created successfully.
+ */
+bool mkimg(const char *file, const char *fmt, unsigned size_mb);
+
 #endif
diff --git a/tests/qtest/libqos/libqos.c b/tests/qtest/libqos/libqos.c
index 5ffda080ec..5c0fa1f7c5 100644
--- a/tests/qtest/libqos/libqos.c
+++ b/tests/qtest/libqos/libqos.c
@@ -137,56 +137,9 @@ void migrate(QOSState *from, QOSState *to, const char *uri)
     migrate_allocator(&from->alloc, &to->alloc);
 }
 
-bool have_qemu_img(void)
-{
-    char *rpath;
-    const char *path = getenv("QTEST_QEMU_IMG");
-    if (!path) {
-        return false;
-    }
-
-    rpath = realpath(path, NULL);
-    if (!rpath) {
-        return false;
-    } else {
-        free(rpath);
-        return true;
-    }
-}
-
-void mkimg(const char *file, const char *fmt, unsigned size_mb)
-{
-    gchar *cli;
-    bool ret;
-    int rc;
-    GError *err = NULL;
-    char *qemu_img_path;
-    gchar *out, *out2;
-    char *qemu_img_abs_path;
-
-    qemu_img_path = getenv("QTEST_QEMU_IMG");
-    g_assert(qemu_img_path);
-    qemu_img_abs_path = realpath(qemu_img_path, NULL);
-    g_assert(qemu_img_abs_path);
-
-    cli = g_strdup_printf("%s create -f %s %s %uM", qemu_img_abs_path,
-                          fmt, file, size_mb);
-    ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
-    if (err || !g_spawn_check_exit_status(rc, &err)) {
-        fprintf(stderr, "%s\n", err->message);
-        g_error_free(err);
-    }
-    g_assert(ret && !err);
-
-    g_free(out);
-    g_free(out2);
-    g_free(cli);
-    free(qemu_img_abs_path);
-}
-
 void mkqcow2(const char *file, unsigned size_mb)
 {
-    return mkimg(file, "qcow2", size_mb);
+    g_assert_true(mkimg(file, "qcow2", size_mb));
 }
 
 void prepare_blkdebug_script(const char *debug_fn, const char *event)
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index 2bfd460531..eaa1e8185f 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1625,3 +1625,55 @@ bool qtest_qom_get_bool(QTestState *s, const char *path, const char *property)
 
     return b;
 }
+
+bool have_qemu_img(void)
+{
+    char *rpath;
+    const char *path = getenv("QTEST_QEMU_IMG");
+    if (!path) {
+        return false;
+    }
+
+    rpath = realpath(path, NULL);
+    if (!rpath) {
+        return false;
+    } else {
+        free(rpath);
+        return true;
+    }
+}
+
+bool mkimg(const char *file, const char *fmt, unsigned size_mb)
+{
+    gchar *cli;
+    bool ret;
+    int rc;
+    GError *err = NULL;
+    char *qemu_img_path;
+    gchar *out, *out2;
+    char *qemu_img_abs_path;
+
+    qemu_img_path = getenv("QTEST_QEMU_IMG");
+    if (!qemu_img_path) {
+        return false;
+    }
+    qemu_img_abs_path = realpath(qemu_img_path, NULL);
+    if (!qemu_img_abs_path) {
+        return false;
+    }
+
+    cli = g_strdup_printf("%s create -f %s %s %uM", qemu_img_abs_path,
+                          fmt, file, size_mb);
+    ret = g_spawn_command_line_sync(cli, &out, &out2, &rc, &err);
+    if (err || !g_spawn_check_exit_status(rc, &err)) {
+        fprintf(stderr, "%s\n", err->message);
+        g_error_free(err);
+    }
+
+    g_free(out);
+    g_free(out2);
+    g_free(cli);
+    free(qemu_img_abs_path);
+
+    return ret && !err;
+}
-- 
2.31.1

[PATCH 6/6] tests/qtest/readconfig: Test docs/config/q35-emulated.cfg

[Thomas Huth]

This config file uses accel = "kvm", so it can only work on Linux.
It also uses two hard-coded image names which we have to replace
with unique temporary files to avoid race conditions. So after
creating the temporary image files, we also have to create a copy
of the config file where we replaced the hard-coded image names.
Once everything is in place, we can start QEMU with this config
file and check that everything is available in QEMU.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/readconfig-test.c | 131 ++++++++++++++++++++++++++++++++++
 1 file changed, 131 insertions(+)

diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c
index 533623f638..fe64376e85 100644
--- a/tests/qtest/readconfig-test.c
+++ b/tests/qtest/readconfig-test.c
@@ -197,6 +197,132 @@ static void test_docs_config_ich9(void)
     qtest_quit(qts);
 }
 
+#ifdef CONFIG_LINUX
+
+static char *make_temp_img(const char *template, const char *format, int size)
+{
+    GError *error = NULL;
+    char *temp_name;
+    int fd;
+
+    /* Create a temporary image names */
+    fd = g_file_open_tmp(template, &temp_name, &error);
+    if (fd == -1) {
+        fprintf(stderr, "unable to create file: %s\n", error->message);
+        g_error_free(error);
+        return NULL;
+    }
+    close(fd);
+
+    if (!mkimg(temp_name, format, size)) {
+        fprintf(stderr, "qemu-img failed to create %s\n", temp_name);
+        g_free(temp_name);
+        return NULL;
+    }
+
+    return temp_name;
+}
+
+static void test_docs_q35_emulated(void)
+{
+    QTestState *qts;
+    QDict *resp;
+    QObject *qobj;
+    int ret, i;
+    g_autofree char *cfg_file = NULL, *sedcmd = NULL;
+    g_autofree char *hd_file = NULL, *cd_file = NULL;
+
+    struct {
+        const char *name;
+        const char *type;
+    } devices[] = {
+        { "ich9-pcie-port-1", "ioh3420" },
+        { "ich9-pcie-port-2", "ioh3420" },
+        { "ich9-pcie-port-3", "ioh3420" },
+        { "ich9-pcie-port-4", "ioh3420" },
+        { "ich9-pci-bridge", "i82801b11-bridge" },
+        { "ich9-ehci-1", "ich9-usb-ehci1" },
+        { "ich9-ehci-2", "ich9-usb-ehci2" },
+        { "ich9-uhci-1", "ich9-usb-uhci1" },
+        { "ich9-uhci-2", "ich9-usb-uhci2" },
+        { "ich9-uhci-3", "ich9-usb-uhci3" },
+        { "ich9-uhci-4", "ich9-usb-uhci4" },
+        { "ich9-uhci-5", "ich9-usb-uhci5" },
+        { "ich9-uhci-6", "ich9-usb-uhci6" },
+        { "sata-disk", "ide-hd" },
+        { "sata-optical-disk", "ide-cd" },
+        { "net", "e1000" },
+        { "video", "VGA" },
+        { "ich9-hda-audio", "ich9-intel-hda" },
+        { "ich9-hda-duplex", "hda-duplex" },
+    };
+
+    /* Check that all the devices are available in the QEMU binary */
+    for (i = 0; i < ARRAY_SIZE(devices); i++) {
+        if (!qtest_has_device(devices[i].type)) {
+            g_test_skip("one of the required devices is not available");
+            return;
+        }
+    }
+
+    hd_file = make_temp_img("qtest_disk_XXXXXX.qcow2", "qcow2", 1);
+    cd_file = make_temp_img("qtest_cdrom_XXXXXX.iso", "raw", 1);
+    if (!hd_file || !cd_file) {
+        g_test_skip("could not create disk images");
+        goto cleanup;
+    }
+
+    /* Create a temporary config file where we replace the disk image names */
+    ret = g_file_open_tmp("q35-emulated-XXXXXX.cfg", &cfg_file, NULL);
+    if (ret == -1) {
+        g_test_skip("could not create temporary config file");
+        goto cleanup;
+    }
+    close(ret);
+
+    sedcmd = g_strdup_printf("sed -e 's,guest.qcow2,%s,' -e 's,install.iso,%s,'"
+                             " docs/config/q35-emulated.cfg > '%s'",
+                             hd_file, cd_file, cfg_file);
+    ret = system(sedcmd);
+    if (ret) {
+        g_test_skip("could not modify temporary config file");
+        goto cleanup;
+    }
+
+    qts = qtest_initf("-machine none -nodefaults -readconfig %s", cfg_file);
+
+    /* Check memory size */
+    resp = qtest_qmp(qts, "{ 'execute': 'query-memdev' }");
+    test_x86_memdev_resp(qdict_get(resp, "return"), "pc.ram", 1024);
+    qobject_unref(resp);
+
+    resp = qtest_qmp(qts, "{ 'execute': 'qom-list',"
+                          "  'arguments': {'path': '/machine/peripheral' }}");
+    qobj = qdict_get(resp, "return");
+
+    /* Check that all the devices have been created */
+    for (i = 0; i < ARRAY_SIZE(devices); i++) {
+        test_object_available(qobj, devices[i].name, devices[i].type);
+    }
+
+    qobject_unref(resp);
+
+    qtest_quit(qts);
+
+cleanup:
+    if (hd_file) {
+        unlink(hd_file);
+    }
+    if (cd_file) {
+        unlink(cd_file);
+    }
+    if (cfg_file) {
+        unlink(cfg_file);
+    }
+}
+
+#endif /* CONFIG_LINUX */
+
 int main(int argc, char *argv[])
 {
     const char *arch;
@@ -208,6 +334,11 @@ int main(int argc, char *argv[])
         g_str_equal(arch, "x86_64")) {
         qtest_add_func("readconfig/x86/memdev", test_x86_memdev);
         qtest_add_func("readconfig/x86/ich9-ehci-uhci", test_docs_config_ich9);
+#ifdef CONFIG_LINUX
+        if (qtest_has_accel("kvm")) {
+            qtest_add_func("readconfig/x86/q35-emulated", test_docs_q35_emulated);
+        }
+#endif
     }
 #ifdef CONFIG_SPICE
     qtest_add_func("readconfig/spice", test_spice);
-- 
2.31.1