Re: [libvirt] [PATCH v2 3/8] cpu: push more parsing logic into common code

2018-08-24 Thread Jiri Denemark
On Thu, Aug 16, 2018 at 13:10:26 +0100, Daniel P. Berrangé wrote:
> 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é 

Reviewed-by: Jiri Denemark 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 3/8] cpu: push more parsing logic into common code

2018-08-16 Thread Daniel P . Berrangé
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é 
---
 src/cpu/cpu_map.c   |  98 +-
 src/cpu/cpu_map.h   |  22 ++---
 src/cpu/cpu_ppc64.c | 112 ++---
 src/cpu/cpu_x86.c   | 196 +---
 4 files changed, 143 insertions(+), 285 deletions(-)

diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c
index bcd3e55417..400e6f1427 100644
--- a/src/cpu/cpu_map.c
+++ b/src/cpu/cpu_map.c
@@ -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, );
-if (n < 0)
+if ((n = virXPathNodeSet(element, ctxt, )) < 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;
 
@@ -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,
@@ -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;
 
@@ -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;
@@ -132,7 +152,7 @@ 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;
 }
@@ -150,7 +170,9 @@ loadIncludes(xmlXPathContextPtr ctxt,
 
 
 int cpuMapLoad(const char *arch,
-   cpuMapLoadCallback cb,
+   cpuMapLoadCallback vendorCB,
+   cpuMapLoadCallback featureCB,
+   cpuMapLoadCallback modelCB,
void *data)
 {
 xmlDocPtr xml = NULL;
@@ -158,7 +180,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",
@@ -174,12 +195,6 @@ int cpuMapLoad(const char *arch,
 goto cleanup;
 }
 
-if (cb == NULL) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
-   "%s", _("no callback