Changeset
src/conf/domain_conf.c          | 69 +++++++++--------------------------------
src/conf/storage_conf.c         |  5 ++-
src/util/virstorageencryption.c | 67 +++++++++++++--------------------------
src/util/virstorageencryption.h |  4 +--
src/util/virstoragefile.c       | 53 ++++++++++---------------------
src/util/virstoragefile.h       |  3 +-
6 files changed, 59 insertions(+), 142 deletions(-)
Git apply log
Switched to a new branch 'cover.1520346349.git.pkrempa@redhat.com'
Applying: util: storage: Simplify error handling in virStorageAuthDefParseXML
Applying: util: storage: Sanitize parsing of disk auth XMLs
Applying: conf: Replace virDomainDiskSourceAuthParse by an XPath query
Applying: util: storageencryption: Refactor cleanup section in virStorageEncryptionParseXML
Applying: util: storage: Sanitize parsing of disk encryption XMLs
Applying: conf: Replace virDomainDiskSourceEncryptionParse by an XPath query
To https://github.com/patchew-project/libvirt
 + 10ce9c11c...7315b975c patchew/cover.1520346349.git.pkrempa@redhat.com -> patchew/cover.1520346349.git.pkrempa@redhat.com (forced update)
Test passed: syntax-check

loading

[libvirt] [PATCH 0/6] Fix overly complex parsing of storage encryption/authentication
Posted by Peter Krempa, 15 weeks ago
Peter Krempa (6):
  util: storage: Simplify error handling in virStorageAuthDefParseXML
  util: storage: Sanitize parsing of disk auth XMLs
  conf: Replace virDomainDiskSourceAuthParse by an XPath query
  util: storageencryption: Refactor cleanup section in
    virStorageEncryptionParseXML
  util: storage: Sanitize parsing of disk encryption XMLs
  conf: Replace virDomainDiskSourceEncryptionParse by an XPath query

 src/conf/domain_conf.c          | 69 +++++++++--------------------------------
 src/conf/storage_conf.c         |  5 ++-
 src/util/virstorageencryption.c | 67 +++++++++++++--------------------------
 src/util/virstorageencryption.h |  4 +--
 src/util/virstoragefile.c       | 53 ++++++++++---------------------
 src/util/virstoragefile.h       |  3 +-
 6 files changed, 59 insertions(+), 142 deletions(-)

-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/6] Fix overly complex parsing of storage encryption/authentication
Posted by John Ferlan, 15 weeks ago

On 03/06/2018 09:27 AM, Peter Krempa wrote:
> Peter Krempa (6):
>   util: storage: Simplify error handling in virStorageAuthDefParseXML
>   util: storage: Sanitize parsing of disk auth XMLs
>   conf: Replace virDomainDiskSourceAuthParse by an XPath query
>   util: storageencryption: Refactor cleanup section in
>     virStorageEncryptionParseXML
>   util: storage: Sanitize parsing of disk encryption XMLs
>   conf: Replace virDomainDiskSourceEncryptionParse by an XPath query
> 
>  src/conf/domain_conf.c          | 69 +++++++++--------------------------------
>  src/conf/storage_conf.c         |  5 ++-
>  src/util/virstorageencryption.c | 67 +++++++++++++--------------------------
>  src/util/virstorageencryption.h |  4 +--
>  src/util/virstoragefile.c       | 53 ++++++++++---------------------
>  src/util/virstoragefile.h       |  3 +-
>  6 files changed, 59 insertions(+), 142 deletions(-)
> 

Reviewed-by: John Ferlan <jferlan@redhat.com>

(series)

John

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/6] util: storage: Simplify error handling in virStorageAuthDefParseXML
Posted by Peter Krempa, 15 weeks ago
Unify the cleanup and error paths and simplify the code flow by removing
some unnecessary variables.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/util/virstoragefile.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index d1972d6d1d..3d17911297 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1813,20 +1813,18 @@ static virStorageAuthDefPtr
 virStorageAuthDefParseXML(xmlXPathContextPtr ctxt)
 {
     virStorageAuthDefPtr authdef = NULL;
+    virStorageAuthDefPtr ret = NULL;
     xmlNodePtr secretnode = NULL;
-    char *username = NULL;
     char *authtype = NULL;

     if (VIR_ALLOC(authdef) < 0)
         return NULL;

-    if (!(username = virXPathString("string(./@username)", ctxt))) {
+    if (!(authdef->username = virXPathString("string(./@username)", ctxt))) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
                        _("missing username for auth"));
-        goto error;
+        goto cleanup;
     }
-    authdef->username = username;
-    username = NULL;

     authdef->authType = VIR_STORAGE_AUTH_TYPE_NONE;
     authtype = virXPathString("string(./@type)", ctxt);
@@ -1837,15 +1835,14 @@ virStorageAuthDefParseXML(xmlXPathContextPtr ctxt)
         if ((authdef->authType = virStorageAuthTypeFromString(authtype)) < 0) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("unknown auth type '%s'"), authtype);
-            goto error;
+            goto cleanup;
         }
-        VIR_FREE(authtype);
     }

     if (!(secretnode = virXPathNode("./secret ", ctxt))) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
                        _("Missing <secret> element in auth"));
-        goto error;
+        goto cleanup;
     }

     /* Used by the domain disk xml parsing in order to ensure the
@@ -1858,15 +1855,15 @@ virStorageAuthDefParseXML(xmlXPathContextPtr ctxt)
     authdef->secrettype = virXMLPropString(secretnode, "type");

     if (virSecretLookupParseSecret(secretnode, &authdef->seclookupdef) < 0)
-        goto error;
+        goto cleanup;

-    return authdef;
+    VIR_STEAL_PTR(ret, authdef);

- error:
+ cleanup:
     VIR_FREE(authtype);
-    VIR_FREE(username);
     virStorageAuthDefFree(authdef);
-    return NULL;
+
+    return ret;
 }


-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] util: storage: Sanitize parsing of disk auth XMLs
Posted by Peter Krempa, 15 weeks ago
Pass in the XPath context as we do in all other places rather than
allocating a new one.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_conf.c    | 21 ++++++++++++---------
 src/conf/storage_conf.c   |  2 +-
 src/util/virstoragefile.c | 32 ++++++++------------------------
 src/util/virstoragefile.h |  3 ++-
 4 files changed, 23 insertions(+), 35 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a248d73de3..a8be0db7e4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7218,7 +7218,8 @@ virDomainHostdevSubsysSCSIHostDefParseXML(xmlNodePtr sourcenode,

 static int
 virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode,
-                                           virDomainHostdevSubsysSCSIPtr def)
+                                           virDomainHostdevSubsysSCSIPtr def,
+                                           xmlXPathContextPtr ctxt)
 {
     int ret = -1;
     int auth_secret_usage = -1;
@@ -7259,7 +7260,7 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode,
     while (cur != NULL) {
         if (cur->type == XML_ELEMENT_NODE &&
             virXMLNodeNameEqual(cur, "auth")) {
-            if (!(authdef = virStorageAuthDefParse(sourcenode->doc, cur)))
+            if (!(authdef = virStorageAuthDefParse(cur, ctxt)))
                 goto cleanup;
             if ((auth_secret_usage =
                  virSecretUsageTypeFromString(authdef->secrettype)) < 0) {
@@ -7288,7 +7289,8 @@ virDomainHostdevSubsysSCSIiSCSIDefParseXML(xmlNodePtr sourcenode,

 static int
 virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr sourcenode,
-                                      virDomainHostdevSubsysSCSIPtr scsisrc)
+                                      virDomainHostdevSubsysSCSIPtr scsisrc,
+                                      xmlXPathContextPtr ctxt)
 {
     char *protocol = NULL;
     int ret = -1;
@@ -7305,7 +7307,7 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr sourcenode,
     }

     if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)
-        ret = virDomainHostdevSubsysSCSIiSCSIDefParseXML(sourcenode, scsisrc);
+        ret = virDomainHostdevSubsysSCSIiSCSIDefParseXML(sourcenode, scsisrc, ctxt);
     else
         ret = virDomainHostdevSubsysSCSIHostDefParseXML(sourcenode, scsisrc);

@@ -7550,7 +7552,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
         break;

     case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
-        if (virDomainHostdevSubsysSCSIDefParseXML(sourcenode, scsisrc) < 0)
+        if (virDomainHostdevSubsysSCSIDefParseXML(sourcenode, scsisrc, ctxt) < 0)
             goto error;
         break;

@@ -8540,7 +8542,8 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,

 static int
 virDomainDiskSourceAuthParse(xmlNodePtr node,
-                             virStorageAuthDefPtr *authdefsrc)
+                             virStorageAuthDefPtr *authdefsrc,
+                             xmlXPathContextPtr ctxt)
 {
     xmlNodePtr child;
     virStorageAuthDefPtr authdef;
@@ -8549,7 +8552,7 @@ virDomainDiskSourceAuthParse(xmlNodePtr node,
         if (child->type == XML_ELEMENT_NODE &&
             virXMLNodeNameEqual(child, "auth")) {

-            if (!(authdef = virStorageAuthDefParse(node->doc, child)))
+            if (!(authdef = virStorageAuthDefParse(child, ctxt)))
                 return -1;

             *authdefsrc = authdef;
@@ -8653,7 +8656,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
         goto cleanup;
     }

-    if (virDomainDiskSourceAuthParse(node, &src->auth) < 0)
+    if (virDomainDiskSourceAuthParse(node, &src->auth, ctxt) < 0)
         goto cleanup;

     if (virDomainDiskSourceEncryptionParse(node, &src->encryption) < 0)
@@ -9401,7 +9404,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                 goto error;
             }

-            if (!(authdef = virStorageAuthDefParse(node->doc, cur)))
+            if (!(authdef = virStorageAuthDefParse(cur, ctxt)))
                 goto error;
         } else if (virXMLNodeNameEqual(cur, "iotune")) {
             if (virDomainDiskDefIotuneParse(def, ctxt) < 0)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index b9135722c1..f1f469d462 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -527,7 +527,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
     }

     if ((authnode = virXPathNode("./auth", ctxt))) {
-        if (!(authdef = virStorageAuthDefParse(node->doc, authnode)))
+        if (!(authdef = virStorageAuthDefParse(authnode, ctxt)))
             goto cleanup;

         if (authdef->authType == VIR_STORAGE_AUTH_TYPE_NONE) {
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 3d17911297..67b9ec71ac 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1809,16 +1809,20 @@ virStorageAuthDefCopy(const virStorageAuthDef *src)
 }


-static virStorageAuthDefPtr
-virStorageAuthDefParseXML(xmlXPathContextPtr ctxt)
+virStorageAuthDefPtr
+virStorageAuthDefParse(xmlNodePtr node,
+                       xmlXPathContextPtr ctxt)
 {
+    xmlNodePtr saveNode = ctxt->node;
     virStorageAuthDefPtr authdef = NULL;
     virStorageAuthDefPtr ret = NULL;
     xmlNodePtr secretnode = NULL;
     char *authtype = NULL;

+    ctxt->node = node;
+
     if (VIR_ALLOC(authdef) < 0)
-        return NULL;
+        goto cleanup;

     if (!(authdef->username = virXPathString("string(./@username)", ctxt))) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -1862,32 +1866,12 @@ virStorageAuthDefParseXML(xmlXPathContextPtr ctxt)
  cleanup:
     VIR_FREE(authtype);
     virStorageAuthDefFree(authdef);
+    ctxt->node = saveNode;

     return ret;
 }


-virStorageAuthDefPtr
-virStorageAuthDefParse(xmlDocPtr xml, xmlNodePtr root)
-{
-    xmlXPathContextPtr ctxt = NULL;
-    virStorageAuthDefPtr authdef = NULL;
-
-    ctxt = xmlXPathNewContext(xml);
-    if (ctxt == NULL) {
-        virReportOOMError();
-        goto cleanup;
-    }
-
-    ctxt->node = root;
-    authdef = virStorageAuthDefParseXML(ctxt);
-
- cleanup:
-    xmlXPathFreeContext(ctxt);
-    return authdef;
-}
-
-
 void
 virStorageAuthDefFormat(virBufferPtr buf,
                         virStorageAuthDefPtr authdef)
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 0095cd1387..596746ccb7 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -366,7 +366,8 @@ int virStorageFileGetSCSIKey(const char *path,

 void virStorageAuthDefFree(virStorageAuthDefPtr def);
 virStorageAuthDefPtr virStorageAuthDefCopy(const virStorageAuthDef *src);
-virStorageAuthDefPtr virStorageAuthDefParse(xmlDocPtr xml, xmlNodePtr root);
+virStorageAuthDefPtr virStorageAuthDefParse(xmlNodePtr node,
+                                            xmlXPathContextPtr ctxt);
 void virStorageAuthDefFormat(virBufferPtr buf, virStorageAuthDefPtr authdef);

 virSecurityDeviceLabelDefPtr
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] conf: Replace virDomainDiskSourceAuthParse by an XPath query
Posted by Peter Krempa, 15 weeks ago
Remove the rather bulky function in favor of an XPath query.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_conf.c | 28 +++-------------------------
 1 file changed, 3 insertions(+), 25 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a8be0db7e4..31b2590a13 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8540,30 +8540,6 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
 }


-static int
-virDomainDiskSourceAuthParse(xmlNodePtr node,
-                             virStorageAuthDefPtr *authdefsrc,
-                             xmlXPathContextPtr ctxt)
-{
-    xmlNodePtr child;
-    virStorageAuthDefPtr authdef;
-
-    for (child = node->children; child; child = child->next) {
-        if (child->type == XML_ELEMENT_NODE &&
-            virXMLNodeNameEqual(child, "auth")) {
-
-            if (!(authdef = virStorageAuthDefParse(child, ctxt)))
-                return -1;
-
-            *authdefsrc = authdef;
-            return 0;
-        }
-    }
-
-    return 0;
-}
-
-
 static int
 virDomainDiskSourceEncryptionParse(xmlNodePtr node,
                                    virStorageEncryptionPtr *encryptionsrc)
@@ -8627,6 +8603,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
 {
     int ret = -1;
     xmlNodePtr saveNode = ctxt->node;
+    xmlNodePtr tmp;

     ctxt->node = node;

@@ -8656,7 +8633,8 @@ virDomainDiskSourceParse(xmlNodePtr node,
         goto cleanup;
     }

-    if (virDomainDiskSourceAuthParse(node, &src->auth, ctxt) < 0)
+    if ((tmp = virXPathNode("./auth", ctxt)) &&
+        !(src->auth = virStorageAuthDefParse(tmp, ctxt)))
         goto cleanup;

     if (virDomainDiskSourceEncryptionParse(node, &src->encryption) < 0)
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] util: storageencryption: Refactor cleanup section in virStorageEncryptionParseXML
Posted by Peter Krempa, 15 weeks ago
The function used the 'cleanup' label only in error cases. This patch
makes the code pass the cleanup label in every case and removes few
unnecessary VIR_FREEs.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/util/virstorageencryption.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c
index 116a2358ae..f3de5ff7a7 100644
--- a/src/util/virstorageencryption.c
+++ b/src/util/virstorageencryption.c
@@ -246,13 +246,14 @@ static virStorageEncryptionPtr
 virStorageEncryptionParseXML(xmlXPathContextPtr ctxt)
 {
     xmlNodePtr *nodes = NULL;
-    virStorageEncryptionPtr ret;
+    virStorageEncryptionPtr encdef = NULL;
+    virStorageEncryptionPtr ret = NULL;
     char *format_str = NULL;
     int n;
     size_t i;

-    if (VIR_ALLOC(ret) < 0)
-        return NULL;
+    if (VIR_ALLOC(encdef) < 0)
+        goto cleanup;

     if (!(format_str = virXPathString("string(./@format)", ctxt))) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -260,60 +261,57 @@ virStorageEncryptionParseXML(xmlXPathContextPtr ctxt)
         goto cleanup;
     }

-    if ((ret->format =
+    if ((encdef->format =
          virStorageEncryptionFormatTypeFromString(format_str)) < 0) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("unknown volume encryption format type %s"),
                        format_str);
         goto cleanup;
     }
-    VIR_FREE(format_str);

     if ((n = virXPathNodeSet("./secret", ctxt, &nodes)) < 0)
         goto cleanup;

     if (n > 0) {
-        if (VIR_ALLOC_N(ret->secrets, n) < 0)
+        if (VIR_ALLOC_N(encdef->secrets, n) < 0)
             goto cleanup;
-        ret->nsecrets = n;
+        encdef->nsecrets = n;

         for (i = 0; i < n; i++) {
-            if (!(ret->secrets[i] =
+            if (!(encdef->secrets[i] =
                   virStorageEncryptionSecretParse(ctxt, nodes[i])))
                 goto cleanup;
         }
-        VIR_FREE(nodes);
     }

-    if (ret->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
+    if (encdef->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS) {
         xmlNodePtr tmpnode;

         if ((tmpnode = virXPathNode("./cipher[1]", ctxt))) {
-            if (virStorageEncryptionInfoParseCipher(tmpnode, &ret->encinfo) < 0)
+            if (virStorageEncryptionInfoParseCipher(tmpnode, &encdef->encinfo) < 0)
                 goto cleanup;
         }

         if ((tmpnode = virXPathNode("./ivgen[1]", ctxt))) {
             /* If no cipher node, then fail */
