On Tue, Aug 04, 2015 at 11:38:04 +0200, Andrea Bolognani wrote:
This will allow us to perform PVR matching more broadly, eg. consider both POWER8 and POWER8E CPUs to be the same even though they have different PVR values. --- src/cpu/cpu_ppc64.c | 73 ++++++++++++++++++++++++++++++++++++++++-------- src/cpu/cpu_ppc64_data.h | 8 +++++- 2 files changed, 69 insertions(+), 12 deletions(-)
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index d186d4c..9351729 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -63,6 +63,7 @@ ppc64DataFree(virCPUppc64Data *data) if (!data) return;
+ VIR_FREE(data->pvr); VIR_FREE(data); }
OK, ignore my comment on the previous patch :-)
@@ -70,16 +71,27 @@ static virCPUppc64Data * ppc64DataCopy(virCPUppc64Data *data) { virCPUppc64Data *copy; + size_t i;
if (!data) return NULL;
if (VIR_ALLOC(copy) < 0) - return NULL; + goto error; + + if (VIR_ALLOC_N(copy->pvr, data->len) < 0) + goto error; + + copy->len = data->len;
- copy->pvr = data->pvr; + for (i = 0; i < data->len; i++) + copy->pvr[i].value = data->pvr[i].value;
return copy; + + error: + ppc64DataFree(copy); + return NULL; }
static void @@ -168,11 +180,13 @@ ppc64ModelFindPVR(const struct ppc64_map *map, uint32_t pvr) { struct ppc64_model *model; + size_t i;
model = map->models; while (model) { - if (model->data->pvr == pvr) - return model; + for (i = 0; i < model->data->len; i++) + if (model->data->pvr[i].value == pvr) + return model;
I think the for loop would deserve {} around its body.
model = model->next; } @@ -274,8 +288,12 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt, struct ppc64_map *map) { struct ppc64_model *model; + xmlNodePtr *nodes = NULL; char *vendor = NULL; + char *prop = NULL; unsigned long pvr; + size_t i; + int n;
if (VIR_ALLOC(model) < 0) return -1; @@ -315,14 +333,38 @@ ppc64ModelLoad(xmlXPathContextPtr ctxt, } }
- if (!virXPathBoolean("boolean(./pvr)", ctxt) || - virXPathULongHex("string(./pvr/@value)", ctxt, &pvr) < 0) { + if ((n = virXPathNodeSet("./pvr", ctxt, &nodes)) <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("Missing or invalid PVR value in CPU model %s"), + _("Missing PVR information for CPU model %s"), model->name); goto ignore; } - model->data->pvr = pvr; + + if (VIR_ALLOC_N(model->data->pvr, n) < 0) + goto ignore; + + model->data->len = n; + + for (i = 0; i < n; i++) { +
Drop the empty line here.
+ if (!(prop = virXMLPropString(nodes[i], "value"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Missing PVR value in CPU model %s"), + model->name); + goto ignore; + } + + if (virStrToLong_ul(prop, NULL, 16, &pvr) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid PVR value in CPU model %s"), + model->name); + goto ignore; + }
Any particular reason to replace virXPathULongHex with the above code?
+ + model->data->pvr[i].value = pvr; + + VIR_FREE(prop); + }
if (!map->models) { map->models = model; ...
Looks OK otherwise. Jirka