There was only one caller, remove the unnecessary wrapper.

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

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 89e2900df2..db02005f5b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23716,38 +23716,45 @@ virDomainDiskSourceFormatPrivateData(virBufferPtr buf,
 }


-static int
-virDomainStorageSourceFormat(virBufferPtr attrBuf,
-                             virBufferPtr childBuf,
-                             virStorageSourcePtr src,
-                             unsigned int flags)
+int
+virDomainDiskSourceFormat(virBufferPtr buf,
+                          virStorageSourcePtr src,
+                          int policy,
+                          bool attrIndex,
+                          unsigned int flags,
+                          virDomainXMLOptionPtr xmlopt)
 {
+    VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
+    VIR_AUTOCLEAN(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)
             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));
         }

@@ -23761,7 +23768,7 @@ virDomainStorageSourceFormat(virBufferPtr attrBuf,
     }

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

     /* Storage Source formatting will not carry through the blunder
@@ -23771,38 +23778,17 @@ virDomainStorageSourceFormat(virBufferPtr attrBuf,
      * 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)
+        virStorageEncryptionFormat(&childBuf, src->encryption) < 0)
         return -1;

     if (src->pr)
-        virStoragePRDefFormat(childBuf, src->pr,
+        virStoragePRDefFormat(&childBuf, src->pr,
                               flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE);
-
-    return 0;
-}
-
-
-int
-virDomainDiskSourceFormat(virBufferPtr buf,
-                          virStorageSourcePtr src,
-                          int policy,
-                          bool attrIndex,
-                          unsigned int flags,
-                          virDomainXMLOptionPtr xmlopt)
-{
-    VIR_AUTOCLEAN(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
-    VIR_AUTOCLEAN(virBuffer) childBuf = VIR_BUFFER_INITIALIZER;
-
-    virBufferSetChildIndent(&childBuf, buf);
-
-    if (virDomainStorageSourceFormat(&attrBuf, &childBuf, src, flags) < 0)
-        return -1;
-
     if (policy && src->type != VIR_STORAGE_TYPE_NETWORK)
         virBufferEscapeString(&attrBuf, " startupPolicy='%s'",
                               virDomainStartupPolicyTypeToString(policy));
-- 
2.20.1

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

On Fri, Mar 22, 2019 at 07:00:49PM +0100, Peter Krempa wrote:
>There was only one caller, remove the unnecessary wrapper.
>
>Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>---
> src/conf/domain_conf.c | 60 ++++++++++++++++--------------------------
> 1 file changed, 23 insertions(+), 37 deletions(-)
>

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

On 3/22/19 1:00 PM, Peter Krempa wrote:
> There was only one caller, remove the unnecessary wrapper.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/conf/domain_conf.c | 60 ++++++++++++++++--------------------------
>  1 file changed, 23 insertions(+), 37 deletions(-)

Alas, my backup code wants to be a second caller.
https://www.redhat.com/archives/libvir-list/2019-March/msg00393.html

virDomainStorageSourceFormat was a nice independent function that did
not care what element it was being formatted to;
virDomainDiskSourceFormat always formats into <disk ....>.

But backup wants to format into <target> (push mode) or <scratch> (pull
mode). So I'll be including a revert of this patch in my next round of
incremental backup patches.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

On 4/4/19 3:16 PM, Eric Blake wrote:
> On 3/22/19 1:00 PM, Peter Krempa wrote:
>> There was only one caller, remove the unnecessary wrapper.
>>
>> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>> ---
>>  src/conf/domain_conf.c | 60 ++++++++++++++++--------------------------
>>  1 file changed, 23 insertions(+), 37 deletions(-)
> 
> Alas, my backup code wants to be a second caller.
> https://www.redhat.com/archives/libvir-list/2019-March/msg00393.html
> 
> virDomainStorageSourceFormat was a nice independent function that did
> not care what element it was being formatted to;
> virDomainDiskSourceFormat always formats into <disk ....>.
> 
> But backup wants to format into <target> (push mode) or <scratch> (pull
> mode). So I'll be including a revert of this patch in my next round of
> incremental backup patches.

Or maybe just tweaking it to add a parameter that says what string name
to use for the overall element.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

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

On Thu, Apr 04, 2019 at 15:29:30 -0500, Eric Blake wrote:
> On 4/4/19 3:16 PM, Eric Blake wrote:
> > On 3/22/19 1:00 PM, Peter Krempa wrote:
> >> There was only one caller, remove the unnecessary wrapper.
> >>
> >> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> >> ---
> >>  src/conf/domain_conf.c | 60 ++++++++++++++++--------------------------
> >>  1 file changed, 23 insertions(+), 37 deletions(-)
> > 
> > Alas, my backup code wants to be a second caller.
> > https://www.redhat.com/archives/libvir-list/2019-March/msg00393.html
> > 
> > virDomainStorageSourceFormat was a nice independent function that did
> > not care what element it was being formatted to;
> > virDomainDiskSourceFormat always formats into <disk ....>.
> > 
> > But backup wants to format into <target> (push mode) or <scratch> (pull
> > mode). So I'll be including a revert of this patch in my next round of
> > incremental backup patches.
> 
> Or maybe just tweaking it to add a parameter that says what string name
> to use for the overall element.

I prefer this one. If you need the element to be something else than
'source' please add a parameter.

Passing back a buffer was a somewhat failed experiment which allowed to
merge 'type' and 'format' attributes into the <source> (or equivalent)
element.

This design enforces users to add a wrapper element similarly to what
other XMLs are having. Original idea was to simplify thing but the
parser didn't turn out simpler at all and all the old XML design can't
be changed anyways.

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