-            if (!ret->encinfo.cipher_name) {
+            if (!encdef->encinfo.cipher_name) {
                 virReportError(VIR_ERR_XML_ERROR, "%s",
                                 _("ivgen element found, but cipher is missing"));
                 goto cleanup;
             }

-            if (virStorageEncryptionInfoParseIvgen(tmpnode, &ret->encinfo) < 0)
+            if (virStorageEncryptionInfoParseIvgen(tmpnode, &encdef->encinfo) < 0)
                 goto cleanup;
         }
     }

-
-    return ret;
+    VIR_STEAL_PTR(ret, encdef);

  cleanup:
     VIR_FREE(format_str);
     VIR_FREE(nodes);
-    virStorageEncryptionFree(ret);
-    return NULL;
+    virStorageEncryptionFree(encdef);
+    return ret;
 }

 virStorageEncryptionPtr
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] util: storage: Sanitize parsing of disk encryption XMLs
Posted by Peter Krempa, 15 weeks ago
Pass in the XPath context as we do in all other places rather than
allocating a new one.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_conf.c          |  9 +++++----
 src/conf/storage_conf.c         |  3 +--
 src/util/virstorageencryption.c | 37 ++++++++-----------------------------
 src/util/virstorageencryption.h |  4 ++--
 4 files changed, 16 insertions(+), 37 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 31b2590a13..f5bc6148a2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8542,7 +8542,8 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,

 static int
 virDomainDiskSourceEncryptionParse(xmlNodePtr node,
-                                   virStorageEncryptionPtr *encryptionsrc)
+                                   virStorageEncryptionPtr *encryptionsrc,
+                                   xmlXPathContextPtr ctxt)
 {
     xmlNodePtr child;
     virStorageEncryptionPtr encryption = NULL;
@@ -8551,7 +8552,7 @@ virDomainDiskSourceEncryptionParse(xmlNodePtr node,
         if (child->type == XML_ELEMENT_NODE &&
             virXMLNodeNameEqual(child, "encryption")) {

-            if (!(encryption = virStorageEncryptionParseNode(node->doc, child)))
+            if (!(encryption = virStorageEncryptionParseNode(child, ctxt)))
                 return -1;

             *encryptionsrc = encryption;
@@ -8637,7 +8638,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
         !(src->auth = virStorageAuthDefParse(tmp, ctxt)))
         goto cleanup;

-    if (virDomainDiskSourceEncryptionParse(node, &src->encryption) < 0)
+    if (virDomainDiskSourceEncryptionParse(node, &src->encryption, ctxt) < 0)
         goto cleanup;

     if (virDomainDiskSourcePrivateDataParse(ctxt, src, flags, xmlopt) < 0)
@@ -9408,7 +9409,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                 goto error;
             }

-            if (!(encryption = virStorageEncryptionParseNode(node->doc, cur)))
+            if (!(encryption = virStorageEncryptionParseNode(cur, ctxt)))
                 goto error;
         } else if (!serial &&
                    virXMLNodeNameEqual(cur, "serial")) {
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index f1f469d462..5036ab9ef8 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1209,8 +1209,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,

     node = virXPathNode("./target/encryption", ctxt);
     if (node != NULL) {
-        ret->target.encryption = virStorageEncryptionParseNode(ctxt->doc,
-                                                               node);
+        ret->target.encryption = virStorageEncryptionParseNode(node, ctxt);
         if (ret->target.encryption == NULL)
             goto error;
     }
diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c
index f3de5ff7a7..77c46faf8e 100644
--- a/src/util/virstorageencryption.c
+++ b/src/util/virstorageencryption.c
@@ -242,9 +242,11 @@ virStorageEncryptionInfoParseIvgen(xmlNodePtr info_node,
 }


-static virStorageEncryptionPtr
-virStorageEncryptionParseXML(xmlXPathContextPtr ctxt)
+virStorageEncryptionPtr
+virStorageEncryptionParseNode(xmlNodePtr node,
+                              xmlXPathContextPtr ctxt)
 {
+    xmlNodePtr saveNode = ctxt->node;
     xmlNodePtr *nodes = NULL;
     virStorageEncryptionPtr encdef = NULL;
     virStorageEncryptionPtr ret = NULL;
@@ -252,6 +254,8 @@ virStorageEncryptionParseXML(xmlXPathContextPtr ctxt)
     int n;
     size_t i;

+    ctxt->node = node;
+
     if (VIR_ALLOC(encdef) < 0)
         goto cleanup;

@@ -311,34 +315,9 @@ virStorageEncryptionParseXML(xmlXPathContextPtr ctxt)
     VIR_FREE(format_str);
     VIR_FREE(nodes);
     virStorageEncryptionFree(encdef);
-    return ret;
-}
-
-virStorageEncryptionPtr
-virStorageEncryptionParseNode(xmlDocPtr xml, xmlNodePtr root)
-{
-    xmlXPathContextPtr ctxt = NULL;
-    virStorageEncryptionPtr enc = NULL;
-
-    if (STRNEQ((const char *) root->name, "encryption")) {
-        virReportError(VIR_ERR_XML_ERROR,
-                       "%s", _("unknown root element for volume "
-                               "encryption information"));
-        goto cleanup;
-    }
+    ctxt->node = saveNode;

-    ctxt = xmlXPathNewContext(xml);
-    if (ctxt == NULL) {
-        virReportOOMError();
-        goto cleanup;
-    }
-
-    ctxt->node = root;
-    enc = virStorageEncryptionParseXML(ctxt);
-
- cleanup:
-    xmlXPathFreeContext(ctxt);
-    return enc;
+    return ret;
 }


diff --git a/src/util/virstorageencryption.h b/src/util/virstorageencryption.h
index 42f990c494..1c0a39c32e 100644
--- a/src/util/virstorageencryption.h
+++ b/src/util/virstorageencryption.h
@@ -83,8 +83,8 @@ virStorageEncryptionPtr virStorageEncryptionCopy(const virStorageEncryption *src

 void virStorageEncryptionFree(virStorageEncryptionPtr enc);

-virStorageEncryptionPtr virStorageEncryptionParseNode(xmlDocPtr xml,
-                                                      xmlNodePtr root);
+virStorageEncryptionPtr virStorageEncryptionParseNode(xmlNodePtr node,
+                                                      xmlXPathContextPtr ctxt);
 int virStorageEncryptionFormat(virBufferPtr buf,
                                virStorageEncryptionPtr enc);

-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] conf: Replace virDomainDiskSourceEncryptionParse by an XPath query
Posted by Peter Krempa, 15 weeks ago
Remove the rather bulky function in favor of an XPath query.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_conf.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f5bc6148a2..c8d756c45d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8540,30 +8540,6 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
 }


