[libvirt PATCH 8/9] tests: cpu: Allow passing flags to cpuTestLoadXML

Tim Wiederhake posted 9 patches 5 years, 4 months ago
There is a newer version of this series
[libvirt PATCH 8/9] tests: cpu: Allow passing flags to cpuTestLoadXML
Posted by Tim Wiederhake 5 years, 4 months ago
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
---
 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);
 
  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);
+    }
 
-    result = virCPUCompare(host->arch, host, cpu, false);
     if (data->result == VIR_CPU_COMPARE_ERROR)
         virResetLastError();
 
@@ -240,9 +248,10 @@ cpuTestGuestCPU(const void *arg)
     virCPUCompareResult cmpResult;
     g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
     char *result = NULL;
+    bool validate = !!(data->flags & VIR_CONNECT_COMPARE_CPU_VALIDATE_XML);
 
-    if (!(host = cpuTestLoadXML(data->arch, data->host)) ||
-        !(cpu = cpuTestLoadXML(data->arch, data->name)))
+    if (!(host = cpuTestLoadXML(data->arch, data->host, validate)) ||
+        !(cpu = cpuTestLoadXML(data->arch, data->name, validate)))
         goto cleanup;
 
     if (virCPUConvertLegacy(host->arch, cpu) < 0)
@@ -381,9 +390,10 @@ cpuTestUpdate(const void *arg)
     virCPUDefPtr migHost = NULL;
     virCPUDefPtr cpu = NULL;
     char *result = NULL;
+    bool validate = !!(data->flags & VIR_CONNECT_COMPARE_CPU_VALIDATE_XML);
 
-    if (!(host = cpuTestLoadXML(data->arch, data->host)) ||
-        !(cpu = cpuTestLoadXML(data->arch, data->name)))
+    if (!(host = cpuTestLoadXML(data->arch, data->host, validate)) ||
+        !(cpu = cpuTestLoadXML(data->arch, data->name, validate)))
         goto cleanup;
 
     if (!(migHost = virCPUCopyMigratable(data->arch, host)))
@@ -413,8 +423,9 @@ cpuTestHasFeature(const void *arg)
     virCPUDefPtr host = NULL;
     virCPUDataPtr hostData = NULL;
     int result;
+    bool validate = !!(data->flags & VIR_CONNECT_COMPARE_CPU_VALIDATE_XML);
 
-    if (!(host = cpuTestLoadXML(data->arch, data->host)))
+    if (!(host = cpuTestLoadXML(data->arch, data->host, validate)))
         goto cleanup;
 
     if (cpuEncode(host->arch, host, NULL, &hostData,
@@ -794,9 +805,10 @@ cpuTestUpdateLive(const void *arg)
     virDomainCapsCPUModelsPtr hvModels = NULL;
     virDomainCapsCPUModelsPtr models = NULL;
     int ret = -1;
+    bool validate = !!(data->flags & VIR_CONNECT_COMPARE_CPU_VALIDATE_XML);
 
     cpuFile = g_strdup_printf("cpuid-%s-guest", data->host);
-    if (!(cpu = cpuTestLoadXML(data->arch, cpuFile)))
+    if (!(cpu = cpuTestLoadXML(data->arch, cpuFile, validate)))
         goto cleanup;
 
     enabledFile = g_strdup_printf("%s/cputestdata/%s-cpuid-%s-enabled.xml",
@@ -812,7 +824,7 @@ cpuTestUpdateLive(const void *arg)
         goto cleanup;
 
     expectedFile = g_strdup_printf("cpuid-%s-json", data->host);
-    if (!(expected = cpuTestLoadXML(data->arch, expectedFile)))
+    if (!(expected = cpuTestLoadXML(data->arch, expectedFile, validate)))
         goto cleanup;
 
     /* In case the host CPU signature does not exactly match any CPU model from
@@ -1018,10 +1030,13 @@ mymain(void)
         VIR_FREE(testLabel); \
     } while (0)
 
-#define DO_TEST_COMPARE(arch, host, cpu, result) \
+#define DO_TEST_COMPARE_FLAGS(arch, host, cpu, result, flags) \
     DO_TEST(arch, cpuTestCompare, \
             host "/" cpu " (" #result ")", \
-            host, cpu, NULL, 0, result)
+            host, cpu, NULL, flags, result)
+
+#define DO_TEST_COMPARE(arch, host, cpu, result) \
+    DO_TEST_COMPARE_FLAGS(arch, host, cpu, result, 0)
 
 #define DO_TEST_UPDATE_ONLY(arch, host, cpu) \
     DO_TEST(arch, cpuTestUpdate, \
-- 
2.26.2

Re: [libvirt PATCH 8/9] tests: cpu: Allow passing flags to cpuTestLoadXML
Posted by Peter Krempa 5 years, 4 months ago
On Mon, Sep 21, 2020 at 15:07:31 +0200, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
> ---
>  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.

Re: [libvirt PATCH 8/9] tests: cpu: Allow passing flags to cpuTestLoadXML
Posted by Tim Wiederhake 5 years, 4 months ago
On Tue, 2020-09-29 at 11:21 +0200, Peter Krempa wrote:
> (...)
> On Mon, Sep 21, 2020 at 15:07:31 +0200, Tim Wiederhake wrote:
> > Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
> > ---
> >  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
> > @@ -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);
> > +    }
> 
> (...) Also reporting a
> validation failure as a COMPARE_ERROR seems wrong IMO.

The enum virCPUCompareResult in libvirt-host.h defines the values
VIR_CPU_COMPARE_{ERROR,INCOMPATIBLE,IDENTICAL,SUPERSET}. I believe
VIR_CPU_COMPARE_ERROR to be correct here, but have rewritten this part
for v3.

Tim