Re: [Qemu-devel] [PATCH 3/7] device-introspect-test: New, covering device introspection

[Eric Blake]

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/

> 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 <address@hidden>
> ---
>  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?

> +++ 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/

> +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?

> +
> +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?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature