Changeset
src/conf/domain_conf.c   | 402 ++++++++++++++++++++++++++++-------------------
src/conf/domain_conf.h   |  13 ++
src/libvirt_private.syms |   2 +
3 files changed, 254 insertions(+), 163 deletions(-)
Git apply log
Switched to a new branch 'cover.1520951803.git.pkrempa@redhat.com'
Applying: conf: Remove unnecessary condition from virDomainDiskSourceFormatInternal
Applying: conf: Refactor seclabel formatting in virDomainDiskSourceFormatInternal
Applying: conf: Remove virDomainDiskSourceDefFormatSeclabel
Applying: conf: Refactor formatting of startupPolicy in virDomainDiskSourceFormatInternal
Applying: conf: disk: Separate virStorageSource formatting
Applying: conf: Validate disk source configuration also for the backing store
Applying: conf: Separate seclabel validation from parsing
Applying: conf: Parse and validate disk source seclabels together with the source
Applying: conf: Extract parsing of storage source related data
Applying: conf: Add and export wrapper for parsing storage source XML
To https://github.com/patchew-project/libvirt
 * [new tag]         patchew/cover.1520951803.git.pkrempa@redhat.com -> patchew/cover.1520951803.git.pkrempa@redhat.com
Test passed: syntax-check

loading

[libvirt] [PATCH 00/10] conf: Make disk source parsing cleaner and reusable
Posted by Peter Krempa, 14 weeks ago
Fixup and cleanup the disk source parsing code and make it usable in
other parts of the code by exporting it.

Peter Krempa (10):
  conf: Remove unnecessary condition from
    virDomainDiskSourceFormatInternal
  conf: Refactor seclabel formatting in
    virDomainDiskSourceFormatInternal
  conf: Remove virDomainDiskSourceDefFormatSeclabel
  conf: Refactor formatting of startupPolicy in
    virDomainDiskSourceFormatInternal
  conf: disk: Separate virStorageSource formatting
  conf: Validate disk source configuration also for the backing store
  conf: Separate seclabel validation from parsing
  conf: Parse and validate disk source seclabels together with the
    source
  conf: Extract parsing of storage source related data
  conf: Add and export wrapper for parsing storage source XML

 src/conf/domain_conf.c   | 402 ++++++++++++++++++++++++++++-------------------
 src/conf/domain_conf.h   |  13 ++
 src/libvirt_private.syms |   2 +
 3 files changed, 254 insertions(+), 163 deletions(-)

-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 00/10] conf: Make disk source parsing cleaner and reusable
Posted by Michal Privoznik, 14 weeks ago
On 03/13/2018 03:37 PM, Peter Krempa wrote:
> Fixup and cleanup the disk source parsing code and make it usable in
> other parts of the code by exporting it.
> 
> Peter Krempa (10):
>   conf: Remove unnecessary condition from
>     virDomainDiskSourceFormatInternal
>   conf: Refactor seclabel formatting in
>     virDomainDiskSourceFormatInternal
>   conf: Remove virDomainDiskSourceDefFormatSeclabel
>   conf: Refactor formatting of startupPolicy in
>     virDomainDiskSourceFormatInternal
>   conf: disk: Separate virStorageSource formatting
>   conf: Validate disk source configuration also for the backing store
>   conf: Separate seclabel validation from parsing
>   conf: Parse and validate disk source seclabels together with the
>     source
>   conf: Extract parsing of storage source related data
>   conf: Add and export wrapper for parsing storage source XML
> 
>  src/conf/domain_conf.c   | 402 ++++++++++++++++++++++++++++-------------------
>  src/conf/domain_conf.h   |  13 ++
>  src/libvirt_private.syms |   2 +
>  3 files changed, 254 insertions(+), 163 deletions(-)
> 

ACK to patches 1-9.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 01/10] conf: Remove unnecessary condition from virDomainDiskSourceFormatInternal
Posted by Peter Krempa, 14 weeks ago
Now that the function is using virXMLFormatElement we don't need to
conditionally format anything, since we'll format the element according
to the presence of content.

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

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 04a6ee77af..ddabc77a9b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22832,81 +22832,79 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
     if (policy)
         startupPolicy = virDomainStartupPolicyTypeToString(policy);

-    if (src->path || src->nhosts > 0 || src->srcpool || startupPolicy) {
-        switch ((virStorageType)src->type) {
-        case VIR_STORAGE_TYPE_FILE:
-            virBufferEscapeString(&attrBuf, " file='%s'", src->path);
-            virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);
-
-            virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
-                                                 src->seclabels, flags,
-                                                 skipSeclabels);
-            break;
-
-        case VIR_STORAGE_TYPE_BLOCK:
-            virBufferEscapeString(&attrBuf, " dev='%s'", src->path);
-            virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);
-
-            virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
-                                                 src->seclabels, flags,
-                                                 skipSeclabels);
-            break;
+    switch ((virStorageType)src->type) {
+    case VIR_STORAGE_TYPE_FILE:
+        virBufferEscapeString(&attrBuf, " file='%s'", src->path);
+        virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);

