On Mon, Sep 21, 2020 at 15:07:31 +0200, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake <twiederh@xxxxxxxxxx>
> ---
> tests/cputest.c | 45 ++++++++++++++++++++++++++++++---------------
> 1 file changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/tests/cputest.c b/tests/cputest.c
> index b40fd7f7f2..30a125c3da 100644
> --- a/tests/cputest.c
> +++ b/tests/cputest.c
> @@ -68,7 +68,7 @@ static virQEMUDriver driver;
>
>
> static virCPUDefPtr
> -cpuTestLoadXML(virArch arch, const char *name)
> +cpuTestLoadXML(virArch arch, const char *name, bool validate)
> {
> char *xml = NULL;
> xmlDocPtr doc = NULL;
> @@ -81,7 +81,7 @@ cpuTestLoadXML(virArch arch, const char *name)
> if (!(doc = virXMLParseFileCtxt(xml, &ctxt)))
> goto cleanup;
>
> - virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_AUTO, &cpu, false);
> + virCPUDefParseXML(ctxt, NULL, VIR_CPU_TYPE_AUTO, &cpu, validate);
Is there any reason why we can't just always pass 'true' here?
>
> cleanup:
> xmlXPathFreeContext(ctxt);
> @@ -203,12 +203,20 @@ cpuTestCompare(const void *arg)
> virCPUDefPtr host = NULL;
> virCPUDefPtr cpu = NULL;
> virCPUCompareResult result;
> + bool validate = !!(data->flags & VIR_CONNECT_COMPARE_CPU_VALIDATE_XML);
>
> - if (!(host = cpuTestLoadXML(data->arch, data->host)) ||
> - !(cpu = cpuTestLoadXML(data->arch, data->name)))
> - goto cleanup;
> + host = cpuTestLoadXML(data->arch, data->host, validate);
> + cpu = cpuTestLoadXML(data->arch, data->name, validate);
> +
> + if (!host || !cpu) {
> + if (validate)
> + result = VIR_CPU_COMPARE_ERROR;
> + else
> + goto cleanup;
> + } else {
> + result = virCPUCompare(host->arch, host, cpu, false);
> + }
Same here, we should always validate the XML files. Also reporting a
validation failure as a COMPARE_ERROR seems wrong IMO.
