[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] Refactor storage XML parsing to be consistent with domain/network conf.



The storage driver arranges its parsing routines in a way that make them
difficult to use in the test driver for non-default file parsing. This
refactoring moves things to be consistent with the way domain_conf and
network_conf do things.

Signed-off-by: Cole Robinson <crobinso redhat com>
---
 src/libvirt_private.syms |    8 ++-
 src/storage_conf.c       |  163 ++++++++++++++++++++++++++++++++--------------
 src/storage_conf.h       |   26 ++++++--
 src/storage_driver.c     |    8 +-
 src/test.c               |   25 ++-----
 5 files changed, 150 insertions(+), 80 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f63fa05..309d7f9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -267,7 +267,9 @@ virSecurityDriverGetModel;
 # storage_conf.h
 virStoragePoolDefFormat;
 virStoragePoolDefFree;
-virStoragePoolDefParse;
+virStoragePoolDefParseString;
+virStoragePoolDefParseFile;
+virStoragePoolDefParseNode;
 virStoragePoolLoadAllConfigs;
 virStoragePoolObjAssignDef;
 virStoragePoolObjClearVols;
@@ -284,7 +286,9 @@ virStorageVolDefFindByName;
 virStorageVolDefFindByPath;
 virStorageVolDefFormat;
 virStorageVolDefFree;
-virStorageVolDefParse;
+virStorageVolDefParseFile;
+virStorageVolDefParseString;
+virStorageVolDefParseNode;
 virStoragePoolFormatDiskTypeToString;
 virStoragePoolFormatFileSystemTypeToString;
 virStoragePoolFormatFileSystemNetTypeToString;
diff --git a/src/storage_conf.c b/src/storage_conf.c
index 5f724dc..7d495ec 100644
--- a/src/storage_conf.c
+++ b/src/storage_conf.c
@@ -452,14 +452,12 @@ error:
     return ret;
 }
 
-
 static virStoragePoolDefPtr
