Re: [libvirt] [PATCH 16/18] cpu: Parse and use PVR masks in the ppc64 driver

6 Aug 2015


      On Thu, 2015-08-06 at 16:42 +0200, Jiri Denemark wrote:
...
...
@@ -364,6 +358,24 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt,
         model->data->pvr[i].value = pvr;
VIR_FREE(prop);
+
+        if (!(prop = virXMLPropString(nodes[i], "mask"))) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Missing PVR mask in CPU model %s"),
+                           model->name);
+            goto ignore;
+        }
+
+        if (virStrToLong_ul(prop, NULL, 16, &pvr) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Invalid PVR mask in CPU model %s"),
+                           model->name);
+            goto ignore;
+        }
+
+        model->data->pvr[i].mask = pvr;
Wouldn't virXPathULongHex be good enough?
See the replay to the same remark in patch 13/18.
...
...
+
+        VIR_FREE(prop);
     }
if (!map->models) {
@@ -607,33 +619,33 @@ ppc64DriverFree(virCPUDataPtr data)
 static virCPUDataPtr
 ppc64DriverNodeData(virArch arch)
 {
-    virCPUDataPtr cpuData;
-
-    if (VIR_ALLOC(cpuData) < 0)
-        goto error;
-
-    if (VIR_ALLOC(cpuData->data.ppc64) < 0)
-        goto error;
-
-    if (VIR_ALLOC_N(cpuData->data.ppc64->pvr, 1) < 0)
-        goto error;
-
-    cpuData->data.ppc64->len = 1;
-
-    cpuData->arch = arch;
+    virCPUDataPtr nodeData = NULL;
+    struct ppc64_map *map = NULL;
+    struct ppc64_model *model = NULL;
+    uint32_t pvr = 0;
#if defined(__powerpc__) || defined(__powerpc64__)
     asm("mfpvr %0"
-        : "=r" (cpuData->data.ppc64->pvr[0].value));
+        : "=r" (pvr));
 #endif
-    return cpuData;
+    if (!(map = ppc64LoadMap()))
+        goto cleanup;
- error:
-    if (cpuData)
-        ppc64DataFree(cpuData->data.ppc64);
-    VIR_FREE(cpuData);
-    return NULL;
+    if (!(model = ppc64ModelFindPVR(map, pvr))) {
+        virReportError(VIR_ERR_OPERATION_FAILED,
+                       _("Cannot find CPU model with PVR 0x%08x"),
+                       pvr);
+        goto cleanup;
+    }
+
+    if (!(nodeData = ppc64MakeCPUData(arch, model->data)))
+        goto cleanup;
+
+ cleanup:
+    ppc64MapFree(map);
+
+    return nodeData;
 }
The ppc64DriverNodeData API is supposed to return raw data from the
host, which is then passed to ppc64DriverDecode to be translated into 
a
CPU model name. In other words, you copied most of the internals of
ppc64DriverDecode here while you in fact only need to add a single 
line
cpuData->data.ppc64->pvr[0].mask = 0xffffffff;
Done.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

    