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

Daniel P. Berrangé berrange at redhat.com
Wed Aug 1 17:02:30 UTC 2018


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 at redhat.com>
---
 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;
+    }
+
+    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'"), xpath, mapfile);
+            goto cleanup;
+        }
+        VIR_DEBUG("Load %s name %s", xpath, name);
+        ctxt->node = nodes[i];
+        rv = callback(ctxt, name, data);
+        ctxt->node = old;
+        VIR_FREE(name);
+        if (rv < 0)
+            goto cleanup;
+    }
 
     ret = 0;
 
@@ -72,13 +92,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,
@@ -93,13 +114,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;
 
@@ -113,7 +135,9 @@ cpuMapLoadInclude(const char *filename,
 
 
 static int loadIncludes(xmlXPathContextPtr ctxt,
-                        cpuMapLoadCallback callback,
+                        cpuMapLoadCallback vendorCB,
+                        cpuMapLoadCallback featureCB,
+                        cpuMapLoadCallback modelCB,
                         void *data)
 {
     int ret = -1;
@@ -131,7 +155,7 @@ static int loadIncludes(xmlXPathContextPtr ctxt,
     for (i = 0; i < n; i++) {
         char *filename = virXMLPropString(nodes[i], "filename");
         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;
         }
@@ -149,7 +173,9 @@ static int loadIncludes(xmlXPathContextPtr ctxt,
 
 
 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;
     }
 
@@ -196,15 +227,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;
diff --git a/src/cpu/cpu_map.h b/src/cpu/cpu_map.h
index 0c7507e98f..4596987150 100644
--- a/src/cpu/cpu_map.h
+++ b/src/cpu/cpu_map.h
@@ -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__ */
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);
 }
 
-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,
@@ -303,57 +301,36 @@ ppc64VendorParse(xmlXPathContextPtr ctxt,
         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,
@@ -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)
 {
@@ -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;
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 int
-x86VendorsLoad(virCPUx86MapPtr map,
-               xmlXPathContextPtr ctxt,
-               xmlNodePtr *nodes,
-               int n)
-{
-    virCPUx86VendorPtr 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 = x86VendorParse(ctxt, map)))
-            return -1;
-        map->vendors[map->nvendors++] = vendor;
-    }
-
-    return 0;
-}
-
-
 static virCPUx86FeaturePtr
 x86FeatureNew(void)
 {
@@ -901,27 +881,27 @@ x86ParseCPUID(xmlXPathContextPtr ctxt,
 }
 
 
-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
-x86FeaturesLoad(virCPUx86MapPtr map,
-                xmlXPathContextPtr ctxt,
-                xmlNodePtr *nodes,
-                int n)
-{
-    virCPUx86FeaturePtr feature;
-    size_t i;
-
-    if (VIR_ALLOC_N(map->features, n) < 0)
-        return -1;
-
-    for (i = 0; i < n; i++) {
-        ctxt->node = nodes[i];
-        if (!(feature = x86FeatureParse(ctxt, map)))
-            return -1;
-        map->features[map->nfeatures++] = feature;
-        if (!feature->migratable &&
-            VIR_APPEND_ELEMENT(map->migrate_blockers,
-                               map->nblockers,
-                               feature) < 0)
-            return -1;
-    }
-
-    return 0;
-}
-
-
 static virCPUx86ModelPtr
 x86ModelNew(void)
 {
@@ -1184,47 +1146,46 @@ x86ModelCompare(virCPUx86ModelPtr model1,
 }
 
 
-static virCPUx86ModelPtr
+static int
 x86ModelParse(xmlXPathContextPtr ctxt,
-              virCPUx86MapPtr map)
+              const char *name,
+              void *data)
 {
+    virCPUx86MapPtr map = data;
     xmlNodePtr *nodes = NULL;
     virCPUx86ModelPtr model;
     char *vendor = NULL;
     size_t i;
     int n;
+    int ret = -1;
 
     if (!(model = x86ModelNew()))
         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 (virXPathNode("./model", ctxt)) {
         virCPUx86ModelPtr ancestor;
-        char *name;
+        char *anname;
 
-        name = virXPathString("string(./model/@name)", ctxt);
-        if (!name) {
+        anname = virXPathString("string(./model/@name)", ctxt);
+        if (!anname) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Missing ancestor's name in CPU model %s"),
                            model->name);
             goto error;
         }
 
-        if (!(ancestor = x86ModelFind(map, name))) {
+        if (!(ancestor = x86ModelFind(map, anname))) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Ancestor model %s not found for CPU model %s"),
-                           name, model->name);
-            VIR_FREE(name);
+                           anname, model->name);
+            VIR_FREE(anname);
             goto error;
         }
 
