[libvirt] [PATCH 02/34] conf: Move formatting of 'index' and 'startupPolicy' for virStorageSource

Peter Krempa posted 34 patches 6 years, 10 months ago
There is a newer version of this series
[libvirt] [PATCH 02/34] conf: Move formatting of 'index' and 'startupPolicy' for virStorageSource
Posted by Peter Krempa 6 years, 10 months ago
Move the formatters from virDomainDiskSourceFormatInternal to
virDomainStorageSourceFormat.

While 'startupPolicy' is attribute of the disk, we can format it when
formating generic virStorageSource into an element to allow simplifying
the code.

Since the arguments for virDomainStorageSourceFormat got complex this
patch also documents them.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_conf.c | 44 ++++++++++++++++++++++++++++++------------
 src/conf/domain_conf.h |  5 ++++-
 src/qemu/qemu_domain.c |  3 ++-
 3 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 97d56c0067..4083839fc8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23755,12 +23755,32 @@ virDomainDiskSourceFormatPrivateData(virBufferPtr buf,
 }


+/**
+ * virDomainStorageSourceFormat:
+ * @attrBuf: buffer for containing attribute portion of @src
+ * @childBuf: buffer for subelements of the formatted element
+ * @src: storage source to format
+ * @flags: XML formatter flags
+ * @seclabels: security labels are formatted if true
+ * @attrIndex: the 'index' attribute is formatted if true
+ * @policy: startup policy, taken from disk (use 0 to omit)
+ * @xmlopt: XML options data (for private data formatters)
+ *
+ * Formats @src into the attributes (@attrBuf) and subelements (@childBuf) ready
+ * for creating a full XML element representing @src.
+ *
+ * Note that this does _not_ format the 'type' and 'format' of @src due to
+ * differences in callers.
+ */
 int
 virDomainStorageSourceFormat(virBufferPtr attrBuf,
                              virBufferPtr childBuf,
                              virStorageSourcePtr src,
                              unsigned int flags,
-                             bool seclabels)
+                             bool seclabels,
+                             bool attrIndex,
+                             int policy,
+                             virDomainXMLOptionPtr xmlopt)
 {
     switch ((virStorageType)src->type) {
     case VIR_STORAGE_TYPE_FILE:
@@ -23823,6 +23843,16 @@ virDomainStorageSourceFormat(virBufferPtr attrBuf,
         virStoragePRDefFormat(childBuf, src->pr,
                               flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE);

+    if (policy && src->type != VIR_STORAGE_TYPE_NETWORK)
+        virBufferEscapeString(attrBuf, " startupPolicy='%s'",
+                              virDomainStartupPolicyTypeToString(policy));
+
+    if (attrIndex && src->id != 0)
+        virBufferAsprintf(attrBuf, " index='%u'", src->id);
+
+    if (virDomainDiskSourceFormatPrivateData(childBuf, src, flags, xmlopt) < 0)
+        return -1;
+
     return 0;
 }

@@ -23843,17 +23873,7 @@ virDomainDiskSourceFormatInternal(virBufferPtr buf,
     virBufferSetChildIndent(&childBuf, buf);

     if (virDomainStorageSourceFormat(&attrBuf, &childBuf, src, flags,
-                                     seclabels) < 0)
-        goto cleanup;
-
-    if (policy && src->type != VIR_STORAGE_TYPE_NETWORK)
-        virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
-                              virDomainStartupPolicyTypeToString(policy));
-
-    if (attrIndex && src->id != 0)
-        virBufferAsprintf(&attrBuf, " index='%u'", src->id);
-
-    if (virDomainDiskSourceFormatPrivateData(&childBuf, src, flags, xmlopt) < 0)
+                                     seclabels, attrIndex, policy, xmlopt) < 0)
         goto cleanup;

     if (virXMLFormatElement(buf, "source", &attrBuf, &childBuf) < 0)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index cad330715b..a6f8e13088 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -3461,7 +3461,10 @@ int virDomainStorageSourceFormat(virBufferPtr attrBuf,
                                  virBufferPtr childBuf,
                                  virStorageSourcePtr src,
                                  unsigned int flags,
-                                 bool seclabels)
+                                 bool seclabels,
+                                 bool attrIndex,
+                                 int policy,
+                                 virDomainXMLOptionPtr xmlopt)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);

 int virDomainStorageSourceParse(xmlNodePtr node,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 341ea7d37c..d25d9ab666 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2358,7 +2358,8 @@ qemuDomainObjPrivateXMLFormatNBDMigrationSource(virBufferPtr buf,
                       virStorageFileFormatTypeToString(src->format));

     if (virDomainStorageSourceFormat(&attrBuf, &childBuf, src,
-                                     VIR_DOMAIN_DEF_FORMAT_STATUS, true) < 0)
+                                     VIR_DOMAIN_DEF_FORMAT_STATUS, true,
+                                     false, 0, NULL) < 0)
         goto cleanup;

     if (qemuStorageSourcePrivateDataFormat(src, &privateDataBuf) < 0)
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 02/34] conf: Move formatting of 'index' and 'startupPolicy' for virStorageSource
Posted by Ján Tomko 6 years, 10 months ago
On Mon, Mar 18, 2019 at 04:54:51PM +0100, Peter Krempa wrote:
>Move the formatters from virDomainDiskSourceFormatInternal to
>virDomainStorageSourceFormat.
>
>While 'startupPolicy' is attribute of the disk, we can format it when
>formating generic virStorageSource into an element to allow simplifying
>the code.

All but one callers pass 0, is it really worth putting a non-source
attribute here?

>
>Since the arguments for virDomainStorageSourceFormat got complex this
>patch also documents them.
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/conf/domain_conf.c | 44 ++++++++++++++++++++++++++++++------------
> src/conf/domain_conf.h |  5 ++++-
> src/qemu/qemu_domain.c |  3 ++-
> 3 files changed, 38 insertions(+), 14 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 97d56c0067..4083839fc8 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -23755,12 +23755,32 @@ virDomainDiskSourceFormatPrivateData(virBufferPtr buf,
> }
>
>
>+/**
>+ * virDomainStorageSourceFormat:
>+ * @attrBuf: buffer for containing attribute portion of @src
>+ * @childBuf: buffer for subelements of the formatted element
>+ * @src: storage source to format
>+ * @flags: XML formatter flags
>+ * @seclabels: security labels are formatted if true
>+ * @attrIndex: the 'index' attribute is formatted if true
>+ * @policy: startup policy, taken from disk (use 0 to omit)
>+ * @xmlopt: XML options data (for private data formatters)
>+ *
>+ * Formats @src into the attributes (@attrBuf) and subelements (@childBuf) ready
>+ * for creating a full XML element representing @src.
>+ *
>+ * Note that this does _not_ format the 'type' and 'format' of @src due to
>+ * differences in callers.

different needs of callers.

>+ */
> int

Reviewed-by: Ján Tomko <jtomko@redhat.com>

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