-        case VIR_STORAGE_TYPE_DIR:
-            virBufferEscapeString(&attrBuf, " dir='%s'", src->path);
-            virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);
-            break;
+        virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
+                                             src->seclabels, flags,
+                                             skipSeclabels);
+        break;

-        case VIR_STORAGE_TYPE_NETWORK:
-            if (virDomainDiskSourceFormatNetwork(&attrBuf, &childBuf,
-                                                 src, flags) < 0)
-                goto error;
-            break;
+    case VIR_STORAGE_TYPE_BLOCK:
+        virBufferEscapeString(&attrBuf, " dev='%s'", src->path);
+        virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);

-        case VIR_STORAGE_TYPE_VOLUME:
-            if (src->srcpool) {
-                virBufferEscapeString(&attrBuf, " pool='%s'", src->srcpool->pool);
-                virBufferEscapeString(&attrBuf, " volume='%s'",
-                                      src->srcpool->volume);
-                if (src->srcpool->mode)
-                    virBufferAsprintf(&attrBuf, " mode='%s'",
-                                      virStorageSourcePoolModeTypeToString(src->srcpool->mode));
-            }
-            virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);
+        virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
+                                             src->seclabels, flags,
+                                             skipSeclabels);
+        break;

-            virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
-                                                 src->seclabels, flags,
-                                                 skipSeclabels);
-            break;
+    case VIR_STORAGE_TYPE_DIR:
+        virBufferEscapeString(&attrBuf, " dir='%s'", src->path);
+        virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);
+        break;

-        case VIR_STORAGE_TYPE_NONE:
-        case VIR_STORAGE_TYPE_LAST:
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("unexpected disk type %d"), src->type);
+    case VIR_STORAGE_TYPE_NETWORK:
+        if (virDomainDiskSourceFormatNetwork(&attrBuf, &childBuf,
+                                             src, flags) < 0)
             goto error;
+        break;
+
+    case VIR_STORAGE_TYPE_VOLUME:
+        if (src->srcpool) {
+            virBufferEscapeString(&attrBuf, " pool='%s'", src->srcpool->pool);
+            virBufferEscapeString(&attrBuf, " volume='%s'",
+                                  src->srcpool->volume);
+            if (src->srcpool->mode)
+                virBufferAsprintf(&attrBuf, " mode='%s'",
+                                  virStorageSourcePoolModeTypeToString(src->srcpool->mode));
         }
+        virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);

-        /* Storage Source formatting will not carry through the blunder
-         * that disk source formatting had at one time to format the
-         * <auth> for a volume source type. The <auth> information is
-         * kept in the storage pool and would be overwritten anyway.
-         * So avoid formatting it for volumes. */
-        if (src->auth && src->authInherited &&
-            src->type != VIR_STORAGE_TYPE_VOLUME)
-            virStorageAuthDefFormat(&childBuf, src->auth);
+        virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
+                                             src->seclabels, flags,
+                                             skipSeclabels);
+        break;

-        /* If we found encryption as a child of <source>, then format it
-         * as we found it. */
-        if (src->encryption && src->encryptionInherited &&
-            virStorageEncryptionFormat(&childBuf, src->encryption) < 0)
-            goto error;
+    case VIR_STORAGE_TYPE_NONE:
+    case VIR_STORAGE_TYPE_LAST:
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("unexpected disk type %d"), src->type);
+        goto error;
+    }

-        if (virDomainDiskSourceFormatPrivateData(&childBuf, src, flags, xmlopt) < 0)
-            goto error;
+    /* Storage Source formatting will not carry through the blunder
+     * that disk source formatting had at one time to format the
+     * <auth> for a volume source type. The <auth> information is
+     * kept in the storage pool and would be overwritten anyway.
+     * So avoid formatting it for volumes. */
+    if (src->auth && src->authInherited &&
+        src->type != VIR_STORAGE_TYPE_VOLUME)
+        virStorageAuthDefFormat(&childBuf, src->auth);

-        if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0)
-            goto error;
-    }
+    /* If we found encryption as a child of <source>, then format it
+     * as we found it. */
+    if (src->encryption && src->encryptionInherited &&
+        virStorageEncryptionFormat(&childBuf, src->encryption) < 0)
+        goto error;
+
+    if (virDomainDiskSourceFormatPrivateData(&childBuf, src, flags, xmlopt) < 0)
+        goto error;
+
+    if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0)
+        goto error;

     return 0;

-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 02/10] conf: Refactor seclabel formatting in virDomainDiskSourceFormatInternal
Posted by Peter Krempa, 14 weeks ago
Call the formatter function only once.

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

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ddabc77a9b..ebe1172fd2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22836,19 +22836,11 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
     case VIR_STORAGE_TYPE_FILE:
         virBufferEscapeString(&attrBuf, " file='%s'", src->path);
         virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);
-
-        virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
-                                             src->seclabels, flags,
-                                             skipSeclabels);
         break;

     case VIR_STORAGE_TYPE_BLOCK:
         virBufferEscapeString(&attrBuf, " dev='%s'", src->path);
         virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);
