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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list