[v3] snapshot: Store both config and live XML in the snapshot domain

[libvirt] [PATCH v3 1/2] qemu: formatting XML from domain def choosing the root name

Posted by Maxiwell S. Garcia 6 years, 5 months ago

The function virDomainDefFormatInternal() has the predefined root name
"domain" to format the XML. But to save both active and inactive domain
in the snapshot XML, the new root name "inactiveDomain" was created.
So, this function was modified to be driven by the new flag
VIR_DOMAIN_DEF_FORMAT_INACTIVE_NODE to choose the correct root name.

Signed-off-by: Maxiwell S. Garcia <maxiwell@linux.ibm.com>
---
 src/conf/domain_conf.c | 13 ++++++++++---
 src/conf/domain_conf.h |  1 +
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 03312afaaf..7d6393b9ac 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -28230,6 +28230,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
     unsigned char *uuid;
     char uuidstr[VIR_UUID_STRING_BUFLEN];
     const char *type = NULL;
+    const char *rootname = NULL;
     int n;
     size_t i;
     char *netprefix = NULL;
@@ -28238,7 +28239,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
                   VIR_DOMAIN_DEF_FORMAT_STATUS |
                   VIR_DOMAIN_DEF_FORMAT_ACTUAL_NET |
                   VIR_DOMAIN_DEF_FORMAT_PCI_ORIG_STATES |