-
-        virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
-                                             src->seclabels, flags,
-                                             skipSeclabels);
         break;

     case VIR_STORAGE_TYPE_DIR:
@@ -22873,9 +22865,6 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
         }
         virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);

-        virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
-                                             src->seclabels, flags,
-                                             skipSeclabels);
         break;

     case VIR_STORAGE_TYPE_NONE:
@@ -22885,6 +22874,12 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
         goto error;
     }

+    if (src->type != VIR_STORAGE_TYPE_NETWORK) {
+        virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
+                                             src->seclabels, flags,
+                                             skipSeclabels);
+    }
+
     /* Storage Source formatting will not carry through the blunder
      * that disk source formatting had at one time to format the
      * <auth> for a volume source type. The <auth> information is
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 03/10] conf: Remove virDomainDiskSourceDefFormatSeclabel
Posted by Peter Krempa, 14 weeks ago
The wrapper functionality can be moved to the only user
virDomainDiskSourceFormatInternal. Also removes comment which does not
reflect the truth any more.

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

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ebe1172fd2..4aa66fe09c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22704,33 +22704,16 @@ virDomainDiskBlockIoDefFormat(virBufferPtr buf,
 }


-/* virDomainSourceDefFormatSeclabel:
- *
- * This function automatically closes the <source> element and formats any
- * possible seclabels.
- */
-static void
-virDomainDiskSourceDefFormatSeclabel(virBufferPtr buf,
-                                     size_t nseclabels,
-                                     virSecurityDeviceLabelDefPtr *seclabels,
-                                     unsigned int flags,
-                                     bool skipSeclables)
-{
-    size_t n;
-
-    if (nseclabels && !skipSeclables) {
-        for (n = 0; n < nseclabels; n++)
-            virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
-    }
-}
-
 static void
 virDomainSourceDefFormatSeclabel(virBufferPtr buf,
                                  size_t nseclabels,
                                  virSecurityDeviceLabelDefPtr *seclabels,
                                  unsigned int flags)
 {
-    virDomainDiskSourceDefFormatSeclabel(buf, nseclabels, seclabels, flags, false);
+    size_t n;
+
+    for (n = 0; n < nseclabels; n++)
+        virSecurityDeviceLabelDefFormat(buf, seclabels[n], flags);
 }


