diff mbox

[v3,4/7] device-introspect-test: New, covering device introspection

Message ID 1443121042-3409-5-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Sept. 24, 2015, 6:57 p.m. UTC
The test doesn't check that the output makes any sense, only that QEMU
survives.  Useful since we've had an astounding number of crash bugs
around there.

In fact, we have a bunch of them right now: several devices crash or
hang, and all CPUs leave a dangling pointer behind.  The test skips
testing the broken parts.  The next commits will fix them, and drop
the skipping.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 tests/Makefile                 |   8 ++-
 tests/device-introspect-test.c | 153 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 158 insertions(+), 3 deletions(-)
 create mode 100644 tests/device-introspect-test.c

Comments

Thomas Huth Sept. 25, 2015, 10:17 a.m. UTC | #1
On 24/09/15 20:57, Markus Armbruster wrote:
> The test doesn't check that the output makes any sense, only that QEMU
> survives.  Useful since we've had an astounding number of crash bugs
> around there.
> 
> In fact, we have a bunch of them right now: several devices crash or
> hang, and all CPUs leave a dangling pointer behind.  The test skips
> testing the broken parts.  The next commits will fix them, and drop
> the skipping.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  tests/Makefile                 |   8 ++-
>  tests/device-introspect-test.c | 153 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 158 insertions(+), 3 deletions(-)
>  create mode 100644 tests/device-introspect-test.c

 Hi Markus,

just a quick note: When I run the tester directly, it aborts:

$ tests/boot-order-test
**
ERROR:/home/thuth/devel/qemu/tests/libqtest.c:517:qtest_get_arch:
assertion failed: (qemu != NULL)
Aborted (core dumped)

... that's a little bit ugly, maybe you could print the help text instead?

 Thomas
Andreas Färber Sept. 25, 2015, 10:18 a.m. UTC | #2
Am 25.09.2015 um 12:17 schrieb Thomas Huth:
> On 24/09/15 20:57, Markus Armbruster wrote:
>> The test doesn't check that the output makes any sense, only that QEMU
>> survives.  Useful since we've had an astounding number of crash bugs
>> around there.
>>
>> In fact, we have a bunch of them right now: several devices crash or
>> hang, and all CPUs leave a dangling pointer behind.  The test skips
>> testing the broken parts.  The next commits will fix them, and drop
>> the skipping.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>> ---
>>  tests/Makefile                 |   8 ++-
>>  tests/device-introspect-test.c | 153 +++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 158 insertions(+), 3 deletions(-)
>>  create mode 100644 tests/device-introspect-test.c
> 
>  Hi Markus,
> 
> just a quick note: When I run the tester directly, it aborts:
> 
> $ tests/boot-order-test
> **
> ERROR:/home/thuth/devel/qemu/tests/libqtest.c:517:qtest_get_arch:
> assertion failed: (qemu != NULL)
> Aborted (core dumped)
> 
> ... that's a little bit ugly, maybe you could print the help text instead?

That's got nothing to do with his test, all QTests require environment
variables set in the Makefile. Feel free to send a patch for qtest.c.

Regards,
Andreas
Markus Armbruster Sept. 25, 2015, 2:13 p.m. UTC | #3
Andreas Färber <afaerber@suse.de> writes:

> Am 25.09.2015 um 12:17 schrieb Thomas Huth:
>> On 24/09/15 20:57, Markus Armbruster wrote:
>>> The test doesn't check that the output makes any sense, only that QEMU
>>> survives.  Useful since we've had an astounding number of crash bugs
>>> around there.
>>>
>>> In fact, we have a bunch of them right now: several devices crash or
>>> hang, and all CPUs leave a dangling pointer behind.  The test skips
>>> testing the broken parts.  The next commits will fix them, and drop
>>> the skipping.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>> ---
>>>  tests/Makefile                 |   8 ++-
>>>  tests/device-introspect-test.c | 153
>>> +++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 158 insertions(+), 3 deletions(-)
>>>  create mode 100644 tests/device-introspect-test.c
>> 
>>  Hi Markus,
>> 
>> just a quick note: When I run the tester directly, it aborts:
>> 
>> $ tests/boot-order-test
>> **
>> ERROR:/home/thuth/devel/qemu/tests/libqtest.c:517:qtest_get_arch:
>> assertion failed: (qemu != NULL)
>> Aborted (core dumped)
>> 
>> ... that's a little bit ugly, maybe you could print the help text instead?
>
> That's got nothing to do with his test, all QTests require environment
> variables set in the Makefile. Feel free to send a patch for qtest.c.

Here's an example run I fished out of my bash history:

$ QTEST_QEMU_BINARY="ppc64-softmmu/qemu-system-ppc64" QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m=quick tests/device-help-test

Or with valgrind spliced in:

$ QTEST_QEMU_BINARY="valgrind --vgdb-error=1 --log-file=vg.log ppc64-softmmu/qemu-system-ppc64" QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((RANDOM % 255 + 1))} gtester -k --verbose -m=quick tests/device-help-test
diff mbox

Patch

diff --git a/tests/Makefile b/tests/Makefile
index 9380e14..2bf7ba1 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -86,7 +86,8 @@  check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
 # All QTests for now are POSIX-only, but the dependencies are
 # really in libqtest, not in the testcases themselves.
 
-check-qtest-generic-y =
+check-qtest-generic-y = tests/device-introspect-test$(EXESUF)
+gcov-files-generic-y = qdev-monitor.c qmp.c
 
 gcov-files-ipack-y += hw/ipack/ipack.c
 check-qtest-ipack-y += tests/ipoctal232-test$(EXESUF)