-        VIR_FREE(name);
+        VIR_FREE(anname);
 
         model->vendor = ancestor->vendor;
         model->signature = ancestor->signature;
@@ -1279,62 +1240,43 @@ x86ModelParse(xmlXPathContextPtr ctxt,
 
     for (i = 0; i < n; i++) {
         virCPUx86FeaturePtr feature;
-        char *name;
+        char *ftname;
 
-        if (!(name = virXMLPropString(nodes[i], "name"))) {
+        if (!(ftname = virXMLPropString(nodes[i], "name"))) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Missing feature name for CPU model %s"), model->name);
             goto error;
         }
 
-        if (!(feature = x86FeatureFind(map, name))) {
+        if (!(feature = x86FeatureFind(map, ftname))) {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Feature %s required by CPU model %s not found"),
-                           name, model->name);
-            VIR_FREE(name);
+                           ftname, model->name);
+            VIR_FREE(ftname);
             goto error;
         }
-        VIR_FREE(name);
+        VIR_FREE(ftname);
 
         if (x86DataAdd(&model->data, &feature->data))
             goto error;
     }
 
+    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;
 }
 
 
-static int
-x86ModelsLoad(virCPUx86MapPtr map,
-              xmlXPathContextPtr ctxt,
-              xmlNodePtr *nodes,
-              int n)
-{
-    virCPUx86ModelPtr 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 = x86ModelParse(ctxt, map)))
-            return -1;
-        map->models[map->nmodels++] = model;
-    }
-
-    return 0;
-}
-
-
 static void
 x86MapFree(virCPUx86MapPtr map)
 {
@@ -1364,30 +1306,6 @@ x86MapFree(virCPUx86MapPtr map)
 }
 
 
-static int
-x86MapLoadCallback(cpuMapElement element,
-                   xmlXPathContextPtr ctxt,
-                   xmlNodePtr *nodes,
-                   int n,
-                   void *data)
-{
-    virCPUx86MapPtr map = data;
-
-    switch (element) {
-    case CPU_MAP_ELEMENT_VENDOR:
-        return x86VendorsLoad(map, ctxt, nodes, n);
-    case CPU_MAP_ELEMENT_FEATURE:
-        return x86FeaturesLoad(map, ctxt, nodes, n);
-    case CPU_MAP_ELEMENT_MODEL:
-        return x86ModelsLoad(map, ctxt, nodes, n);
-    case CPU_MAP_ELEMENT_LAST:
-        break;
-    }
-
-    return 0;
-}
-
-
 static virCPUx86MapPtr
 virCPUx86LoadMap(void)
 {
@@ -1396,7 +1314,7 @@ virCPUx86LoadMap(void)
     if (VIR_ALLOC(map) < 0)
         return NULL;
 
-    if (cpuMapLoad("x86", x86MapLoadCallback, map) < 0)
+    if (cpuMapLoad("x86", x86VendorParse, x86FeatureParse, x86ModelParse, map) < 0)
         goto error;
 
     return map;
-- 
2.17.1




More information about the libvir-list mailing list