[Qemu-devel] [PATCH v3 0/2] Add a test for serial output (and sort tests by architecture in the Makefile)

[Qemu-devel] [PATCH v3 0/2] Add a test for serial output (and sort tests by architecture in the Makefile)

[Thomas Huth]

Since some of the machines that we support in QEMU sometimes get
broken by other changes, and it then takes a while 'till somebody
notices the breakage, it would be nice to get some more automatic
test coverage for various machines during "make check". The
second patch adds such a test for the machines where we've got
a firmware image for and thus can test for some magic strings
in the serial output of the firmware.

The first patch is just a clean-up for the Makefile to avoid that
new tests get added with "=" instead of "+=" (and thus completely
overwrite the correspond variable by accident).

v3:
- Rebased to master (there were some conflicts in Makefile.include)

v2:
- Some more clean-ups for the Makefile (i.e. some additional empty
  lines)
- Check return value of read() to make sure that there is no error

Thomas Huth (2):
  tests: Resort check-qtest entries in Makefile.include
  tests: Check serial output of firmware boot of some machines

 tests/Makefile.include   |  52 +++++++++++++++-------
 tests/boot-serial-test.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 146 insertions(+), 16 deletions(-)
 create mode 100644 tests/boot-serial-test.c

-- 
1.8.3.1

[Qemu-devel] [PATCH v3 1/2] tests: Resort check-qtest entries in Makefile.include

[Thomas Huth]

The rather random list of check-qtest-xxx entries caused some
confusion in the past, where to use "=" and where to use "+="
(see commits 0ccac16f59462b8e2b9afbc1 and 1f5c1cfbaec0792cd2e5da
for example).
Sorting the check-qtest-xxx entries by architecure instead and
using some empty lines inbetween should help to ease this
situation a little bit, so that it is hopefully now obvious
that new tests should be added with "+=" instead of "=".
While we are at it, this patch also comments out two of the
"gcov-files-..." lines since the corresponding m48t59-test is
disabled for sparc and sparc64, too.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include | 44 ++++++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 14be491..3873ee0 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -241,37 +241,49 @@ check-qtest-i386-y += tests/postcopy-test$(EXESUF)
 check-qtest-x86_64-y += $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
 gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
+
 check-qtest-mips-y = tests/endianness-test$(EXESUF)
+
 check-qtest-mips64-y = tests/endianness-test$(EXESUF)
+
 check-qtest-mips64el-y = tests/endianness-test$(EXESUF)
+
 check-qtest-ppc-y = tests/endianness-test$(EXESUF)
-check-qtest-ppc64-y = tests/endianness-test$(EXESUF)
+check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
+check-qtest-ppc-y += tests/prom-env-test$(EXESUF)
+check-qtest-ppc-y += tests/drive_del-test$(EXESUF)
+
+check-qtest-ppc64-y = tests/spapr-phb-test$(EXESUF)
+gcov-files-ppc64-y = ppc64-softmmu/hw/ppc/spapr_pci.c
+check-qtest-ppc64-y += tests/endianness-test$(EXESUF)
+check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
+check-qtest-ppc64-y += tests/prom-env-test$(EXESUF)
+check-qtest-ppc64-y += tests/drive_del-test$(EXESUF)
+check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
+
 check-qtest-sh4-y = tests/endianness-test$(EXESUF)
+
 check-qtest-sh4eb-y = tests/endianness-test$(EXESUF)
+
+check-qtest-sparc-y = tests/prom-env-test$(EXESUF)
+#check-qtest-sparc-y += tests/m48t59-test$(EXESUF)
+#gcov-files-sparc-y = hw/timer/m48t59.c
+
 check-qtest-sparc64-y = tests/endianness-test$(EXESUF)
-#check-qtest-sparc-y = tests/m48t59-test$(EXESUF)
 #check-qtest-sparc64-y += tests/m48t59-test$(EXESUF)
-gcov-files-sparc-y += hw/timer/m48t59.c
-gcov-files-sparc64-y += hw/timer/m48t59.c
+#gcov-files-sparc64-y += hw/timer/m48t59.c
+#Disabled for now, triggers a TCG bug on 32-bit hosts
+#check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
+
 check-qtest-arm-y = tests/tmp105-test$(EXESUF)
 check-qtest-arm-y += tests/ds1338-test$(EXESUF)
 gcov-files-arm-y += hw/misc/tmp105.c
 check-qtest-arm-y += tests/virtio-blk-test$(EXESUF)
 gcov-files-arm-y += arm-softmmu/hw/block/virtio-blk.c