-virStoragePoolDefParseDoc(virConnectPtr conn,
-                          xmlXPathContextPtr ctxt,
-                          xmlNodePtr root) {
+virStoragePoolDefParseXML(virConnectPtr conn,
+                          xmlXPathContextPtr ctxt) {
     virStoragePoolOptionsPtr options;
     virStoragePoolDefPtr ret;
-    xmlChar *type = NULL;
+    char *type = NULL;
     char *uuid = NULL;
     char *authType = NULL;
 
@@ -468,13 +466,7 @@ virStoragePoolDefParseDoc(virConnectPtr conn,
         return NULL;
     }
 
-    if (STRNEQ((const char *)root->name, "pool")) {
-        virStorageReportError(conn, VIR_ERR_XML_ERROR,
-                          "%s", _("unknown root element for storage pool"));
-        goto cleanup;
-    }
-
-    type = xmlGetProp(root, BAD_CAST "type");
+    type = virXPathString(conn, "string(./@type)", ctxt);
     if ((ret->type = virStoragePoolTypeFromString((const char *)type)) < 0) {
         virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
                               _("unknown storage pool type %s"), (const char*)type);
@@ -656,6 +648,32 @@ catchXMLError (void *ctx, const char *msg ATTRIBUTE_UNUSED, ...)
 }
 
 virStoragePoolDefPtr
+virStoragePoolDefParseNode(virConnectPtr conn,
+                           xmlDocPtr xml,
+                           xmlNodePtr root) {
+    xmlXPathContextPtr ctxt = NULL;
+    virStoragePoolDefPtr def = NULL;
+
+    if (STRNEQ((const char *)root->name, "pool")) {
+        virStorageReportError(conn, VIR_ERR_XML_ERROR,
+                          "%s", _("unknown root element for storage pool"));
+        goto cleanup;
+    }
+
+    ctxt = xmlXPathNewContext(xml);
+    if (ctxt == NULL) {
+        virReportOOMError(conn);
+        goto cleanup;
+    }
+
+    ctxt->node = root;
+    def = virStoragePoolDefParseXML(conn, ctxt);
+cleanup:
+    xmlXPathFreeContext(ctxt);
+    return def;
+}
+
+static virStoragePoolDefPtr
 virStoragePoolDefParse(virConnectPtr conn,
                        const char *xmlStr,
                        const char *filename) {
@@ -663,7 +681,6 @@ virStoragePoolDefParse(virConnectPtr conn,
     xmlParserCtxtPtr pctxt;
     xmlDocPtr xml = NULL;
     xmlNodePtr node = NULL;
-    xmlXPathContextPtr ctxt = NULL;
 
     /* Set up a parser context so we can catch the details of XML errors. */
     pctxt = xmlNewParserCtxt ();
@@ -673,10 +690,17 @@ virStoragePoolDefParse(virConnectPtr conn,
     pctxt->_private = conn;
 
     if (conn) virResetError (&conn->err);
-    xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr,
-                          filename ? filename : "storage.xml", NULL,
-                          XML_PARSE_NOENT | XML_PARSE_NONET |
-                          XML_PARSE_NOWARNING);
+    if (filename) {
+        xml = xmlCtxtReadFile (pctxt, filename, NULL,
+                               XML_PARSE_NOENT | XML_PARSE_NONET |
+                               XML_PARSE_NOWARNING);
+    } else {
+        xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr,
+                              "storage.xml", NULL,
+                              XML_PARSE_NOENT | XML_PARSE_NONET |
+                              XML_PARSE_NOWARNING);
+    }
+
     if (!xml) {
         if (conn && conn->err.code == VIR_ERR_NONE)
               virStorageReportError(conn, VIR_ERR_XML_ERROR,
@@ -684,12 +708,6 @@ virStoragePoolDefParse(virConnectPtr conn,
         goto cleanup;
     }
 
-    ctxt = xmlXPathNewContext(xml);
-    if (ctxt == NULL) {
-        virReportOOMError(conn);
-        goto cleanup;
-    }
-
     node = xmlDocGetRootElement(xml);
     if (node == NULL) {
         virStorageReportError(conn, VIR_ERR_XML_ERROR,
@@ -697,21 +715,33 @@ virStoragePoolDefParse(virConnectPtr conn,
         goto cleanup;
     }
 
-    ret = virStoragePoolDefParseDoc(conn, ctxt, node);
+    ret = virStoragePoolDefParseNode(conn, xml, node);
 
     xmlFreeParserCtxt (pctxt);
-    xmlXPathFreeContext(ctxt);
     xmlFreeDoc(xml);
 
     return ret;
 
  cleanup:
     xmlFreeParserCtxt (pctxt);
-    xmlXPathFreeContext(ctxt);
     xmlFreeDoc(xml);
     return NULL;
 }
 
+virStoragePoolDefPtr
+virStoragePoolDefParseString(virConnectPtr conn,
+                             const char *xmlStr)
+{
+    return virStoragePoolDefParse(conn, xmlStr, NULL);
+}
+
+virStoragePoolDefPtr
+virStoragePoolDefParseFile(virConnectPtr conn,
+                           const char *filename)
+{
+    return virStoragePoolDefParse(conn, NULL, filename);
+}
+
 static int
 virStoragePoolSourceFormat(virConnectPtr conn,
                            virBufferPtr buf,
@@ -916,10 +946,9 @@ virStorageSize(virConnectPtr conn,
 }
 
 static virStorageVolDefPtr
-virStorageVolDefParseDoc(virConnectPtr conn,
+virStorageVolDefParseXML(virConnectPtr conn,
                          virStoragePoolDefPtr pool,
-                         xmlXPathContextPtr ctxt,
-                         xmlNodePtr root) {
+                         xmlXPathContextPtr ctxt) {
     virStorageVolDefPtr ret;
     virStorageVolOptionsPtr options;
     char *allocation = NULL;
@@ -935,12 +964,6 @@ virStorageVolDefParseDoc(virConnectPtr conn,
         return NULL;
     }
 
-    if (STRNEQ((const char *)root->name, "volume")) {
-        virStorageReportError(conn, VIR_ERR_XML_ERROR,
-                              "%s", _("unknown root element"));
-        goto cleanup;
-    }
-
     ret->name = virXPathString(conn, "string(/volume/name)", ctxt);
     if (ret->name == NULL) {
         virStorageReportError(conn, VIR_ERR_XML_ERROR,
@@ -1028,8 +1051,34 @@ virStorageVolDefParseDoc(virConnectPtr conn,
     return NULL;
 }
 
-
 virStorageVolDefPtr
+virStorageVolDefParseNode(virConnectPtr conn,
+                          virStoragePoolDefPtr pool,
+                          xmlDocPtr xml,
+                          xmlNodePtr root) {
+    xmlXPathContextPtr ctxt = NULL;
+    virStorageVolDefPtr def = NULL;
+
+    if (STRNEQ((const char *)root->name, "volume")) {
+        virStorageReportError(conn, VIR_ERR_XML_ERROR,
+                          "%s", _("unknown root element for storage vol"));
+        goto cleanup;
+    }
+
+    ctxt = xmlXPathNewContext(xml);
+    if (ctxt == NULL) {
+        virReportOOMError(conn);
+        goto cleanup;
+    }
+
+    ctxt->node = root;
+    def = virStorageVolDefParseXML(conn, pool, ctxt);
+cleanup:
+    xmlXPathFreeContext(ctxt);
+    return def;
+}
+
+static virStorageVolDefPtr
 virStorageVolDefParse(virConnectPtr conn,
                       virStoragePoolDefPtr pool,
                       const char *xmlStr,
@@ -1038,7 +1087,6 @@ virStorageVolDefParse(virConnectPtr conn,
     xmlParserCtxtPtr pctxt;
     xmlDocPtr xml = NULL;
     xmlNodePtr node = NULL;
-    xmlXPathContextPtr ctxt = NULL;
 
     /* Set up a parser context so we can catch the details of XML errors. */
     pctxt = xmlNewParserCtxt ();
@@ -1048,10 +1096,18 @@ virStorageVolDefParse(virConnectPtr conn,
     pctxt->_private = conn;
 
     if (conn) virResetError (&conn->err);
-    xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr,
-                          filename ? filename : "storage.xml", NULL,
-                          XML_PARSE_NOENT | XML_PARSE_NONET |
-                          XML_PARSE_NOWARNING);
+
+    if (filename) {
+        xml = xmlCtxtReadFile (pctxt, filename, NULL,
+                               XML_PARSE_NOENT | XML_PARSE_NONET |
+                               XML_PARSE_NOWARNING);
+    } else {
+        xml = xmlCtxtReadDoc (pctxt, BAD_CAST xmlStr,
+                              "storage.xml", NULL,
+                              XML_PARSE_NOENT | XML_PARSE_NONET |
+                              XML_PARSE_NOWARNING);
+    }
+
     if (!xml) {
         if (conn && conn->err.code == VIR_ERR_NONE)
               virStorageReportError(conn, VIR_ERR_XML_ERROR,
@@ -1059,12 +1115,6 @@ virStorageVolDefParse(virConnectPtr conn,
         goto cleanup;
     }
 
-    ctxt = xmlXPathNewContext(xml);
-    if (ctxt == NULL) {
-        virReportOOMError(conn);
-        goto cleanup;
-    }
-
     node = xmlDocGetRootElement(xml);
     if (node == NULL) {
         virStorageReportError(conn, VIR_ERR_XML_ERROR,
@@ -1072,21 +1122,34 @@ virStorageVolDefParse(virConnectPtr conn,
         goto cleanup;
     }
 
-    ret = virStorageVolDefParseDoc(conn, pool, ctxt, node);
+    ret = virStorageVolDefParseNode(conn, pool, xml, node);
 
     xmlFreeParserCtxt (pctxt);
-    xmlXPathFreeContext(ctxt);
     xmlFreeDoc(xml);
 
     return ret;
 
  cleanup:
     xmlFreeParserCtxt (pctxt);
-    xmlXPathFreeContext(ctxt);
     xmlFreeDoc(xml);
     return NULL;
 }
 
+virStorageVolDefPtr
+virStorageVolDefParseString(virConnectPtr conn,
+                            virStoragePoolDefPtr pool,
+                            const char *xmlStr)
+{
+    return virStorageVolDefParse(conn, pool, xmlStr, NULL);
+}
+
+virStorageVolDefPtr
+virStorageVolDefParseFile(virConnectPtr conn,
+                          virStoragePoolDefPtr pool,
+                          const char *filename)
+{
+    return virStorageVolDefParse(conn, pool, NULL, filename);
+}
 
 static int
 virStorageVolTargetDefFormat(virConnectPtr conn,
diff --git a/src/storage_conf.h b/src/storage_conf.h
index 8a4fed2..a2a164e 100644
--- a/src/storage_conf.h
+++ b/src/storage_conf.h
@@ -28,6 +28,8 @@
 #include "util.h"
 #include "threads.h"
 
+#include <libxml/tree.h>
+
 /* Shared structs */
 
 
@@ -306,16 +308,26 @@ virStorageVolDefPtr virStorageVolDefFindByName(virStoragePoolObjPtr pool,
 
 void virStoragePoolObjClearVols(virStoragePoolObjPtr pool);
 
-virStoragePoolDefPtr virStoragePoolDefParse(virConnectPtr conn,
-                                            const char *xml,
-                                            const char *filename);
+virStoragePoolDefPtr virStoragePoolDefParseString(virConnectPtr conn,
+                                                  const char *xml);
+virStoragePoolDefPtr virStoragePoolDefParseFile(virConnectPtr conn,
+                                                const char *filename);
+virStoragePoolDefPtr virStoragePoolDefParseNode(virConnectPtr conn,
+                                                xmlDocPtr xml,
+                                                xmlNodePtr root);
 char *virStoragePoolDefFormat(virConnectPtr conn,
                               virStoragePoolDefPtr def);
 
-virStorageVolDefPtr virStorageVolDefParse(virConnectPtr conn,
-                                          virStoragePoolDefPtr pool,
-                                          const char *xml,
-                                          const char *filename);
+virStorageVolDefPtr virStorageVolDefParseString(virConnectPtr conn,
+                                                virStoragePoolDefPtr pool,
+                                                const char *xml);
+virStorageVolDefPtr virStorageVolDefParseFile(virConnectPtr conn,
+                                              virStoragePoolDefPtr pool,
+                                              const char *filename);
+virStorageVolDefPtr virStorageVolDefParseNode(virConnectPtr conn,
+                                              virStoragePoolDefPtr pool,
+                                              xmlDocPtr xml,
+                                              xmlNodePtr root);
 char *virStorageVolDefFormat(virConnectPtr conn,
                              virStoragePoolDefPtr pool,
                              virStorageVolDefPtr def);
diff --git a/src/storage_driver.c b/src/storage_driver.c
index 5a248a3..5fe22c6 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -466,7 +466,7 @@ storagePoolCreate(virConnectPtr conn,
     virStorageBackendPtr backend;
 
     storageDriverLock(driver);
-    if (!(def = virStoragePoolDefParse(conn, xml, NULL)))
+    if (!(def = virStoragePoolDefParseString(conn, xml)))
         goto cleanup;
 
     pool = virStoragePoolObjFindByUUID(&driver->pools, def->uuid);
@@ -518,7 +518,7 @@ storagePoolDefine(virConnectPtr conn,
     virStorageBackendPtr backend;
 
     storageDriverLock(driver);
-    if (!(def = virStoragePoolDefParse(conn, xml, NULL)))
+    if (!(def = virStoragePoolDefParseString(conn, xml)))
         goto cleanup;
 
     if ((backend = virStorageBackendForType(def->type)) == NULL)
@@ -1220,7 +1220,7 @@ storageVolumeCreateXML(virStoragePoolPtr obj,
     if ((backend = virStorageBackendForType(pool->def->type)) == NULL)
         goto cleanup;
 
-    voldef = virStorageVolDefParse(obj->conn, pool->def, xmldesc, NULL);
+    voldef = virStorageVolDefParseString(obj->conn, pool->def, xmldesc);
     if (voldef == NULL)
         goto cleanup;
 
@@ -1362,7 +1362,7 @@ storageVolumeCreateXMLFrom(virStoragePoolPtr obj,
         goto cleanup;
     }
 
-    newvol = virStorageVolDefParse(obj->conn, pool->def, xmldesc, NULL);
+    newvol = virStorageVolDefParseString(obj->conn, pool->def, xmldesc);
     if (newvol == NULL)
         goto cleanup;
 
diff --git a/src/test.c b/src/test.c
index 2a672a3..6874d63 100644
--- a/src/test.c
+++ b/src/test.c
@@ -286,7 +286,7 @@ static int testOpenDefault(virConnectPtr conn) {
     netobj->persistent = 1;
     virNetworkObjUnlock(netobj);
 
-    if (!(pooldef = virStoragePoolDefParse(conn, defaultPoolXML, NULL)))
+    if (!(pooldef = virStoragePoolDefParseString(conn, defaultPoolXML)))
         goto error;
 
     if (!(poolobj = virStoragePoolObjAssignDef(conn, &privconn->pools,
@@ -564,22 +564,13 @@ static int testOpenFromFile(virConnectPtr conn,
                 goto error;
             }
 
-            def = virStoragePoolDefParse(conn, NULL, absFile);
+            def = virStoragePoolDefParseFile(conn, absFile);
             VIR_FREE(absFile);
             if (!def)
                 goto error;
         } else {
-            xmlBufferPtr buf;
-            xmlSaveCtxtPtr sctxt;
-
-            buf = xmlBufferCreate();
-            sctxt = xmlSaveToBuffer(buf, NULL, 0);
-            xmlSaveTree(sctxt, pools[i]);
-            xmlSaveClose(sctxt);
-            if ((def = virStoragePoolDefParse(conn,
-                                              (const char *) buf->content,
-                                              NULL)) == NULL) {
-                xmlBufferFree(buf);
+            if ((def = virStoragePoolDefParseNode(conn, xml,
+                                                  pools[i])) == NULL) {
                 goto error;
             }
         }
@@ -2514,7 +2505,7 @@ testStoragePoolCreate(virConnectPtr conn,
     virStoragePoolPtr ret = NULL;
 
     testDriverLock(privconn);
-    if (!(def = virStoragePoolDefParse(conn, xml, NULL)))
+    if (!(def = virStoragePoolDefParseString(conn, xml)))
         goto cleanup;
 
     pool = virStoragePoolObjFindByUUID(&privconn->pools, def->uuid);
@@ -2557,7 +2548,7 @@ testStoragePoolDefine(virConnectPtr conn,
     virStoragePoolPtr ret = NULL;
 
     testDriverLock(privconn);
-    if (!(def = virStoragePoolDefParse(conn, xml, NULL)))
+    if (!(def = virStoragePoolDefParseString(conn, xml)))
         goto cleanup;
 
     def->capacity = defaultPoolCap;
@@ -3082,7 +3073,7 @@ testStorageVolumeCreateXML(virStoragePoolPtr pool,
         goto cleanup;
     }
 
-    privvol = virStorageVolDefParse(pool->conn, privpool->def, xmldesc, NULL);
+    privvol = virStorageVolDefParseString(pool->conn, privpool->def, xmldesc);
     if (privvol == NULL)
         goto cleanup;
 
@@ -3163,7 +3154,7 @@ testStorageVolumeCreateXMLFrom(virStoragePoolPtr pool,
         goto cleanup;
     }
 
-    privvol = virStorageVolDefParse(pool->conn, privpool->def, xmldesc, NULL);
+    privvol = virStorageVolDefParseString(pool->conn, privpool->def, xmldesc);
     if (privvol == NULL)
         goto cleanup;
 
-- 
1.6.3.2


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]