@@ -22875,9 +22858,9 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
     }

     if (src->type != VIR_STORAGE_TYPE_NETWORK) {
-        virDomainDiskSourceDefFormatSeclabel(&childBuf, src->nseclabels,
-                                             src->seclabels, flags,
-                                             skipSeclabels);
+        if (!skipSeclabels)
+            virDomainSourceDefFormatSeclabel(&childBuf, src->nseclabels,
+                                             src->seclabels, flags);
     }

     /* Storage Source formatting will not carry through the blunder
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 04/10] conf: Refactor formatting of startupPolicy in virDomainDiskSourceFormatInternal
Posted by Peter Krempa, 14 weeks ago
Move it to a single location which also allows to get rid of the
temporrary variable.

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

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4aa66fe09c..b77cc8ed9f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22806,29 +22806,22 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
                                   bool skipSeclabels,
                                   virDomainXMLOptionPtr xmlopt)
 {
-    const char *startupPolicy = NULL;
     virBuffer attrBuf = VIR_BUFFER_INITIALIZER;
     virBuffer childBuf = VIR_BUFFER_INITIALIZER;

     virBufferSetChildIndent(&childBuf, buf);

-    if (policy)
-        startupPolicy = virDomainStartupPolicyTypeToString(policy);
-
     switch ((virStorageType)src->type) {
     case VIR_STORAGE_TYPE_FILE:
         virBufferEscapeString(&attrBuf, " file='%s'", src->path);
-        virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);
         break;

     case VIR_STORAGE_TYPE_BLOCK:
         virBufferEscapeString(&attrBuf, " dev='%s'", src->path);
-        virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);
         break;

     case VIR_STORAGE_TYPE_DIR:
         virBufferEscapeString(&attrBuf, " dir='%s'", src->path);
-        virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);
         break;

     case VIR_STORAGE_TYPE_NETWORK:
@@ -22846,7 +22839,6 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
                 virBufferAsprintf(&attrBuf, " mode='%s'",
                                   virStorageSourcePoolModeTypeToString(src->srcpool->mode));
         }
-        virBufferEscapeString(&attrBuf, " startupPolicy='%s'", startupPolicy);

         break;

@@ -22858,6 +22850,10 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
     }

     if (src->type != VIR_STORAGE_TYPE_NETWORK) {
+        if (policy)
+            virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
+                                  virDomainStartupPolicyTypeToString(policy));
+
         if (!skipSeclabels)
             virDomainSourceDefFormatSeclabel(&childBuf, src->nseclabels,
                                              src->seclabels, flags);
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 05/10] conf: disk: Separate virStorageSource formatting
Posted by Peter Krempa, 14 weeks ago
Move out formatting of 'startuPolicy' which is a property of the disk
out of the <source> element. Extracting the code formating the content
and attributes will also allow reuse in other parts of the code.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_conf.c   | 90 ++++++++++++++++++++++++++++--------------------
 src/conf/domain_conf.h   |  7 ++++
 src/libvirt_private.syms |  1 +
 3 files changed, 60 insertions(+), 38 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b77cc8ed9f..1c79d2b49b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -22798,45 +22798,39 @@ virDomainDiskSourceFormatPrivateData(virBufferPtr buf,
 }


-static int
-virDomainDiskSourceFormatInternal(virBufferPtr buf,
-                                  virStorageSourcePtr src,
-                                  int policy,
-                                  unsigned int flags,
-                                  bool skipSeclabels,
-                                  virDomainXMLOptionPtr xmlopt)
+int
+virDomainStorageSourceFormat(virBufferPtr attrBuf,
+                             virBufferPtr childBuf,
+                             virStorageSourcePtr src,
+                             unsigned int flags,
+                             bool skipSeclabels)
 {
-    virBuffer attrBuf = VIR_BUFFER_INITIALIZER;
-    virBuffer childBuf = VIR_BUFFER_INITIALIZER;
-
-    virBufferSetChildIndent(&childBuf, buf);
-
     switch ((virStorageType)src->type) {
     case VIR_STORAGE_TYPE_FILE:
-        virBufferEscapeString(&attrBuf, " file='%s'", src->path);
+        virBufferEscapeString(attrBuf, " file='%s'", src->path);
         break;

     case VIR_STORAGE_TYPE_BLOCK:
-        virBufferEscapeString(&attrBuf, " dev='%s'", src->path);
+        virBufferEscapeString(attrBuf, " dev='%s'", src->path);
         break;

     case VIR_STORAGE_TYPE_DIR:
-        virBufferEscapeString(&attrBuf, " dir='%s'", src->path);
+        virBufferEscapeString(attrBuf, " dir='%s'", src->path);
         break;

     case VIR_STORAGE_TYPE_NETWORK:
-        if (virDomainDiskSourceFormatNetwork(&attrBuf, &childBuf,
+        if (virDomainDiskSourceFormatNetwork(attrBuf, childBuf,
                                              src, flags) < 0)
-            goto error;
+            return -1;
         break;

     case VIR_STORAGE_TYPE_VOLUME:
         if (src->srcpool) {
-            virBufferEscapeString(&attrBuf, " pool='%s'", src->srcpool->pool);
-            virBufferEscapeString(&attrBuf, " volume='%s'",
+            virBufferEscapeString(attrBuf, " pool='%s'", src->srcpool->pool);
+            virBufferEscapeString(attrBuf, " volume='%s'",
                                   src->srcpool->volume);
             if (src->srcpool->mode)
-                virBufferAsprintf(&attrBuf, " mode='%s'",
+                virBufferAsprintf(attrBuf, " mode='%s'",
                                   virStorageSourcePoolModeTypeToString(src->srcpool->mode));
         }

@@ -22846,18 +22840,12 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
     case VIR_STORAGE_TYPE_LAST:
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("unexpected disk type %d"), src->type);
-        goto error;
+        return -1;
     }

-    if (src->type != VIR_STORAGE_TYPE_NETWORK) {
-        if (policy)
-            virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
-                                  virDomainStartupPolicyTypeToString(policy));
-
-        if (!skipSeclabels)
-            virDomainSourceDefFormatSeclabel(&childBuf, src->nseclabels,
-                                             src->seclabels, flags);
-    }
+    if (!skipSeclabels && src->type != VIR_STORAGE_TYPE_NETWORK)
+        virDomainSourceDefFormatSeclabel(childBuf, src->nseclabels,
+                                         src->seclabels, flags);

     /* Storage Source formatting will not carry through the blunder
      * that disk source formatting had at one time to format the
@@ -22866,26 +22854,52 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
      * So avoid formatting it for volumes. */
     if (src->auth && src->authInherited &&
         src->type != VIR_STORAGE_TYPE_VOLUME)
-        virStorageAuthDefFormat(&childBuf, src->auth);
+        virStorageAuthDefFormat(childBuf, src->auth);

     /* If we found encryption as a child of <source>, then format it
      * as we found it. */
     if (src->encryption && src->encryptionInherited &&
-        virStorageEncryptionFormat(&childBuf, src->encryption) < 0)
-        goto error;
+        virStorageEncryptionFormat(childBuf, src->encryption) < 0)
+        return -1;
+
+    return 0;
+}
+
+
+static int
+virDomainDiskSourceFormatInternal(virBufferPtr buf,
+                                  virStorageSourcePtr src,
+                                  int policy,
+                                  unsigned int flags,
+                                  bool skipSeclabels,
+                                  virDomainXMLOptionPtr xmlopt)
+{
+    virBuffer attrBuf = VIR_BUFFER_INITIALIZER;
+    virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+    int ret = -1;
+
+    virBufferSetChildIndent(&childBuf, buf);
+
+    if (virDomainStorageSourceFormat(&attrBuf, &childBuf, src, flags,
+                                     skipSeclabels) < 0)
+        goto cleanup;
+
+    if (policy && src->type != VIR_STORAGE_TYPE_NETWORK)
+        virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
+                              virDomainStartupPolicyTypeToString(policy));

     if (virDomainDiskSourceFormatPrivateData(&childBuf, src, flags, xmlopt) < 0)