@@ -381,6 +382,7 @@  libqos-imx-obj-y = $(libqos-obj-y) tests/libqos/i2c-imx.o
 libqos-usb-obj-y = $(libqos-pc-obj-y) tests/libqos/usb.o
 libqos-virtio-obj-y = $(libqos-pc-obj-y) tests/libqos/virtio.o tests/libqos/virtio-pci.o tests/libqos/virtio-mmio.o tests/libqos/malloc-generic.o
 
+tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
 tests/rtc-test$(EXESUF): tests/rtc-test.o
 tests/m48t59-test$(EXESUF): tests/m48t59-test.o
 tests/endianness-test$(EXESUF): tests/endianness-test.o
@@ -488,7 +490,7 @@  $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y)
 		QTEST_QEMU_IMG=qemu-img$(EXESUF) \
 		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
 		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER $@")
-	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \
+	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y) $(gcov-files-generic-y); do \
 	  echo Gcov report for $$f:;\
 	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
 	done,)
@@ -499,7 +501,7 @@  $(patsubst %, check-%, $(check-unit-y)): check-%: %
 	$(call quiet-command, \
 		MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \
 		gtester $(GTESTER_OPTIONS) -m=$(SPEED) $*,"GTESTER $*")
-	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y); do \
+	$(if $(CONFIG_GCOV),@for f in $(gcov-files-$(subst tests/,,$*)-y) $(gcov-files-generic-y); do \
 	  echo Gcov report for $$f:;\
 	  $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
 	done,)
diff --git a/tests/device-introspect-test.c b/tests/device-introspect-test.c
new file mode 100644
index 0000000..44da30e
--- /dev/null
+++ b/tests/device-introspect-test.c
@@ -0,0 +1,153 @@ 
+/*
+ * Device introspection test cases
+ *
+ * Copyright (c) 2015 Red Hat Inc.
+ *
+ * Authors:
+ *  Markus Armbruster <armbru@redhat.com>,
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+/*
+ * Covers QMP device-list-properties and HMP device_add help.  We
+ * currently don't check that their output makes sense, only that QEMU
+ * survives.  Useful since we've had an astounding number of crash
+ * bugs around here.
+ */
+
+#include <glib.h>
+#include <stdarg.h>
+#include "qemu-common.h"
+#include "qapi/qmp/qstring.h"
+#include "libqtest.h"
+
+const char common_args[] = "-nodefaults -machine none";
+
+static QList *device_type_list(bool abstract)
+{
+    QDict *resp;
+    QList *ret;
+
+    resp = qmp("{'execute': 'qom-list-types',"
+               " 'arguments': {'implements': 'device', 'abstract': %i}}",
+               abstract);
+    g_assert(qdict_haskey(resp, "return"));
+    ret = qdict_get_qlist(resp, "return");
+    QINCREF(ret);
+    QDECREF(resp);
+    return ret;
+}
+
+static void test_one_device(const char *type)
+{
+    QDict *resp;
+    char *help;
+
+    /*
+     * Skip this part for the abstract device test case, because
+     * device-list-properties crashes for such devices.
+     * FIXME fix it not to crash
+     */
+    if (strcmp(type, "device")) {
+        resp = qmp("{'execute': 'device-list-properties',"
+                   " 'arguments': {'typename': %s}}",
+                   type);
+        QDECREF(resp);
+    }
+
+    help = hmp("device_add \"%s,help\"", type);
+    g_free(help);
+}
+
+static void test_device_intro_list(void)
+{
+    QList *types;
+    char *help;
+
+    qtest_start(common_args);
+
+    types = device_type_list(true);
+    QDECREF(types);
+
+    help = hmp("device_add help");
+    g_free(help);
+
+    qtest_end();
+}
+
+static void test_device_intro_none(void)
+{
+    qtest_start(common_args);
+    test_one_device("nonexistent");
+    qtest_end();
+}
+
+static void test_device_intro_abstract(void)
+{
+    qtest_start(common_args);
+    test_one_device("device");
+    qtest_end();
+}
+
+static bool blacklisted(const char *type)
+{
+    static const char *blacklist[] = {
+        /* crash in object_unref(): */
+        "pxa2xx-pcmcia",
+        /* hang in object_unref(): */
+        "realview_pci", "versatile_pci", "s390-sclp-event-facility", "sclp",
+        /* create a CPU, thus use after free (see below): */
+        "allwinner-a10", "digic", "fsl,imx25", "fsl,imx31", "xlnx,zynqmp",
+    };
+    size_t len = strlen(type);
+    int i;
+
+    if (len >= 4 && !strcmp(type + len - 4, "-cpu")) {
+        /* use after free: cpu_exec_init() saves CPUState in cpus */
+        return true;
+    }
+
+    for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
+        if (!strcmp(blacklist[i], type)) {
+            return true;
+        }
+    }
+    return false;
+}
+
+static void test_device_intro_concrete(void)
+{
+    QList *types;
+    QListEntry *entry;
+    const char *type;
+
+    qtest_start(common_args);
+    types = device_type_list(false);
+
+    QLIST_FOREACH_ENTRY(types, entry) {
+        type = qdict_get_try_str(qobject_to_qdict(qlist_entry_obj(entry)),
+                                "name");
+        g_assert(type);
+        if (blacklisted(type)) {
+            continue;           /* FIXME broken device, skip */
+        }
+        test_one_device(type);
+    }
+
+    QDECREF(types);
+    qtest_end();
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    qtest_add_func("device/introspect/list", test_device_intro_list);
+    qtest_add_func("device/introspect/none", test_device_intro_none);
+    qtest_add_func("device/introspect/abstract", test_device_intro_abstract);
+    qtest_add_func("device/introspect/concrete", test_device_intro_concrete);
+
+    return g_test_run();
+}