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

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



On 06/19/2009 01:09 PM, Daniel P. Berrange wrote:
> On Fri, Jun 19, 2009 at 12:37:11PM -0400, Cole Robinson wrote:
>> 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(-)
> 
> ACK, this was on my todo list too.
> 
> Daniel

Hmm, this isn't quite complete. The above doesn't work for inlined pool/volume
definitions for a custom test driver (reading a separate file, like the
example in the docs section, works fine). Reason being that the xpaths in the
parsing routines are all absolute, and not relative to the root node. The
following additive patch solves that issue (this is what domain and network
parsers do as well).

Thanks,
Cole
diff --git a/src/storage_conf.c b/src/storage_conf.c
index 7d495ec..63bc6df 100644
--- a/src/storage_conf.c
+++ b/src/storage_conf.c
@@ -363,14 +363,14 @@ static int
 virStoragePoolDefParseAuthChap(virConnectPtr conn,
                                xmlXPathContextPtr ctxt,
                                virStoragePoolAuthChapPtr auth) {
-    auth->login = virXPathString(conn, "string(/pool/source/auth/@login)", ctxt);
+    auth->login = virXPathString(conn, "string(./source/auth/@login)", ctxt);
     if (auth->login == NULL) {
         virStorageReportError(conn, VIR_ERR_XML_ERROR,
                               "%s", _("missing auth host attribute"));
         return -1;
     }
 
-    auth->passwd = virXPathString(conn, "string(/pool/source/auth/@passwd)", ctxt);
+    auth->passwd = virXPathString(conn, "string(./source/auth/@passwd)", ctxt);
     if (auth->passwd == NULL) {
         virStorageReportError(conn, VIR_ERR_XML_ERROR,
                               "%s", _("missing auth passwd attribute"));
@@ -480,17 +480,17 @@ virStoragePoolDefParseXML(virConnectPtr conn,
         goto cleanup;
     }
 
-    ret->name = virXPathString(conn, "string(/pool/name)", ctxt);
+    ret->name = virXPathString(conn, "string(./name)", ctxt);
     if (ret->name == NULL &&
         options->flags & VIR_STORAGE_POOL_SOURCE_NAME)
-        ret->name = virXPathString(conn, "string(/pool/source/name)", ctxt);
+        ret->name = virXPathString(conn, "string(./source/name)", ctxt);
     if (ret->name == NULL) {
         virStorageReportError(conn, VIR_ERR_XML_ERROR,
                               "%s", _("missing pool source name element"));
         goto cleanup;
     }
 
-    uuid = virXPathString(conn, "string(/pool/uuid)", ctxt);
+    uuid = virXPathString(conn, "string(./uuid)", ctxt);
     if (uuid == NULL) {
         if (virUUIDGenerate(ret->uuid) < 0) {
             virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
@@ -507,7 +507,7 @@ virStoragePoolDefParseXML(virConnectPtr conn,
     }
 
     if (options->formatFromString) {
-        char *format = virXPathString(conn, "string(/pool/source/format/@type)", ctxt);
+        char *format = virXPathString(conn, "string(./source/format/@type)", ctxt);
         if (format == NULL)
             ret->source.format = options->defaultFormat;
         else
@@ -523,7 +523,7 @@ virStoragePoolDefParseXML(virConnectPtr conn,
     }
 
     if (options->flags & VIR_STORAGE_POOL_SOURCE_HOST) {
-        if ((ret->source.host.name = virXPathString(conn, "string(/pool/source/host/@name)", ctxt)) == NULL) {
+        if ((ret->source.host.name = virXPathString(conn, "string(./source/host/@name)", ctxt)) == NULL) {
             virStorageReportError(conn, VIR_ERR_XML_ERROR,
                              "%s", _("missing storage pool source host name"));
             goto cleanup;
@@ -533,7 +533,7 @@ virStoragePoolDefParseXML(virConnectPtr conn,
         xmlNodePtr *nodeset = NULL;
         int nsource, i;
 
-        if ((nsource = virXPathNodeSet(conn, "/pool/source/device", ctxt, &nodeset)) < 0) {
+        if ((nsource = virXPathNodeSet(conn, "./source/device", ctxt, &nodeset)) < 0) {
             virStorageReportError(conn, VIR_ERR_XML_ERROR,
                         "%s", _("cannot extract storage pool source devices"));
             goto cleanup;
@@ -557,14 +557,14 @@ virStoragePoolDefParseXML(virConnectPtr conn,
         ret->source.ndevice = nsource;
     }
     if (options->flags & VIR_STORAGE_POOL_SOURCE_DIR) {
-        if ((ret->source.dir = virXPathString(conn, "string(/pool/source/dir/@path)", ctxt)) == NULL) {
+        if ((ret->source.dir = virXPathString(conn, "string(./source/dir/@path)", ctxt)) == NULL) {
             virStorageReportError(conn, VIR_ERR_XML_ERROR,
                                 "%s", _("missing storage pool source path"));
             goto cleanup;
         }
     }
     if (options->flags & VIR_STORAGE_POOL_SOURCE_NAME) {
-        ret->source.name = virXPathString(conn, "string(/pool/source/name)",
+        ret->source.name = virXPathString(conn, "string(./source/name)",
                                           ctxt);
         if (ret->source.name == NULL) {
             /* source name defaults to pool name */
@@ -578,7 +578,7 @@ virStoragePoolDefParseXML(virConnectPtr conn,
 
     if (options->flags & VIR_STORAGE_POOL_SOURCE_ADAPTER) {
         if ((ret->source.adapter = virXPathString(conn,
-                                                  "string(/pool/source/adapter/@name)",
+                                                  "string(./source/adapter/@name)",
                                                   ctxt)) == NULL) {
             virStorageReportError(conn, VIR_ERR_XML_ERROR,
                              "%s", _("missing storage pool source adapter name"));
@@ -586,7 +586,7 @@ virStoragePoolDefParseXML(virConnectPtr conn,
         }
     }
 
-    authType = virXPathString(conn, "string(/pool/source/auth/@type)", ctxt);
+    authType = virXPathString(conn, "string(./source/auth/@type)", ctxt);
     if (authType == NULL) {
         ret->source.authType = VIR_STORAGE_POOL_AUTH_NONE;
     } else {
@@ -607,14 +607,14 @@ virStoragePoolDefParseXML(virConnectPtr conn,
             goto cleanup;
     }
 
-    if ((ret->target.path = virXPathString(conn, "string(/pool/target/path)", ctxt)) == NULL) {
+    if ((ret->target.path = virXPathString(conn, "string(./target/path)", ctxt)) == NULL) {
         virStorageReportError(conn, VIR_ERR_XML_ERROR,
                               "%s", _("missing storage pool target path"));
         goto cleanup;
     }
 
     if (virStorageDefParsePerms(conn, ctxt, &ret->target.perms,
-                                "/pool/target/permissions", 0700) < 0)
+                                "./target/permissions", 0700) < 0)
         goto cleanup;
 
     return ret;
@@ -964,7 +964,7 @@ virStorageVolDefParseXML(virConnectPtr conn,
         return NULL;
     }
 
-    ret->name = virXPathString(conn, "string(/volume/name)", ctxt);
+    ret->name = virXPathString(conn, "string(./name)", ctxt);
     if (ret->name == NULL) {
         virStorageReportError(conn, VIR_ERR_XML_ERROR,
                               "%s", _("missing volume name element"));
@@ -972,10 +972,10 @@ virStorageVolDefParseXML(virConnectPtr conn,
     }
 
     /* Auto-generated so deliberately ignore */
-    /*ret->key = virXPathString(conn, "string(/volume/key)", ctxt);*/
+    /*ret->key = virXPathString(conn, "string(./key)", ctxt);*/
 
-    capacity = virXPathString(conn, "string(/volume/capacity)", ctxt);
-    unit = virXPathString(conn, "string(/volume/capacity/@unit)", ctxt);
+    capacity = virXPathString(conn, "string(./capacity)", ctxt);
+    unit = virXPathString(conn, "string(./capacity/@unit)", ctxt);
     if (capacity == NULL) {
         virStorageReportError(conn, VIR_ERR_XML_ERROR,
                               "%s", _("missing capacity element"));
@@ -986,9 +986,9 @@ virStorageVolDefParseXML(virConnectPtr conn,
     VIR_FREE(capacity);
     VIR_FREE(unit);
 
-    allocation = virXPathString(conn, "string(/volume/allocation)", ctxt);
+    allocation = virXPathString(conn, "string(./allocation)", ctxt);
     if (allocation) {
-        unit = virXPathString(conn, "string(/volume/allocation/@unit)", ctxt);
+        unit = virXPathString(conn, "string(./allocation/@unit)", ctxt);
         if (virStorageSize(conn, unit, allocation, &ret->allocation) < 0)
             goto cleanup;
         VIR_FREE(allocation);
@@ -997,9 +997,9 @@ virStorageVolDefParseXML(virConnectPtr conn,
         ret->allocation = ret->capacity;
     }
 
-    ret->target.path = virXPathString(conn, "string(/volume/target/path)", ctxt);
+    ret->target.path = virXPathString(conn, "string(./target/path)", ctxt);
     if (options->formatFromString) {
-        char *format = virXPathString(conn, "string(/volume/target/format/@type)", ctxt);
+        char *format = virXPathString(conn, "string(./target/format/@type)", ctxt);
         if (format == NULL)
             ret->target.format = options->defaultFormat;
         else
@@ -1015,14 +1015,14 @@ virStorageVolDefParseXML(virConnectPtr conn,
     }
 
     if (virStorageDefParsePerms(conn, ctxt, &ret->target.perms,
-                                "/volume/target/permissions", 0600) < 0)
+                                "./target/permissions", 0600) < 0)
         goto cleanup;
 
 
 
-    ret->backingStore.path = virXPathString(conn, "string(/volume/backingStore/path)", ctxt);
+    ret->backingStore.path = virXPathString(conn, "string(./backingStore/path)", ctxt);
     if (options->formatFromString) {
-        char *format = virXPathString(conn, "string(/volume/backingStore/format/@type)", ctxt);
+        char *format = virXPathString(conn, "string(./backingStore/format/@type)", ctxt);
         if (format == NULL)
             ret->backingStore.format = options->defaultFormat;
         else
@@ -1038,7 +1038,7 @@ virStorageVolDefParseXML(virConnectPtr conn,
     }
 
     if (virStorageDefParsePerms(conn, ctxt, &ret->backingStore.perms,
-                                "/volume/backingStore/permissions", 0600) < 0)
+                                "./backingStore/permissions", 0600) < 0)
         goto cleanup;
 
     return ret;

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