-        goto error;
+        goto cleanup;

     if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0)
-        goto error;
+        goto cleanup;

-    return 0;
+    ret = 0;

- error:
+ cleanup:
     virBufferFreeAndReset(&attrBuf);
     virBufferFreeAndReset(&childBuf);
-    return -1;
+    return ret;
 }


diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 337ce79425..61379e50fe 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3427,6 +3427,13 @@ int virDomainDiskDefCheckDuplicateInfo(const virDomainDiskDef *a,
                                        const virDomainDiskDef *b)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

+int virDomainStorageSourceFormat(virBufferPtr attrBuf,
+                                 virBufferPtr childBuf,
+                                 virStorageSourcePtr src,
+                                 unsigned int flags,
+                                 bool skipSeclabels)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+
 int virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
                                      int maplen,
                                      int ncpumaps,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3766e20d3b..c67bce7389 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -542,6 +542,7 @@ virDomainStateReasonFromString;
 virDomainStateReasonToString;
 virDomainStateTypeFromString;
 virDomainStateTypeToString;
+virDomainStorageSourceFormat;
 virDomainTaintTypeFromString;
 virDomainTaintTypeToString;
 virDomainTimerModeTypeFromString;
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 06/10] conf: Validate disk source configuration also for the backing store
Posted by Peter Krempa, 14 weeks ago
Since we already parse the <backingStore> of a disk source, we should
also validate the configuration for the whole backing chain and not only
for the top level image.

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

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1c79d2b49b..8cd41edb5e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8974,6 +8974,8 @@ virDomainDiskSourceDefParseAuthValidate(const virStorageSource *src)
 static int
 virDomainDiskDefParseValidate(const virDomainDiskDef *def)
 {
+    virStorageSourcePtr next;
+
     if (def->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) {
         if (def->event_idx != VIR_TRISTATE_SWITCH_ABSENT) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -9044,19 +9046,21 @@ virDomainDiskDefParseValidate(const virDomainDiskDef *def)
         }
     }

-    if (virDomainDiskSourceDefParseAuthValidate(def->src) < 0)
-        return -1;
+    for (next = def->src; next; next = next->backingStore) {
+        if (virDomainDiskSourceDefParseAuthValidate(next) < 0)
+            return -1;

-    if (def->src->encryption) {
-        virStorageEncryptionPtr encryption = def->src->encryption;
+        if (next->encryption) {
+            virStorageEncryptionPtr encryption = next->encryption;

-        if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
-            encryption->encinfo.cipher_name) {
+            if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS &&
+                encryption->encinfo.cipher_name) {

-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("supplying <cipher> for domain disk definition "
-                             "is unnecessary"));
-            return -1;
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("supplying <cipher> for domain disk definition "
+                                 "is unnecessary"));
+                return -1;
+            }
         }
     }