-check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
-check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
-check-qtest-ppc-y += tests/drive_del-test$(EXESUF)
-check-qtest-ppc64-y += tests/drive_del-test$(EXESUF)
-check-qtest-ppc64-y += tests/spapr-phb-test$(EXESUF)
-gcov-files-ppc64-y += ppc64-softmmu/hw/ppc/spapr_pci.c
-check-qtest-ppc-y += tests/prom-env-test$(EXESUF)
-check-qtest-ppc64-y += tests/prom-env-test$(EXESUF)
-check-qtest-sparc-y += tests/prom-env-test$(EXESUF)
-#Disabled for now, triggers a TCG bug on 32-bit hosts
-#check-qtest-sparc64-y += tests/prom-env-test$(EXESUF)
+
 check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
+
 check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
-check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
 
 check-qtest-generic-y += tests/qom-test$(EXESUF)
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 2/2] tests: Check serial output of firmware boot of some machines

[Thomas Huth]

Some of the machines that we have got a firmware image for write
some output to the serial console while booting up. We can use
this output to make sure that the machine is basically working,
so this adds a test that checks the output of these machines
for some well-known "magic" strings.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/Makefile.include   |   8 ++++
 tests/boot-serial-test.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 118 insertions(+)
 create mode 100644 tests/boot-serial-test.c

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3873ee0..1a0da69 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -195,6 +195,7 @@ check-qtest-i386-y += tests/hd-geo-test$(EXESUF)
 gcov-files-i386-y += hw/block/hd-geometry.c
 check-qtest-i386-y += tests/boot-order-test$(EXESUF)
 check-qtest-i386-y += tests/bios-tables-test$(EXESUF)
+check-qtest-i386-y += tests/boot-serial-test$(EXESUF)
 check-qtest-i386-y += tests/pxe-test$(EXESUF)
 check-qtest-i386-y += tests/rtc-test$(EXESUF)
 check-qtest-i386-y += tests/ipmi-kcs-test$(EXESUF)
@@ -242,6 +243,8 @@ check-qtest-x86_64-y += $(check-qtest-i386-y)
 gcov-files-i386-y += i386-softmmu/hw/timer/mc146818rtc.c
 gcov-files-x86_64-y = $(subst i386-softmmu/,x86_64-softmmu/,$(gcov-files-i386-y))
 
+check-qtest-alpha-y = tests/boot-serial-test$(EXESUF)
+
 check-qtest-mips-y = tests/endianness-test$(EXESUF)
 
 check-qtest-mips64-y = tests/endianness-test$(EXESUF)
@@ -252,6 +255,7 @@ check-qtest-ppc-y = tests/endianness-test$(EXESUF)
 check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
 check-qtest-ppc-y += tests/prom-env-test$(EXESUF)
 check-qtest-ppc-y += tests/drive_del-test$(EXESUF)
+check-qtest-ppc-y += tests/boot-serial-test$(EXESUF)
 
 check-qtest-ppc64-y = tests/spapr-phb-test$(EXESUF)
 gcov-files-ppc64-y = ppc64-softmmu/hw/ppc/spapr_pci.c
@@ -260,6 +264,7 @@ check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
 check-qtest-ppc64-y += tests/prom-env-test$(EXESUF)
 check-qtest-ppc64-y += tests/drive_del-test$(EXESUF)
 check-qtest-ppc64-y += tests/postcopy-test$(EXESUF)
+check-qtest-ppc64-y += tests/boot-serial-test$(EXESUF)
 
 check-qtest-sh4-y = tests/endianness-test$(EXESUF)
 
@@ -285,6 +290,8 @@ check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
 
 check-qtest-xtensaeb-y = $(check-qtest-xtensa-y)
 
+check-qtest-s390x-y = tests/boot-serial-test$(EXESUF)
+
 check-qtest-generic-y += tests/qom-test$(EXESUF)
 
 qapi-schema += alternate-any.json
@@ -590,6 +597,7 @@ tests/ipmi-kcs-test$(EXESUF): tests/ipmi-kcs-test.o
 tests/ipmi-bt-test$(EXESUF): tests/ipmi-bt-test.o
 tests/hd-geo-test$(EXESUF): tests/hd-geo-test.o
 tests/boot-order-test$(EXESUF): tests/boot-order-test.o $(libqos-obj-y)
+tests/boot-serial-test$(EXESUF): tests/boot-serial-test.o $(libqos-obj-y)
 tests/bios-tables-test$(EXESUF): tests/bios-tables-test.o \
 	tests/boot-sector.o $(libqos-obj-y)
 tests/pxe-test$(EXESUF): tests/pxe-test.o tests/boot-sector.o $(libqos-obj-y)
