Skip to content

Commit

Permalink
cpu: push more parsing logic into common code
Browse files Browse the repository at this point in the history 
The x86 and ppc impls both duplicate some logic when parsing CPU
features. Change the callback signature so that this duplication can be
pushed up a level to common code.

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
  • Loading branch information
@berrange
berrange committed Aug 28, 2018
1 parent 118fcdd commit 0815f51
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 285 deletions.
98 changes: 57 additions & 41 deletions src/cpu/cpu_map.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,31 +35,47 @@

VIR_LOG_INIT("cpu.cpu_map");

VIR_ENUM_IMPL(cpuMapElement, CPU_MAP_ELEMENT_LAST,
"vendor",
"feature",
"model")


static int load(xmlXPathContextPtr ctxt,
cpuMapElement element,
cpuMapLoadCallback callback,
void *data)
static int
loadData(const char *mapfile,
xmlXPathContextPtr ctxt,
const char *element,
cpuMapLoadCallback callback,
void *data)
{
int ret = -1;
xmlNodePtr ctxt_node;
xmlNodePtr *nodes = NULL;
int n;
size_t i;
int rv;

ctxt_node = ctxt->node;

n = virXPathNodeSet(cpuMapElementTypeToString(element), ctxt, &nodes);
if (n < 0)
if ((n = virXPathNodeSet(element, ctxt, &nodes)) < 0)
goto cleanup;

if (n > 0 &&
callback(element, ctxt, nodes, n, data) < 0)
if (n > 0 && !callback) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Unexpected element '%s' in CPU map '%s'"), element, mapfile);
goto cleanup;
}

for (i = 0; i < n; i++) {
xmlNodePtr old = ctxt->node;
char *name = virXMLPropString(nodes[i], "name");
if (!name) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot find %s name in CPU map '%s'"), element, mapfile);
goto cleanup;
}
VIR_DEBUG("Load %s name %s", element, name);
ctxt->node = nodes[i];
rv = callback(ctxt, name, data);
ctxt->node = old;
VIR_FREE(name);
if (rv < 0)
goto cleanup;
}

ret = 0;