-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 07/10] conf: Separate seclabel validation from parsing
Posted by Peter Krempa, 14 weeks ago
Rather than checking that the security label is legal when parsing it
move the code into a separate function.

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

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8cd41edb5e..6c2a2f3a75 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8214,8 +8214,7 @@ virSecurityLabelDefsParseXML(virDomainDefPtr def,
 static int
 virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
                                   size_t *nseclabels_rtn,
-                                  virSecurityLabelDefPtr *vmSeclabels,
-                                  int nvmSeclabels, xmlXPathContextPtr ctxt,
+                                  xmlXPathContextPtr ctxt,
                                   unsigned int flags)
 {
     virSecurityDeviceLabelDefPtr *seclabels = NULL;
@@ -8223,7 +8222,6 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
     int n;
     size_t i, j;
     xmlNodePtr *list = NULL;
-    virSecurityLabelDefPtr vmDef = NULL;
     char *model, *relabel, *label, *labelskip;

     if ((n = virXPathNodeSet("./seclabel", ctxt, &list)) < 0)
@@ -8243,14 +8241,6 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
         /* get model associated to this override */
         model = virXMLPropString(list[i], "model");
         if (model) {
-            /* find the security label that it's being overridden */
-            for (j = 0; j < nvmSeclabels; j++) {
-                if (STREQ(vmSeclabels[j]->model, model)) {
-                    vmDef = vmSeclabels[j];
-                    break;
-                }
-            }
-
             /* check for duplicate seclabels */
             for (j = 0; j < i; j++) {
                 if (STREQ_NULLABLE(model, seclabels[j]->model)) {
@@ -8262,14 +8252,6 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
             seclabels[i]->model = model;
         }

-        /* Can't use overrides if top-level doesn't allow relabeling.  */
-        if (vmDef && !vmDef->relabel) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("label overrides require relabeling to be "
-                             "enabled at the domain level"));
-            goto error;
-        }
-
         relabel = virXMLPropString(list[i], "relabel");
         if (relabel != NULL) {
             if (STREQ(relabel, "yes")) {
@@ -8324,6 +8306,37 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDefPtr **seclabels_rtn,
 }


+static int
+virSecurityDeviceLabelDefValidateXML(virSecurityDeviceLabelDefPtr *seclabels,
+                                     size_t nseclabels,
+                                     virSecurityLabelDefPtr *vmSeclabels,
+                                     size_t nvmSeclabels)
+{
+    virSecurityDeviceLabelDefPtr seclabel;
+    size_t i;
+    size_t j;
+
+    for (i = 0; i < nseclabels; i++) {
+        seclabel = seclabels[i];
+
+        /* find the security label that it's being overridden */
+        for (j = 0; j < nvmSeclabels; j++) {
+            if (STRNEQ_NULLABLE(vmSeclabels[j]->model, seclabel->model))
+                continue;
+
+            if (!vmSeclabels[j]->relabel) {
+                virReportError(VIR_ERR_XML_ERROR, "%s",
+                               _("label overrides require relabeling to be "
+                                 "enabled at the domain level"));
+                return -1;
+            }
+        }
+    }
+
+    return 0;
+}
+
+
 /* Parse the XML definition for a lease
  */
 static virDomainLeaseDefPtr
@@ -9453,11 +9466,16 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
         ctxt->node = sourceNode;
         if (virSecurityDeviceLabelDefParseXML(&def->src->seclabels,
                                               &def->src->nseclabels,
-                                              vmSeclabels,
-                                              nvmSeclabels,
                                               ctxt,
                                               flags) < 0)
             goto error;
+
+        if (virSecurityDeviceLabelDefValidateXML(def->src->seclabels,
+                                                 def->src->nseclabels,
+                                                 vmSeclabels,
+                                                 nvmSeclabels) < 0)
+            goto error;
+
         ctxt->node = saved_node;
     }

@@ -12133,10 +12151,12 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDefPtr def,
                 ctxt->node = cur;
                 if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
                                                       &def->nseclabels,
-                                                      vmSeclabels,
-                                                      nvmSeclabels,
                                                       ctxt,
-                                                      flags) < 0) {
+                                                      flags) < 0 ||
+                    virSecurityDeviceLabelDefValidateXML(def->seclabels,
+                                                         def->nseclabels,
+                                                         vmSeclabels,
+                                                         nvmSeclabels) < 0) {
                     ctxt->node = saved_node;
                     goto error;
                 }
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 08/10] conf: Parse and validate disk source seclabels together with the source
Posted by Peter Krempa, 14 weeks ago
Since seclabels are formatted along with the source element and will
also make sense to be passed for the backing chain we should parse them
in the place where we parse the disk source. Same applies for
validation.

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

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6c2a2f3a75..d1ff80feb7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8631,6 +8631,10 @@ virDomainDiskSourceParse(xmlNodePtr node,
         !(src->encryption = virStorageEncryptionParseNode(tmp, ctxt)))
         goto cleanup;

+    if (virSecurityDeviceLabelDefParseXML(&src->seclabels, &src->nseclabels,
+                                          ctxt, flags) < 0)
+        goto cleanup;
+
     if (virDomainDiskSourcePrivateDataParse(ctxt, src, flags, xmlopt) < 0)
         goto cleanup;

@@ -8985,7 +8989,10 @@ virDomainDiskSourceDefParseAuthValidate(const virStorageSource *src)


 static int
-virDomainDiskDefParseValidate(const virDomainDiskDef *def)
+virDomainDiskDefParseValidate(const virDomainDiskDef *def,
+                              virSecurityLabelDefPtr *vmSeclabels,
+                              size_t nvmSeclabels)
+
 {
     virStorageSourcePtr next;

@@ -9075,6 +9082,12 @@ virDomainDiskDefParseValidate(const virDomainDiskDef *def)
                 return -1;
             }
         }
+
+        if (virSecurityDeviceLabelDefValidateXML(next->seclabels,
+                                                 next->nseclabels,
+                                                 vmSeclabels,
+                                                 nvmSeclabels) < 0)
+            return -1;
     }

     return 0;