diff --git a/tests/boot-serial-test.c b/tests/boot-serial-test.c
new file mode 100644
index 0000000..b36c6bf
--- /dev/null
+++ b/tests/boot-serial-test.c
@@ -0,0 +1,110 @@
+/*
+ * Test serial output of some machines.
+ *
+ * Copyright 2016 Thomas Huth, Red Hat Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2
+ * or later. See the COPYING file in the top-level directory.
+ *
+ * This test is used to check that the serial output of the firmware
+ * (that we provide for some machines) contains an expected string.
+ * Thus we check that the firmware still boots at least to a certain
+ * point and so we know that the machine is not completely broken.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+
+typedef struct testdef {
+    const char *arch;       /* Target architecture */
+    const char *machine;    /* Name of the machine */
+    const char *extra;      /* Additional parameters */
+    const char *expect;     /* Expected string in the serial output */
+} testdef_t;
+
+static testdef_t tests[] = {
+    { "alpha", "clipper", "", "PCI:" },
+    { "ppc", "ppce500", "", "U-Boot" },
+    { "ppc", "prep", "", "Open Hack'Ware BIOS" },
+    { "ppc64", "ppce500", "", "U-Boot" },
+    { "ppc64", "prep", "", "Open Hack'Ware BIOS" },
+    { "ppc64", "pseries", "", "Open Firmware" },
+    { "i386", "isapc", "-device sga", "SGABIOS" },
+    { "i386", "pc", "-device sga", "SGABIOS" },
+    { "i386", "q35", "-device sga", "SGABIOS" },
+    { "x86_64", "isapc", "-device sga", "SGABIOS" },
+    { "x86_64", "q35", "-device sga", "SGABIOS" },
+    { "s390x", "s390-ccw-virtio",
+      "-nodefaults -device sclpconsole,chardev=serial0", "virtio device" },
+    { NULL }
+};
+
+static void check_guest_output(const testdef_t *test, int fd)
+{
+    bool output_ok = false;
+    int i, nbr, pos = 0;
+    char ch;
+
+    /* Poll serial output... Wait at most 60 seconds */
+    for (i = 0; i < 6000; ++i) {
+        while ((nbr = read(fd, &ch, 1)) == 1) {
+            if (ch == test->expect[pos]) {
+                pos += 1;
+                if (test->expect[pos] == '\0') {
+                    /* We've reached the end of the expected string! */
+                    output_ok = true;
+                    goto done;
+                }
+            } else {
+                pos = 0;
+            }
+        }
+        g_assert(nbr >= 0);
+        g_usleep(10000);
+    }
+
+done:
+    g_assert(output_ok);
+}
+
+static void test_machine(const void *data)
+{
+    const testdef_t *test = data;
+    char *args;
+    char tmpname[] = "/tmp/qtest-boot-serial-XXXXXX";
+    int fd;
+
+    fd = mkstemp(tmpname);
+    g_assert(fd != -1);
+
+    args = g_strdup_printf("-M %s,accel=tcg -chardev file,id=serial0,path=%s"
+                           " -serial chardev:serial0 %s", test->machine,
+                           tmpname, test->extra);
+
+    qtest_start(args);
+    unlink(tmpname);
+
+    check_guest_output(test, fd);
+    qtest_quit(global_qtest);
+
+    g_free(args);
+    close(fd);
+}
+
+int main(int argc, char *argv[])
+{
+    const char *arch = qtest_get_arch();
+    int i;
+
+    g_test_init(&argc, &argv, NULL);
+
+    for (i = 0; tests[i].arch != NULL; i++) {
+        if (strcmp(arch, tests[i].arch) == 0) {
+            char *name = g_strdup_printf("boot-serial/%s", tests[i].machine);
+            qtest_add_data_func(name, &tests[i], test_machine);
+            g_free(name);
+        }
+    }
+
+    return g_test_run();
+}
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 0/2] Add a test for serial output (and sort tests by architecture in the Makefile)

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1576 bytes --]

On Sat, Sep 03, 2016 at 11:57:49AM +0200, Thomas Huth wrote:
> Since some of the machines that we support in QEMU sometimes get
> broken by other changes, and it then takes a while 'till somebody
> notices the breakage, it would be nice to get some more automatic
> test coverage for various machines during "make check". The
> second patch adds such a test for the machines where we've got
> a firmware image for and thus can test for some magic strings
> in the serial output of the firmware.
> 
> The first patch is just a clean-up for the Makefile to avoid that
> new tests get added with "=" instead of "+=" (and thus completely
> overwrite the correspond variable by accident).

Applied to ppc-for-2.8, thanks.

> 
> v3:
> - Rebased to master (there were some conflicts in Makefile.include)
> 
> v2:
> - Some more clean-ups for the Makefile (i.e. some additional empty
>   lines)
> - Check return value of read() to make sure that there is no error
> 
> Thomas Huth (2):
>   tests: Resort check-qtest entries in Makefile.include
>   tests: Check serial output of firmware boot of some machines
> 
>  tests/Makefile.include   |  52 +++++++++++++++-------
>  tests/boot-serial-test.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 146 insertions(+), 16 deletions(-)
>  create mode 100644 tests/boot-serial-test.c
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]