Eric Blake <ebl...@redhat.com> writes: > On 09/18/2015 06:00 AM, Markus Armbruster wrote: >> The test doesn't check the output makes any sense, only that QEMU > > Reads slightly better as: > s/check/check that/
Okay. >> 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. Blacklist them in >> the test. The next commits will fix. >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> tests/Makefile | 11 ++- >> tests/device-introspect-test.c | 149 >> +++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 157 insertions(+), 3 deletions(-) >> create mode 100644 tests/device-introspect-test.c >> > >> @@ -478,8 +483,8 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): >> check-qtest-%: $(check-qtest-y) >> $(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \ >> QTEST_QEMU_IMG=qemu-img$(EXESUF) \ >> MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \ >> - gtester $(GTESTER_OPTIONS) -m=$(SPEED) >> $(check-qtest-$*-y),"GTESTER $@") >> - $(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \ >> + gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-generic-y) >> $(check-qtest-$*-y),"GTESTER $@") > > Long line; worth adding a \ line-wrap? I'll look into it. >> +++ b/tests/device-introspect-test.c >> @@ -0,0 +1,149 @@ > >> +/* >> + * Covers QMP device-list-properties and HMP device_add help. We >> + * currently don't check their output makes sense, only that QEMU > > again, might be better as: > s/check/check that/ Okay. >> +static void test_one_device(const char *type) >> +{ >> + QDict *resp; >> + char *help; >> + >> + /* FIXME device-list-properties crashes for abstract device, skip */ >> + 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); > > The comment mentions a skip, and the commit message header mentioned a > blacklist, but I'm not seeing a skip here. Am I missing something? The conditional skips part of the test when type is "device", because that part crashes. >> + >> +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", >> + }; > > Okay, here's the blacklist the commit message mentioned, but maybe that > means the earlier comment is stale? The commit message is slightly inaccurate: unlike the other devices, "device" isn't blacklisted completely. Good enough?