@@ -9222,7 +9235,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
                          unsigned int flags)
 {
     virDomainDiskDefPtr def;
-    xmlNodePtr sourceNode = NULL;
     xmlNodePtr cur;
     xmlNodePtr save_ctxt = ctxt->node;
     char *tmp = NULL;
@@ -9281,8 +9293,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
             continue;

         if (!source && virXMLNodeNameEqual(cur, "source")) {
-            sourceNode = cur;
-
             if (virDomainDiskSourceParse(cur, ctxt, def->src, flags, xmlopt) < 0)
                 goto error;

@@ -9460,25 +9470,6 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
         goto error;
     }

-    /* If source is present, check for an optional seclabel override.  */
-    if (sourceNode) {
-        xmlNodePtr saved_node = ctxt->node;
-        ctxt->node = sourceNode;
-        if (virSecurityDeviceLabelDefParseXML(&def->src->seclabels,
-                                              &def->src->nseclabels,
-                                              ctxt,
-                                              flags) < 0)
-            goto error;
-
-        if (virSecurityDeviceLabelDefValidateXML(def->src->seclabels,
-                                                 def->src->nseclabels,
-                                                 vmSeclabels,
-                                                 nvmSeclabels) < 0)
-            goto error;
-
-        ctxt->node = saved_node;
-    }
-
     if (!target && !(flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) {
         if (def->src->srcpool) {
             if (virAsprintf(&tmp, "pool = '%s', volume = '%s'",
@@ -9644,7 +9635,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
             goto error;
     }

-    if (virDomainDiskDefParseValidate(def) < 0)
+    if (virDomainDiskDefParseValidate(def, vmSeclabels, nvmSeclabels) < 0)
         goto error;

  cleanup:
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 09/10] conf: Extract parsing of storage source related data
Posted by Peter Krempa, 14 weeks ago
Split out the parser and separate it from the private data part so that
it can be later reused in other parts of the code.

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

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d1ff80feb7..86fc275116 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8554,24 +8554,26 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,


 static int
-virDomainDiskSourcePrivateDataParse(xmlXPathContextPtr ctxt,
+virDomainDiskSourcePrivateDataParse(xmlNodePtr node,
+                                    xmlXPathContextPtr ctxt,
                                     virStorageSourcePtr src,
                                     unsigned int flags,
                                     virDomainXMLOptionPtr xmlopt)
 {
     xmlNodePtr saveNode = ctxt->node;
-    xmlNodePtr node;
     int ret = -1;

     if (!(flags & VIR_DOMAIN_DEF_PARSE_STATUS) ||
         !xmlopt || !xmlopt->privateData.storageParse)
         return 0;

-    if (!(node = virXPathNode("./privateData", ctxt)))
-        return 0;
-
     ctxt->node = node;

+    if (!(ctxt->node = virXPathNode("./privateData", ctxt))) {
+        ret = 0;
+        goto cleanup;
+    }
+
     if (xmlopt->privateData.storageParse(ctxt, src) < 0)
         goto cleanup;

@@ -8584,12 +8586,11 @@ virDomainDiskSourcePrivateDataParse(xmlXPathContextPtr ctxt,
 }


-int
-virDomainDiskSourceParse(xmlNodePtr node,
-                         xmlXPathContextPtr ctxt,
-                         virStorageSourcePtr src,
-                         unsigned int flags,
-                         virDomainXMLOptionPtr xmlopt)
+static int
+virDomainStorageSourceParse(xmlNodePtr node,
+                            xmlXPathContextPtr ctxt,
+                            virStorageSourcePtr src,
+                            unsigned int flags)
 {
     int ret = -1;
     xmlNodePtr saveNode = ctxt->node;
@@ -8635,9 +8636,6 @@ virDomainDiskSourceParse(xmlNodePtr node,
                                           ctxt, flags) < 0)
         goto cleanup;

-    if (virDomainDiskSourcePrivateDataParse(ctxt, src, flags, xmlopt) < 0)
-        goto cleanup;
-
     /* People sometimes pass a bogus '' source path when they mean to omit the
      * source element completely (e.g. CDROM without media). This is just a
      * little compatibility check to help those broken apps */
@@ -8652,6 +8650,23 @@ virDomainDiskSourceParse(xmlNodePtr node,
 }


+int
+virDomainDiskSourceParse(xmlNodePtr node,
+                         xmlXPathContextPtr ctxt,
+                         virStorageSourcePtr src,
+                         unsigned int flags,
+                         virDomainXMLOptionPtr xmlopt)
+{
+    if (virDomainStorageSourceParse(node, ctxt, src, flags) < 0)
+        return -1;
+
+    if (virDomainDiskSourcePrivateDataParse(node, ctxt, src, flags, xmlopt) < 0)
+        return -1;
+
+    return 0;
+}
+
+
 static int
 virDomainDiskBackingStoreParse(xmlXPathContextPtr ctxt,
                                virStorageSourcePtr src,
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 10/10] conf: Add and export wrapper for parsing storage source XML
Posted by Peter Krempa, 14 weeks ago
Add a helper that parses a storage source XML node into a new
virStorageSource object. Since there are multiple approaches to store
the 'type' and 'format' attributes, they need to be parsed manually
prior to calling the function.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_conf.c   | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/conf/domain_conf.h   |  6 +++++
 src/libvirt_private.syms |  1 +
 3 files changed, 67 insertions(+)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 86fc275116..0b25c6316f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8650,6 +8650,66 @@ virDomainStorageSourceParse(xmlNodePtr node,
 }


+/**
+ * virDomainStorageSourceParseNew:
+ * @node: XML node object to parse
+ * @ctxt: XML XPath context
+ * @flags: virDomainDefParseFlags
+ *
+ * Parses the XML @node and returns a virStorageSource object with the parsed
+ * data. Note that 'format' and 'type' attributes need to be members of the same
+ * object and need to be provided.
+ */
+virStorageSourcePtr
+virDomainStorageSourceParseNew(xmlNodePtr node,
+                               xmlXPathContextPtr ctxt,
+                               unsigned int flags)
+{
+    virStorageSourcePtr src = NULL;
+    virStorageSourcePtr ret = NULL;
+    char *format = NULL;
+    char *type = NULL;
+
+    if (VIR_ALLOC(src) < 0)
+        return NULL;
+
+    if (!(type = virXMLPropString(node, "type"))) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("missing storage source type"));
+        goto cleanup;
+    }
+
+    if (!(format = virXMLPropString(node, "format"))) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       ("missing storage source format"));
+        goto cleanup;
+    }
+
+    if ((src->type = virStorageTypeFromString(type)) <= 0) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("unknown storage source type '%s'"), type);
+        goto cleanup;
+    }
+
+    if ((src->format = virStorageFileFormatTypeFromString(format)) <= 0) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("unknown storage source format '%s'"), format);
+        goto cleanup;
+    }
+
+    if (virDomainStorageSourceParse(node, ctxt, src, flags) < 0)
+        goto cleanup;
+
+    VIR_STEAL_PTR(ret, src);
+
+ cleanup:
+    virStorageSourceFree(src);
+    VIR_FREE(format);
+    VIR_FREE(type);
+    return ret;
+}
+
+
 int
 virDomainDiskSourceParse(xmlNodePtr node,
                          xmlXPathContextPtr ctxt,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 61379e50fe..c82a23d220 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3434,6 +3434,12 @@ int virDomainStorageSourceFormat(virBufferPtr attrBuf,
                                  bool skipSeclabels)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);

