Re: [libvirt] [PATCH 2/4] cpu: push more parsing logic into common code

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. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/cpu/cpu_map.c | 106 +++++++++++++++--------- src/cpu/cpu_map.h | 22 ++--- src/cpu/cpu_ppc64.c | 112 ++++++------------------- src/cpu/cpu_x86.c | 196 +++++++++++++------------------------------- 4 files changed, 155 insertions(+), 281 deletions(-) diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c index 9e090919ed..17ed53fda6 100644 --- a/src/cpu/cpu_map.c +++ b/src/cpu/cpu_map.c @@ -35,31 +35,51 @@ 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 *xpath, + 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) + n = virXPathNodeSet(xpath, ctxt, &nodes); + if (n < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find '%s' in CPU map '%s'"), xpath, mapfile); goto cleanup; + } - if (n > 0 && - callback(element, ctxt, nodes, n, data) < 0) + if (n > 0 && !callback) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected %s in CPU map '%s'"), xpath, mapfile);

goto cleanup; + } +

int cpuMapLoad(const char *arch, - cpuMapLoadCallback cb, + cpuMapLoadCallback vendorCB, + cpuMapLoadCallback featureCB, + cpuMapLoadCallback modelCB, void *data) { xmlDocPtr xml = NULL; @@ -157,7 +183,6 @@ int cpuMapLoad(const char *arch, virBuffer buf = VIR_BUFFER_INITIALIZER; char *xpath = NULL; int ret = -1; - int element; char *mapfile; if (!(mapfile = virFileFindResource("cpu_map.xml", @@ -173,9 +198,15 @@ int cpuMapLoad(const char *arch, goto cleanup; } - if (cb == NULL) { + if (vendorCB == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("no vendor callback provided")); + goto cleanup; + }

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

diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index d562677fa3..75da5b77d8 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -281,21 +281,19 @@ ppc64MapFree(struct ppc64_map *map) VIR_FREE(map); }

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, @@ -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; }

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 809da94117..76f1d417c1 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -712,22 +712,21 @@ x86VendorFind(virCPUx86MapPtr map, } -static virCPUx86VendorPtr +static int x86VendorParse(xmlXPathContextPtr ctxt, - virCPUx86MapPtr map) + const char *name, + void *data) { + virCPUx86MapPtr map = data; virCPUx86VendorPtr vendor = NULL; char *string = NULL; + int ret = -1; if (VIR_ALLOC(vendor) < 0) goto error; - 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 (x86VendorFind(map, vendor->name)) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -746,40 +745,21 @@ x86VendorParse(xmlXPathContextPtr ctxt, if (virCPUx86VendorToCPUID(string, &vendor->cpuid) < 0) goto error; + if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0) + goto error;

+ + ret = 0; + cleanup: VIR_FREE(string); - return vendor; + return ret; error: x86VendorFree(vendor); - vendor = NULL; goto cleanup; }

-static virCPUx86FeaturePtr +static int x86FeatureParse(xmlXPathContextPtr ctxt, - virCPUx86MapPtr map) + const char *name, + void *data) { + virCPUx86MapPtr map = data; xmlNodePtr *nodes = NULL; virCPUx86FeaturePtr feature; virCPUx86CPUID cpuid; size_t i; int n; char *str = NULL; + int ret = -1; if (!(feature = x86FeatureNew())) goto error; feature->migratable = true; - feature->name = virXPathString("string(@name)", ctxt); - if (!feature->name) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("Missing CPU feature name")); + + if (VIR_STRDUP(feature->name, name) < 0) goto error; - } if (x86FeatureFind(map, feature->name)) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -949,46 +929,28 @@ x86FeatureParse(xmlXPathContextPtr ctxt, goto error; } + if (!feature->migratable && + VIR_APPEND_ELEMENT_COPY(map->migrate_blockers, + map->nblockers, + feature) < 0) + goto error; + + if (VIR_APPEND_ELEMENT(map->features, map->nfeatures, feature) < 0) + goto error; +

+ ret = 0; + cleanup: VIR_FREE(nodes); VIR_FREE(str); - return feature; + return ret; error: x86FeatureFree(feature); - feature = NULL; goto cleanup; }

+static int x86ModelParse(xmlXPathContextPtr ctxt, - virCPUx86MapPtr map) + const char *name, + void *data) {

+ 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: x86ModelFree(model); - model = NULL; goto cleanup; }