[libvirt] [PATCH 12/41] tests: Add CPU detection tests

[Peter Krempa]

On Wed, Jun 08, 2016 at 10:22:26 +0200, Jiri Denemark wrote:
> So far we only test CPUID -> CPU def conversion on artificial CPUID data
> computed from another CPU def. This patch adds the infrastructure to
> test this conversion on real data gathered from a host CPU and two
> helper scripts for adding new test data:
> 
> - cpu-gather.sh runs cpuid tool and qemu-system-x86_64 to get CPUID data
>   from the host CPU; this is what users can be asked to run if they run
>   into an issue with host CPU detection in libvirt
> 
> - cpu-parse.sh takes the data generated by cpu-gather.sh and creates
>   data files for CPU detection tests
> 
> The CPUID data queried from QEMU will eventually switch to the format
> used by query-host-cpu QMP command once QEMU implements it. Until then
> we just spawn QEMU with -cpu host and query the guest CPU in QOM. They
> should both provide the same CPUID results, but query-host-cpu does not
> require any guest CPU to be created by QEMU.
> 
> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
> ---
>  tests/Makefile.am               |   4 ++
>  tests/cputest.c                 | 156 +++++++++++++++++++++++++++++++++++++++-
>  tests/cputestdata/cpu-gather.sh |  35 +++++++++
>  tests/cputestdata/cpu-parse.sh  |  57 +++++++++++++++
>  4 files changed, 249 insertions(+), 3 deletions(-)
>  create mode 100755 tests/cputestdata/cpu-gather.sh
>  create mode 100755 tests/cputestdata/cpu-parse.sh

[...]

> diff --git a/tests/cputest.c b/tests/cputest.c
> index 431b587..2b243bb 100644
> --- a/tests/cputest.c
> +++ b/tests/cputest.c

[...]

> @@ -458,12 +474,114 @@ cpuTestHasFeature(const void *arg)
>  }
>  
>  
> +static int
> +cpuTestCPUID(const void *arg)
> +{
> +    const struct data *data = arg;
> +    int ret = -1;
> +    virCPUDataPtr hostData = NULL;
> +    char *hostFile = NULL;

hostFile is not freed

> +    char *host;

'host' is uninitialized and not freed

> +    virCPUDefPtr cpu = NULL;
> +    char *result = NULL;
> +
> +    if (virAsprintf(&hostFile, "%s/cputestdata/%s-cpuid-%s.xml",
> +                    abs_srcdir, data->arch, data->host) < 0 ||
> +        virtTestLoadFile(hostFile, &host) < 0 ||
> +        !(hostData = cpuDataParse(host)))
> +        goto cleanup;

This one monster if condition is ugly.

> +
> +    if (VIR_ALLOC(cpu) < 0)
> +        goto cleanup;
> +
> +    cpu->arch = hostData->arch;
> +    if (data->api == API_GUEST_CPUID) {
> +        cpu->type = VIR_CPU_TYPE_GUEST;
> +        cpu->match = VIR_CPU_MATCH_EXACT;
> +        cpu->fallback = VIR_CPU_FALLBACK_FORBID;
> +    } else {
> +        cpu->type = VIR_CPU_TYPE_HOST;
> +    }
> +
> +    if (cpuDecode(cpu, hostData, NULL, 0, NULL) < 0)
> +        goto cleanup;
> +
> +    if (virAsprintf(&result, "cpuid-%s-%s",
> +                    data->host,
> +                    data->api == API_HOST_CPUID ? "host" : "guest") < 0)
> +        goto cleanup;
> +
> +    ret = cpuTestCompareXML(data->arch, cpu, result, false);
> +
> + cleanup:
> +    cpuDataFree(hostData);
> +    virCPUDefFree(cpu);
> +    VIR_FREE(result);
> +    return ret;
> +}

ACK