[libvirt] [PATCH 01/10] test: Split object parsing into their own functions

Cole Robinson crobinso at redhat.com
Wed Aug 7 23:28:55 UTC 2013


The function that parses custom driver XML was getting pretty unruly,
split the object parsing into their own functions. Rename some variables
to be consistent across each function. This should be functionally
identical.
---
 src/test/test_driver.c | 463 +++++++++++++++++++++++++++++--------------------
 1 file changed, 276 insertions(+), 187 deletions(-)

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index f7eaf06..718f83c 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -689,131 +689,13 @@ static char *testBuildFilename(const char *relativeTo,
     }
 }
 
-static int testOpenVolumesForPool(xmlDocPtr xml,
-                                  xmlXPathContextPtr ctxt,
-                                  const char *file,
-                                  virStoragePoolObjPtr pool,
-                                  int poolidx) {
-    char *vol_xpath;
-    size_t i;
-    int ret, func_ret = -1;
-    xmlNodePtr *vols = NULL;
-    virStorageVolDefPtr def = NULL;
-
-    /* Find storage volumes */
-    if (virAsprintf(&vol_xpath, "/node/pool[%d]/volume", poolidx) < 0)
-        goto error;
-
-    ret = virXPathNodeSet(vol_xpath, ctxt, &vols);
-    VIR_FREE(vol_xpath);
-    if (ret < 0) {
-        goto error;
-    }
-
-    for (i = 0; i < ret; i++) {
-        char *relFile = virXMLPropString(vols[i], "file");
-        if (relFile != NULL) {
-            char *absFile = testBuildFilename(file, relFile);
-            VIR_FREE(relFile);
-            if (!absFile) {
-                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                               _("resolving volume filename"));
-                goto error;
-            }
-
-            def = virStorageVolDefParseFile(pool->def, absFile);
-            VIR_FREE(absFile);
-            if (!def)
-                goto error;
-        } else {
-            if ((def = virStorageVolDefParseNode(pool->def, xml,
-                                                 vols[i])) == NULL) {
-                goto error;
-            }
-        }
-
-        if (VIR_REALLOC_N(pool->volumes.objs,
-                          pool->volumes.count+1) < 0)
-            goto error;
-
-        if (def->target.path == NULL) {
-            if (virAsprintf(&def->target.path, "%s/%s",
-                            pool->def->target.path,
-                            def->name) == -1)
-                goto error;
-        }
-
-        if (!def->key && VIR_STRDUP(def->key, def->target.path) < 0)
-            goto error;
-
-        pool->def->allocation += def->allocation;
-        pool->def->available = (pool->def->capacity -
-                                pool->def->allocation);
-
-        pool->volumes.objs[pool->volumes.count++] = def;
-        def = NULL;
-    }
-
-    func_ret = 0;
-error:
-    virStorageVolDefFree(def);
-    VIR_FREE(vols);
-    return func_ret;
-}
-
-static int testOpenFromFile(virConnectPtr conn,
-                            const char *file) {
-    size_t i;
-    int ret;
-    long l;
+static int
+testParseNodeInfo(virNodeInfoPtr nodeInfo, xmlXPathContextPtr ctxt)
+{
     char *str;
-    xmlDocPtr xml = NULL;
-    xmlNodePtr *domains = NULL, *networks = NULL, *ifaces = NULL,
-               *pools = NULL, *devs = NULL;
-    xmlXPathContextPtr ctxt = NULL;
-    virNodeInfoPtr nodeInfo;
-    virNetworkObjPtr net;
-    virInterfaceObjPtr iface;
-    virDomainObjPtr dom;
-    testConnPtr privconn;
-    if (VIR_ALLOC(privconn) < 0)
-        return VIR_DRV_OPEN_ERROR;
-    if (virMutexInit(&privconn->lock) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       "%s", _("cannot initialize mutex"));
-        VIR_FREE(privconn);
-        return VIR_DRV_OPEN_ERROR;
-    }
-
-    testDriverLock(privconn);
-    conn->privateData = privconn;
-
-    if (!(privconn->domains = virDomainObjListNew()))
-        goto error;
-
-    if (!(privconn->caps = testBuildCapabilities(conn)))
-        goto error;
-
-    if (!(privconn->xmlopt = testBuildXMLConfig()))
-        goto error;
-
-    if (!(xml = virXMLParseFileCtxt(file, &ctxt))) {
-        goto error;
-    }
-
-    if (!xmlStrEqual(ctxt->node->name, BAD_CAST "node")) {
-        virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("Root element is not 'node'"));
-        goto error;
-    }
-
-    privconn->nextDomID = 1;
-    privconn->numCells = 0;
-    if (VIR_STRDUP(privconn->path, file) < 0)
-        goto error;
-    memmove(&privconn->nodeInfo, &defaultNodeInfo, sizeof(defaultNodeInfo));
+    long l;
+    int ret;
 