-static int
-virDomainDiskSourceEncryptionParse(xmlNodePtr node,
-                                   virStorageEncryptionPtr *encryptionsrc,
-                                   xmlXPathContextPtr ctxt)
-{
-    xmlNodePtr child;
-    virStorageEncryptionPtr encryption = NULL;
-
-    for (child = node->children; child; child = child->next) {
-        if (child->type == XML_ELEMENT_NODE &&
-            virXMLNodeNameEqual(child, "encryption")) {
-
-            if (!(encryption = virStorageEncryptionParseNode(child, ctxt)))
-                return -1;
-
-            *encryptionsrc = encryption;
-            return 0;
-        }
-    }
-
-    return 0;
-}
-
-
 static int
 virDomainDiskSourcePrivateDataParse(xmlXPathContextPtr ctxt,
                                     virStorageSourcePtr src,
@@ -8638,7 +8614,8 @@ virDomainDiskSourceParse(xmlNodePtr node,
         !(src->auth = virStorageAuthDefParse(tmp, ctxt)))
         goto cleanup;

-    if (virDomainDiskSourceEncryptionParse(node, &src->encryption, ctxt) < 0)
+    if ((tmp = virXPathNode("./encryption", ctxt)) &&
+        !(src->encryption = virStorageEncryptionParseNode(tmp, ctxt)))
         goto cleanup;

     if (virDomainDiskSourcePrivateDataParse(ctxt, src, flags, xmlopt) < 0)
-- 
2.16.2

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