Expand All @@ -72,13 +88,14 @@ static int load(xmlXPathContextPtr ctxt,

static int
cpuMapLoadInclude(const char *filename,
cpuMapLoadCallback cb,
cpuMapLoadCallback vendorCB,
cpuMapLoadCallback featureCB,
cpuMapLoadCallback modelCB,
void *data)
{
xmlDocPtr xml = NULL;
xmlXPathContextPtr ctxt = NULL;
int ret = -1;
int element;
char *mapfile;

if (!(mapfile = virFileFindResource(filename,
Expand All @@ -93,13 +110,14 @@ cpuMapLoadInclude(const char *filename,

ctxt->node = xmlDocGetRootElement(xml);

for (element = 0; element < CPU_MAP_ELEMENT_LAST; element++) {
if (load(ctxt, element, cb, data) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot parse CPU map '%s'"), mapfile);
goto cleanup;
}
}
if (loadData(mapfile, ctxt, "vendor", vendorCB, data) < 0)
goto cleanup;

if (loadData(mapfile, ctxt, "feature", featureCB, data) < 0)
goto cleanup;

if (loadData(mapfile, ctxt, "model", modelCB, data) < 0)
goto cleanup;

ret = 0;

Expand All @@ -114,7 +132,9 @@ cpuMapLoadInclude(const char *filename,

static int
loadIncludes(xmlXPathContextPtr ctxt,
cpuMapLoadCallback callback,
cpuMapLoadCallback vendorCB,
cpuMapLoadCallback featureCB,
cpuMapLoadCallback modelCB,
void *data)
{
int ret = -1;
Expand All @@ -137,7 +157,7 @@ loadIncludes(xmlXPathContextPtr ctxt,
goto cleanup;
}
VIR_DEBUG("Finding CPU map include '%s'", filename);
if (cpuMapLoadInclude(filename, callback, data) < 0) {
if (cpuMapLoadInclude(filename, vendorCB, featureCB, modelCB, data) < 0) {
VIR_FREE(filename);
goto cleanup;
}
Expand All @@ -155,15 +175,16 @@ loadIncludes(xmlXPathContextPtr ctxt,


int cpuMapLoad(const char *arch,
cpuMapLoadCallback cb,
cpuMapLoadCallback vendorCB,
cpuMapLoadCallback featureCB,
cpuMapLoadCallback modelCB,
void *data)
{
xmlDocPtr xml = NULL;
xmlXPathContextPtr ctxt = NULL;
virBuffer buf = VIR_BUFFER_INITIALIZER;
char *xpath = NULL;
int ret = -1;
int element;
char *mapfile;

if (!(mapfile = virFileFindResource("cpu_map.xml",
Expand All @@ -179,12 +200,6 @@ int cpuMapLoad(const char *arch,
goto cleanup;
}

if (cb == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("no callback provided"));
goto cleanup;
}

if (!(xml = virXMLParseFileCtxt(mapfile, &ctxt)))
goto cleanup;

Expand All @@ -202,15 +217,16 @@ int cpuMapLoad(const char *arch,
goto cleanup;
}

for (element = 0; element < CPU_MAP_ELEMENT_LAST; element++) {
if (load(ctxt, element, cb, data) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot parse CPU map '%s'"), mapfile);
goto cleanup;
}
}
if (loadData(mapfile, ctxt, "vendor", vendorCB, data) < 0)
goto cleanup;

if (loadData(mapfile, ctxt, "feature", featureCB, data) < 0)
goto cleanup;

if (loadData(mapfile, ctxt, "model", modelCB, data) < 0)
goto cleanup;

if (loadIncludes(ctxt, cb, data) < 0)
if (loadIncludes(ctxt, vendorCB, featureCB, modelCB, data) < 0)
goto cleanup;

ret = 0;
Expand Down
22 changes: 5 additions & 17 deletions src/cpu/cpu_map.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,28 +26,16 @@

# include "virxml.h"


typedef enum {
CPU_MAP_ELEMENT_VENDOR,
CPU_MAP_ELEMENT_FEATURE,
CPU_MAP_ELEMENT_MODEL,

CPU_MAP_ELEMENT_LAST
} cpuMapElement;

VIR_ENUM_DECL(cpuMapElement)


typedef int
(*cpuMapLoadCallback) (cpuMapElement element,
xmlXPathContextPtr ctxt,
xmlNodePtr *nodes,
int n,
(*cpuMapLoadCallback) (xmlXPathContextPtr ctxt,
const char *name,
void *data);

int
cpuMapLoad(const char *arch,
cpuMapLoadCallback cb,
cpuMapLoadCallback vendorCB,
cpuMapLoadCallback featureCB,
cpuMapLoadCallback modelCB,
void *data);

#endif /* __VIR_CPU_MAP_H__ */
112 changes: 24 additions & 88 deletions src/cpu/cpu_ppc64.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,79 +281,56 @@ ppc64MapFree(struct ppc64_map *map)
VIR_FREE(map);
}

static struct ppc64_vendor *
ppc64VendorParse(xmlXPathContextPtr ctxt,
struct ppc64_map *map)
static int
ppc64VendorParse(xmlXPathContextPtr ctxt ATTRIBUTE_UNUSED,
const char *name,
void *data)
{
struct ppc64_map *map = data;
struct ppc64_vendor *vendor;

if (VIR_ALLOC(vendor) < 0)
return NULL;
return -1;

vendor->name = virXPathString("string(@name)", ctxt);
if (!vendor->name) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Missing CPU vendor name"));
if (VIR_STRDUP(vendor->name, name) < 0)
goto error;
}

if (ppc64VendorFind(map, vendor->name)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("CPU vendor %s already defined"), vendor->name);
goto error;
}

return vendor;
if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0)
goto error;

return 0;

error:
ppc64VendorFree(vendor);
return NULL;
return -1;
}


static int
ppc64VendorsLoad(struct ppc64_map *map,
xmlXPathContextPtr ctxt,
xmlNodePtr *nodes,
int n)
{
struct ppc64_vendor *vendor;
size_t i;

if (VIR_ALLOC_N(map->vendors, n) < 0)
return -1;

for (i = 0; i < n; i++) {
ctxt->node = nodes[i];
if (!(vendor = ppc64VendorParse(ctxt, map)))
return -1;
map->vendors[map->nvendors++] = vendor;
}

return 0;
}


static struct ppc64_model *
ppc64ModelParse(xmlXPathContextPtr ctxt,
struct ppc64_map *map)
const char *name,
void *data)
{
struct ppc64_map *map = data;
struct ppc64_model *model;
xmlNodePtr *nodes = NULL;
char *vendor = NULL;
unsigned long pvr;
size_t i;
int n;
int ret = -1;

if (VIR_ALLOC(model) < 0)
goto error;

model->name = virXPathString("string(@name)", ctxt);
if (!model->name) {
virReportError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Missing CPU model name"));
if (VIR_STRDUP(model->name, name) < 0)
goto error;
}

if (ppc64ModelFind(map, model->name)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
Expand Down Expand Up @@ -410,63 +387,22 @@ ppc64ModelParse(xmlXPathContextPtr ctxt,
model->data.pvr[i].mask = pvr;
}

if (VIR_APPEND_ELEMENT(map->models, map->nmodels, model) < 0)
goto error;

ret = 0;

cleanup:
VIR_FREE(vendor);
VIR_FREE(nodes);
return model;
return ret;

error:
ppc64ModelFree(model);
model = NULL;
goto cleanup;
}


static int
ppc64ModelsLoad(struct ppc64_map *map,
xmlXPathContextPtr ctxt,
xmlNodePtr *nodes,
int n)
{
struct ppc64_model *model;
size_t i;

if (VIR_ALLOC_N(map->models, n) < 0)
return -1;

for (i = 0; i < n; i++) {
ctxt->node = nodes[i];
if (!(model = ppc64ModelParse(ctxt, map)))
return -1;
map->models[map->nmodels++] = model;
}

return 0;
}


static int
ppc64MapLoadCallback(cpuMapElement element,
xmlXPathContextPtr ctxt,
xmlNodePtr *nodes,
int n,
void *data)
{
struct ppc64_map *map = data;

switch (element) {
case CPU_MAP_ELEMENT_VENDOR:
return ppc64VendorsLoad(map, ctxt, nodes, n);
case CPU_MAP_ELEMENT_MODEL:
return ppc64ModelsLoad(map, ctxt, nodes, n);
case CPU_MAP_ELEMENT_FEATURE:
case CPU_MAP_ELEMENT_LAST:
break;
}

return 0;
}

static struct ppc64_map *
ppc64LoadMap(void)
{
Expand All @@ -475,7 +411,7 @@ ppc64LoadMap(void)
if (VIR_ALLOC(map) < 0)
goto error;

if (cpuMapLoad("ppc64", ppc64MapLoadCallback, map) < 0)
if (cpuMapLoad("ppc64", ppc64VendorParse, NULL, ppc64ModelParse, map) < 0)
goto error;

return map;
Expand Down
Loading

0 comments on commit 0815f51

Please sign in to comment.