-                  VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST,
+                  VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST |
+                  VIR_DOMAIN_DEF_FORMAT_INACTIVE_NODE,
                   -1);
 
     if (!(type = virDomainVirtTypeToString(def->virtType))) {
@@ -28250,7 +28252,12 @@ virDomainDefFormatInternal(virDomainDefPtr def,
     if (def->id == -1)
         flags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE;
 
-    virBufferAsprintf(buf, "<domain type='%s'", type);
+    if (flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE_NODE)
+        rootname = "inactiveDomain";
+    else
+        rootname = "domain";
+
+    virBufferAsprintf(buf, "<%s type='%s'", rootname, type);
     if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
         virBufferAsprintf(buf, " id='%d'", def->id);
     if (def->namespaceData && def->ns.href)
@@ -28732,7 +28739,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
     virDomainSEVDefFormat(buf, def->sev);
 
     virBufferAdjustIndent(buf, -2);
-    virBufferAddLit(buf, "</domain>\n");
+    virBufferAsprintf(buf, "</%s>\n", rootname);
 
     if (virBufferCheckError(buf) < 0)
         goto error;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bce47443c8..63791a8002 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2984,6 +2984,7 @@ typedef enum {
     VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM       = 1 << 6,
     VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT      = 1 << 7,
     VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST    = 1 << 8,
+    VIR_DOMAIN_DEF_FORMAT_INACTIVE_NODE   = 1 << 9,
 } virDomainDefFormatFlags;
 
 /* Use these flags to skip specific domain ABI consistency checks done
-- 
2.20.1

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

Re: [libvirt] [PATCH v3 1/2] qemu: formatting XML from domain def choosing the root name

Posted by Daniel Henrique Barboza 6 years, 5 months ago


On 8/14/19 11:47 AM, Maxiwell S. Garcia wrote:
> The function virDomainDefFormatInternal() has the predefined root name
> "domain" to format the XML. But to save both active and inactive domain
> in the snapshot XML, the new root name "inactiveDomain" was created.
> So, this function was modified to be driven by the new flag
> VIR_DOMAIN_DEF_FORMAT_INACTIVE_NODE to choose the correct root name.
>
> Signed-off-by: Maxiwell S. Garcia <maxiwell@linux.ibm.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   src/conf/domain_conf.c | 13 ++++++++++---
>   src/conf/domain_conf.h |  1 +
>   2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 03312afaaf..7d6393b9ac 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -28230,6 +28230,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>       unsigned char *uuid;
>       char uuidstr[VIR_UUID_STRING_BUFLEN];
>       const char *type = NULL;
> +    const char *rootname = NULL;
>       int n;
>       size_t i;
>       char *netprefix = NULL;
> @@ -28238,7 +28239,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>                     VIR_DOMAIN_DEF_FORMAT_STATUS |
>                     VIR_DOMAIN_DEF_FORMAT_ACTUAL_NET |
>                     VIR_DOMAIN_DEF_FORMAT_PCI_ORIG_STATES |
> -                  VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST,
> +                  VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST |
> +                  VIR_DOMAIN_DEF_FORMAT_INACTIVE_NODE,
>                     -1);
>   
>       if (!(type = virDomainVirtTypeToString(def->virtType))) {
> @@ -28250,7 +28252,12 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>       if (def->id == -1)
>           flags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE;
>   
> -    virBufferAsprintf(buf, "<domain type='%s'", type);
> +    if (flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE_NODE)
> +        rootname = "inactiveDomain";
> +    else
> +        rootname = "domain";
> +
> +    virBufferAsprintf(buf, "<%s type='%s'", rootname, type);
>       if (!(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
>           virBufferAsprintf(buf, " id='%d'", def->id);
>       if (def->namespaceData && def->ns.href)
> @@ -28732,7 +28739,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>       virDomainSEVDefFormat(buf, def->sev);
>   
>       virBufferAdjustIndent(buf, -2);
> -    virBufferAddLit(buf, "</domain>\n");
> +    virBufferAsprintf(buf, "</%s>\n", rootname);
>   
>       if (virBufferCheckError(buf) < 0)
>           goto error;
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index bce47443c8..63791a8002 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2984,6 +2984,7 @@ typedef enum {
>       VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM       = 1 << 6,
>       VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT      = 1 << 7,
>       VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST    = 1 << 8,
> +    VIR_DOMAIN_DEF_FORMAT_INACTIVE_NODE   = 1 << 9,
>   } virDomainDefFormatFlags;
>   
>   /* Use these flags to skip specific domain ABI consistency checks done

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

Re: [libvirt] [PATCH v3 1/2] qemu: formatting XML from domain def choosing the root name

Posted by Jiri Denemark 6 years, 5 months ago

On Wed, Aug 14, 2019 at 11:47:21 -0300, Maxiwell S. Garcia wrote:
> The function virDomainDefFormatInternal() has the predefined root name
> "domain" to format the XML. But to save both active and inactive domain
> in the snapshot XML, the new root name "inactiveDomain" was created.
> So, this function was modified to be driven by the new flag
> VIR_DOMAIN_DEF_FORMAT_INACTIVE_NODE to choose the correct root name.
> 
> Signed-off-by: Maxiwell S. Garcia <maxiwell@linux.ibm.com>
> ---
>  src/conf/domain_conf.c | 13 ++++++++++---
>  src/conf/domain_conf.h |  1 +
>  2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 03312afaaf..7d6393b9ac 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -28230,6 +28230,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>      unsigned char *uuid;
>      char uuidstr[VIR_UUID_STRING_BUFLEN];
>      const char *type = NULL;
> +    const char *rootname = NULL;
>      int n;
>      size_t i;
>      char *netprefix = NULL;
> @@ -28238,7 +28239,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>                    VIR_DOMAIN_DEF_FORMAT_STATUS |
>                    VIR_DOMAIN_DEF_FORMAT_ACTUAL_NET |
>                    VIR_DOMAIN_DEF_FORMAT_PCI_ORIG_STATES |
> -                  VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST,
> +                  VIR_DOMAIN_DEF_FORMAT_CLOCK_ADJUST |
> +                  VIR_DOMAIN_DEF_FORMAT_INACTIVE_NODE,
>                    -1);
>  
>      if (!(type = virDomainVirtTypeToString(def->virtType))) {
> @@ -28250,7 +28252,12 @@ virDomainDefFormatInternal(virDomainDefPtr def,
>      if (def->id == -1)
>          flags |= VIR_DOMAIN_DEF_FORMAT_INACTIVE;
>  
> -    virBufferAsprintf(buf, "<domain type='%s'", type);
> +    if (flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE_NODE)
> +        rootname = "inactiveDomain";
> +    else
> +        rootname = "domain";

This looks like an ugly hack. I suggest a bit different approach which
involves creating a new function. Just add a new parameter for the root
element name, rename this function, and create a new one with the
original name which would be a tiny wrapper around the renamed function
passing "domain" as the root element name.

This way you can keep existing code untouched and just call the new
function when you need a different root element name.

In other places we sometimes separate formatter for the internals from
the code formatting the container element. But since we need to add
attributes to the container element, such approach would be quite ugly
in this case.

Jirka

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

[libvirt] [PATCH v3 1/2] qemu: formatting XML from domain def choosing the root name
[libvirt] [PATCH v3 2/2] snapshot: Store both config and live XML in the snapshot domain