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

[Andrea Bolognani]

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