+virStorageSourcePtr
+virDomainStorageSourceParseNew(xmlNodePtr node,
+                               xmlXPathContextPtr ctxt,
+                               unsigned int flags)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
 int virDomainDefGetVcpuPinInfoHelper(virDomainDefPtr def,
                                      int maplen,
                                      int ncpumaps,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c67bce7389..4dfcbb4230 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -543,6 +543,7 @@ virDomainStateReasonToString;
 virDomainStateTypeFromString;
 virDomainStateTypeToString;
 virDomainStorageSourceFormat;
+virDomainStorageSourceParseNew;
 virDomainTaintTypeFromString;
 virDomainTaintTypeToString;
 virDomainTimerModeTypeFromString;
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/10] conf: Add and export wrapper for parsing storage source XML
Posted by Michal Privoznik, 14 weeks ago
On 03/13/2018 03:37 PM, Peter Krempa wrote:
> Add a helper that parses a storage source XML node into a new
> virStorageSource object. Since there are multiple approaches to store
> the 'type' and 'format' attributes, they need to be parsed manually
> prior to calling the function.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/conf/domain_conf.c   | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
>  src/conf/domain_conf.h   |  6 +++++
>  src/libvirt_private.syms |  1 +
>  3 files changed, 67 insertions(+)

Perhaps you forgot to include next patch that uses
virDomainStorageSourceParseNew()? Because this patch does nothing more
than introducing a function that is never used.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/10] conf: Add and export wrapper for parsing storage source XML
Posted by Peter Krempa, 14 weeks ago
On Wed, Mar 14, 2018 at 11:00:46 +0100, Michal Privoznik wrote:
> On 03/13/2018 03:37 PM, Peter Krempa wrote:
> > Add a helper that parses a storage source XML node into a new
> > virStorageSource object. Since there are multiple approaches to store
> > the 'type' and 'format' attributes, they need to be parsed manually
> > prior to calling the function.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  src/conf/domain_conf.c   | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  src/conf/domain_conf.h   |  6 +++++
> >  src/libvirt_private.syms |  1 +
> >  3 files changed, 67 insertions(+)
> 
> Perhaps you forgot to include next patch that uses
> virDomainStorageSourceParseNew()? Because this patch does nothing more
> than introducing a function that is never used.

Yes. The patch that uses this function was not posted yet. I'm sending
these sub-series out so that I don't have to send 50+ patches at once.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/10] conf: Add and export wrapper for parsing storage source XML
Posted by Michal Privoznik, 14 weeks ago
On 03/14/2018 11:57 AM, Peter Krempa wrote:
> On Wed, Mar 14, 2018 at 11:00:46 +0100, Michal Privoznik wrote:
>> On 03/13/2018 03:37 PM, Peter Krempa wrote:
>>> Add a helper that parses a storage source XML node into a new
>>> virStorageSource object. Since there are multiple approaches to store
>>> the 'type' and 'format' attributes, they need to be parsed manually
>>> prior to calling the function.
>>>
>>> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>>> ---
>>>  src/conf/domain_conf.c   | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>  src/conf/domain_conf.h   |  6 +++++
>>>  src/libvirt_private.syms |  1 +
>>>  3 files changed, 67 insertions(+)
>>
>> Perhaps you forgot to include next patch that uses
>> virDomainStorageSourceParseNew()? Because this patch does nothing more
>> than introducing a function that is never used.
> 
> Yes. The patch that uses this function was not posted yet. I'm sending
> these sub-series out so that I don't have to send 50+ patches at once.

Okay. ACK then.

Michal

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