-    nodeInfo = &privconn->nodeInfo;
     ret = virXPathLong("string(/node/cpu/nodes[1])", ctxt, &l);
     if (ret == 0) {
         nodeInfo->nodes = l;
@@ -850,7 +732,8 @@ static int testOpenFromFile(virConnectPtr conn,
         goto error;
     }
 
-    nodeInfo->cpus = nodeInfo->cores * nodeInfo->threads * nodeInfo->sockets * nodeInfo->nodes;
+    nodeInfo->cpus = (nodeInfo->cores * nodeInfo->threads *
+                      nodeInfo->sockets * nodeInfo->nodes);
     ret = virXPathLong("string(/node/cpu/active[1])", ctxt, &l);
     if (ret == 0) {
         if (l < nodeInfo->cpus) {
@@ -890,14 +773,29 @@ static int testOpenFromFile(virConnectPtr conn,
         goto error;
     }
 
-    ret = virXPathNodeSet("/node/domain", ctxt, &domains);
-    if (ret < 0) {
+    return 0;
+error:
+    return -1;
+}
+
+static int
+testParseDomains(virConnectPtr conn,
+                 testConnPtr privconn, const char *file,
+                 xmlDocPtr doc, xmlXPathContextPtr ctxt)
+{
+    int num, ret = -1;
+    size_t i;
+    xmlNodePtr *nodes = NULL;
+    virDomainObjPtr obj;
+
+    num = virXPathNodeSet("/node/domain", ctxt, &nodes);
+    if (num < 0) {
         goto error;
     }
 
-    for (i = 0; i < ret; i++) {
+    for (i = 0; i < num; i++) {
         virDomainDefPtr def;
-        char *relFile = virXMLPropString(domains[i], "file");
+        char *relFile = virXMLPropString(nodes[i], "file");
         if (relFile != NULL) {
             char *absFile = testBuildFilename(file, relFile);
             VIR_FREE(relFile);
@@ -914,7 +812,7 @@ static int testOpenFromFile(virConnectPtr conn,
             if (!def)
                 goto error;
         } else {
-            if ((def = virDomainDefParseNode(xml, domains[i],
+            if ((def = virDomainDefParseNode(doc, nodes[i],
                                              privconn->caps, privconn->xmlopt,
                                              1 << VIR_DOMAIN_VIRT_TEST,
                                              VIR_DOMAIN_XML_INACTIVE)) == NULL)
@@ -922,7 +820,7 @@ static int testOpenFromFile(virConnectPtr conn,
         }
 
         if (testDomainGenerateIfnames(def) < 0 ||
-            !(dom = virDomainObjListAdd(privconn->domains,
+            !(obj = virDomainObjListAdd(privconn->domains,
                                         def,
                                         privconn->xmlopt,
                                         0, NULL))) {
@@ -930,23 +828,38 @@ static int testOpenFromFile(virConnectPtr conn,
             goto error;
         }
 
-        dom->persistent = 1;
-        if (testDomainStartState(conn, dom, VIR_DOMAIN_RUNNING_BOOTED) < 0) {
-            virObjectUnlock(dom);
+        obj->persistent = 1;
+        if (testDomainStartState(conn, obj, VIR_DOMAIN_RUNNING_BOOTED) < 0) {
+            virObjectUnlock(obj);
             goto error;
         }
 
-        virObjectUnlock(dom);
+        virObjectUnlock(obj);
     }
-    VIR_FREE(domains);
 
-    ret = virXPathNodeSet("/node/network", ctxt, &networks);
-    if (ret < 0) {
+    ret = 0;
+error:
+    VIR_FREE(nodes);
+    return ret;
+}
+
+static int
+testParseNetworks(testConnPtr privconn, const char *file,
+                  xmlDocPtr doc, xmlXPathContextPtr ctxt)
+{
+    int num, ret = -1;
+    size_t i;
+    xmlNodePtr *nodes = NULL;
+    virNetworkObjPtr obj;
+
+    num = virXPathNodeSet("/node/network", ctxt, &nodes);
+    if (num < 0) {
         goto error;
     }
-    for (i = 0; i < ret; i++) {
+
+    for (i = 0; i < num; i++) {
         virNetworkDefPtr def;
-        char *relFile = virXMLPropString(networks[i], "file");
+        char *relFile = virXMLPropString(nodes[i], "file");
         if (relFile != NULL) {
             char *absFile = testBuildFilename(file, relFile);
             VIR_FREE(relFile);
@@ -961,27 +874,43 @@ static int testOpenFromFile(virConnectPtr conn,
             if (!def)
                 goto error;
         } else {
-            if ((def = virNetworkDefParseNode(xml, networks[i])) == NULL)
+            if ((def = virNetworkDefParseNode(doc, nodes[i])) == NULL)
                 goto error;
         }
-        if (!(net = virNetworkAssignDef(&privconn->networks, def, false))) {
+
+        if (!(obj = virNetworkAssignDef(&privconn->networks, def, false))) {
             virNetworkDefFree(def);
             goto error;
         }
-        net->persistent = 1;
-        net->active = 1;
-        virNetworkObjUnlock(net);
+
+        obj->persistent = 1;
+        obj->active = 1;
+        virNetworkObjUnlock(obj);
     }
-    VIR_FREE(networks);
 
-    /* Parse interface definitions */
-    ret = virXPathNodeSet("/node/interface", ctxt, &ifaces);
-    if (ret < 0) {
+    ret = 0;
+error:
+    VIR_FREE(nodes);
+    return ret;
+}
+
+static int
+testParseInterfaces(testConnPtr privconn, const char *file,
+                    xmlDocPtr doc, xmlXPathContextPtr ctxt)
+{
+    int num, ret = -1;
+    size_t i;
+    xmlNodePtr *nodes = NULL;
+    virInterfaceObjPtr obj;
+
+    num = virXPathNodeSet("/node/interface", ctxt, &nodes);
+    if (num < 0) {
         goto error;
     }
-    for (i = 0; i < ret; i++) {
+
+    for (i = 0; i < num; i++) {
         virInterfaceDefPtr def;
-        char *relFile = virXMLPropString(ifaces[i], "file");
+        char *relFile = virXMLPropString(nodes[i], "file");
         if (relFile != NULL) {
             char *absFile = testBuildFilename(file, relFile);
             VIR_FREE(relFile);
@@ -996,29 +925,116 @@ static int testOpenFromFile(virConnectPtr conn,
             if (!def)
                 goto error;
         } else {
-            if ((def = virInterfaceDefParseNode(xml, ifaces[i])) == NULL)
+            if ((def = virInterfaceDefParseNode(doc, nodes[i])) == NULL)
                 goto error;
         }
 
-        if (!(iface = virInterfaceAssignDef(&privconn->ifaces, def))) {
+        if (!(obj = virInterfaceAssignDef(&privconn->ifaces, def))) {
             virInterfaceDefFree(def);
             goto error;
         }
 
-        iface->active = 1;
-        virInterfaceObjUnlock(iface);
+        obj->active = 1;
+        virInterfaceObjUnlock(obj);
+    }
+
+    ret = 0;
+error:
+    VIR_FREE(nodes);
+    return ret;
+}
+
+static int
+testOpenVolumesForPool(xmlDocPtr xml,
+                       xmlXPathContextPtr ctxt,
+                       const char *file,
+                       virStoragePoolObjPtr pool,
+                       int poolidx)
+{
+    char *vol_xpath;
+    size_t i;
+    int num, ret = -1;
+    xmlNodePtr *nodes = NULL;
+    virStorageVolDefPtr def = NULL;
+
+    /* Find storage volumes */
+    if (virAsprintf(&vol_xpath, "/node/pool[%d]/volume", poolidx) < 0)
+        goto error;
+
+    num = virXPathNodeSet(vol_xpath, ctxt, &nodes);
+    VIR_FREE(vol_xpath);
+    if (num < 0) {
+        goto error;
     }
-    VIR_FREE(ifaces);
 
-    /* Parse Storage Pool list */
-    ret = virXPathNodeSet("/node/pool", ctxt, &pools);
-    if (ret < 0) {
+    for (i = 0; i < num; i++) {
+        char *relFile = virXMLPropString(nodes[i], "file");
+        if (relFile != NULL) {
+            char *absFile = testBuildFilename(file, relFile);
+            VIR_FREE(relFile);
+            if (!absFile) {
+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                               _("resolving volume filename"));
+                goto error;
+            }
+
+            def = virStorageVolDefParseFile(pool->def, absFile);
+            VIR_FREE(absFile);
+            if (!def)
+                goto error;
+        } else {
+            if ((def = virStorageVolDefParseNode(pool->def, xml,
+                                                 nodes[i])) == NULL) {
+                goto error;
+            }
+        }
+
+        if (VIR_REALLOC_N(pool->volumes.objs,
+                          pool->volumes.count+1) < 0)
+            goto error;
+
+        if (def->target.path == NULL) {
+            if (virAsprintf(&def->target.path, "%s/%s",
+                            pool->def->target.path,
+                            def->name) == -1)
+                goto error;
+        }
+
+        if (!def->key && VIR_STRDUP(def->key, def->target.path) < 0)
+            goto error;
+
+        pool->def->allocation += def->allocation;
+        pool->def->available = (pool->def->capacity -
+                                pool->def->allocation);
+
+        pool->volumes.objs[pool->volumes.count++] = def;
+        def = NULL;
+    }
+
+    ret = 0;
+error:
+    virStorageVolDefFree(def);
+    VIR_FREE(nodes);
+    return ret;
+}
+
+static int
+testParseStorage(testConnPtr privconn, const char *file,
+                 xmlDocPtr doc, xmlXPathContextPtr ctxt)
+{
+    int num, ret = -1;
+    size_t i;
+    xmlNodePtr *nodes = NULL;
+    virStoragePoolObjPtr obj;
+
+    num = virXPathNodeSet("/node/pool", ctxt, &nodes);
+    if (num < 0) {
         goto error;
     }
-    for (i = 0; i < ret; i++) {
+
+    for (i = 0; i < num; i++) {
         virStoragePoolDefPtr def;
-        virStoragePoolObjPtr pool;
-        char *relFile = virXMLPropString(pools[i], "file");
+        char *relFile = virXMLPropString(nodes[i], "file");
         if (relFile != NULL) {
             char *absFile = testBuildFilename(file, relFile);
             VIR_FREE(relFile);
@@ -1033,42 +1049,56 @@ static int testOpenFromFile(virConnectPtr conn,
             if (!def)
                 goto error;
         } else {
-            if ((def = virStoragePoolDefParseNode(xml,
-                                                  pools[i])) == NULL) {
+            if ((def = virStoragePoolDefParseNode(doc,
+                                                  nodes[i])) == NULL) {
                 goto error;
             }
         }
 
-        if (!(pool = virStoragePoolObjAssignDef(&privconn->pools,
+        if (!(obj = virStoragePoolObjAssignDef(&privconn->pools,
                                                 def))) {
             virStoragePoolDefFree(def);
             goto error;
         }
 
-        if (testStoragePoolObjSetDefaults(pool) == -1) {
-            virStoragePoolObjUnlock(pool);
+        if (testStoragePoolObjSetDefaults(obj) == -1) {
+            virStoragePoolObjUnlock(obj);
             goto error;
         }
-        pool->active = 1;
+        obj->active = 1;
 
         /* Find storage volumes */
-        if (testOpenVolumesForPool(xml, ctxt, file, pool, i+1) < 0) {
-            virStoragePoolObjUnlock(pool);
+        if (testOpenVolumesForPool(doc, ctxt, file, obj, i+1) < 0) {
+            virStoragePoolObjUnlock(obj);
             goto error;
         }
 
-        virStoragePoolObjUnlock(pool);
+        virStoragePoolObjUnlock(obj);
     }
-    VIR_FREE(pools);
 
-    ret = virXPathNodeSet("/node/device", ctxt, &devs);
-    if (ret < 0) {
+    ret = 0;
+error:
+    VIR_FREE(nodes);
+    return ret;
+}
+
+static int
+testParseNodedevs(testConnPtr privconn, const char *file,
+                  xmlDocPtr doc, xmlXPathContextPtr ctxt)
+{
+    int num, ret = -1;
+    size_t i;
+    xmlNodePtr *nodes = NULL;
+    virNodeDeviceObjPtr obj;
+
+    num = virXPathNodeSet("/node/device", ctxt, &nodes);
+    if (num < 0) {
         goto error;
     }
-    for (i = 0; i < ret; i++) {
+
+    for (i = 0; i < num; i++) {
         virNodeDeviceDefPtr def;
-        virNodeDeviceObjPtr dev;
-        char *relFile = virXMLPropString(devs[i], "file");
+        char *relFile = virXMLPropString(nodes[i], "file");
 
         if (relFile != NULL) {
             char *absFile = testBuildFilename(file, relFile);
@@ -1085,32 +1115,91 @@ static int testOpenFromFile(virConnectPtr conn,
             if (!def)
                 goto error;
         } else {
-            if ((def = virNodeDeviceDefParseNode(xml, devs[i], 0, NULL)) == NULL)
+            if ((def = virNodeDeviceDefParseNode(doc,
+                                                 nodes[i], 0, NULL)) == NULL)
                 goto error;
         }
-        if (!(dev = virNodeDeviceAssignDef(&privconn->devs, def))) {
+
+        if (!(obj = virNodeDeviceAssignDef(&privconn->devs, def))) {
             virNodeDeviceDefFree(def);
             goto error;
         }
-        virNodeDeviceObjUnlock(dev);
+
+        virNodeDeviceObjUnlock(obj);
     }
-    VIR_FREE(devs);
 
+    ret = 0;
+error:
+    VIR_FREE(nodes);
+    return ret;
+}
+
+static int
+testOpenFromFile(virConnectPtr conn, const char *file)
+{
+    xmlDocPtr doc = NULL;
+    xmlXPathContextPtr ctxt = NULL;
+    testConnPtr privconn;
+
+    if (VIR_ALLOC(privconn) < 0)
+        return VIR_DRV_OPEN_ERROR;
+    if (virMutexInit(&privconn->lock) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       "%s", _("cannot initialize mutex"));
+        VIR_FREE(privconn);
+        return VIR_DRV_OPEN_ERROR;
+    }
+
+    testDriverLock(privconn);
+    conn->privateData = privconn;
+
+    if (!(privconn->domains = virDomainObjListNew()))
+        goto error;
+
+    if (!(privconn->caps = testBuildCapabilities(conn)))
+        goto error;
+
+    if (!(privconn->xmlopt = testBuildXMLConfig()))
+        goto error;
+
+    if (!(doc = virXMLParseFileCtxt(file, &ctxt))) {
+        goto error;
+    }
+
+    if (!xmlStrEqual(ctxt->node->name, BAD_CAST "node")) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("Root element is not 'node'"));
+        goto error;
+    }
+
+    privconn->nextDomID = 1;
+    privconn->numCells = 0;
+    if (VIR_STRDUP(privconn->path, file) < 0)
+        goto error;
+    memmove(&privconn->nodeInfo, &defaultNodeInfo, sizeof(defaultNodeInfo));
+
+    if (testParseNodeInfo(&privconn->nodeInfo, ctxt) < 0)
+        goto error;
+    if (testParseDomains(conn, privconn, file, doc, ctxt) < 0)
+        goto error;
+    if (testParseNetworks(privconn, file, doc, ctxt) < 0)
+        goto error;
+    if (testParseInterfaces(privconn, file, doc, ctxt) < 0)
+        goto error;
+    if (testParseStorage(privconn, file, doc, ctxt) < 0)
+        goto error;
+    if (testParseNodedevs(privconn, file, doc, ctxt) < 0)
+        goto error;
 
     xmlXPathFreeContext(ctxt);
-    xmlFreeDoc(xml);
+    xmlFreeDoc(doc);
     testDriverUnlock(privconn);
 
     return 0;
 
  error:
     xmlXPathFreeContext(ctxt);
-    xmlFreeDoc(xml);
-    VIR_FREE(domains);
-    VIR_FREE(networks);
-    VIR_FREE(ifaces);
-    VIR_FREE(pools);
-    VIR_FREE(devs);
+    xmlFreeDoc(doc);
     virObjectUnref(privconn->domains);
     virNetworkObjListFree(&privconn->networks);
     virInterfaceObjListFree(&privconn->ifaces);
-- 
1.8.3.1




More information about the libvir-list mailing list