This new element will hold the new disk overlay created when reverting
to non-leaf snapshot in order to remember the files libvirt created.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/conf/schemas/domainsnapshot.rng |  7 +++++++
 src/conf/snapshot_conf.c            | 27 +++++++++++++++++++++++++++
 src/conf/snapshot_conf.h            |  5 +++++
 3 files changed, 39 insertions(+)

diff --git a/src/conf/schemas/domainsnapshot.rng b/src/conf/schemas/domainsnapshot.rng
index 45f01b96cd..2549c47b22 100644
--- a/src/conf/schemas/domainsnapshot.rng
+++ b/src/conf/schemas/domainsnapshot.rng
@@ -60,6 +60,13 @@
             </zeroOrMore>
           </element>
         </optional>
+        <optional>
+          <element name="revertDisks">
+            <zeroOrMore>
+              <ref name="disksnapshot"/>
+            </zeroOrMore>
+          </element>
+        </optional>
         <optional>
           <element name="active">
             <choice>
diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
index da1c694cb9..949104c1fd 100644
--- a/src/conf/snapshot_conf.c
+++ b/src/conf/snapshot_conf.c
@@ -376,6 +376,22 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt,
             return NULL;
     }
 
+    if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE) {
+        g_autofree xmlNodePtr *revertDiskNodes = NULL;
+
+        if ((n = virXPathNodeSet("./revertDisks/*", ctxt, &revertDiskNodes)) < 0)
+            return NULL;
+        if (n)
+            def->revertdisks = g_new0(virDomainSnapshotDiskDef, n);
+        def->nrevertdisks = n;
+        for (i = 0; i < def->nrevertdisks; i++) {
+            if (virDomainSnapshotDiskDefParseXML(revertDiskNodes[i], ctxt,
+                                                 &def->revertdisks[i],
+                                                 flags, xmlopt) < 0)
+                return NULL;
+        }
+    }
+
     if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL) {
         int active;
 
@@ -843,6 +859,17 @@ virDomainSnapshotDefFormatInternal(virBuffer *buf,
         virBufferAddLit(buf, "</disks>\n");
     }
 
+    if (def->nrevertdisks) {
+        virBufferAddLit(buf, "<revertDisks>\n");
+        virBufferAdjustIndent(buf, 2);
+        for (i = 0; i < def->nrevertdisks; i++) {
+            if (virDomainSnapshotDiskDefFormat(buf, &def->revertdisks[i], xmlopt) < 0)
+                return -1;
+        }
+        virBufferAdjustIndent(buf, -2);
+        virBufferAddLit(buf, "</revertDisks>\n");
+    }
+
     if (def->parent.dom) {
         if (virDomainDefFormatInternal(def->parent.dom, xmlopt,
                                        buf, domainflags) < 0)
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index ad49990a1e..ab76af604a 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -80,6 +80,11 @@ struct _virDomainSnapshotDef {
     size_t ndisks; /* should not exceed dom->ndisks */
     virDomainSnapshotDiskDef *disks;
 
+    /* When we revert to non-leaf snapshot we need to
+     * store the new overlay disks. */
+    size_t nrevertdisks;
+    virDomainSnapshotDiskDef *revertdisks;
+
     virObject *cookie;
 };
 
-- 
2.39.2

On Mon, Mar 13, 2023 at 16:42:05 +0100, Pavel Hrdina wrote:
> This new element will hold the new disk overlay created when reverting
> to non-leaf snapshot in order to remember the files libvirt created.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  src/conf/schemas/domainsnapshot.rng |  7 +++++++
>  src/conf/snapshot_conf.c            | 27 +++++++++++++++++++++++++++
>  src/conf/snapshot_conf.h            |  5 +++++
>  3 files changed, 39 insertions(+)

[...]

> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index da1c694cb9..949104c1fd 100644
> --- a/src/conf/snapshot_conf.c
> +++ b/src/conf/snapshot_conf.c

[...]

> @@ -843,6 +859,17 @@ virDomainSnapshotDefFormatInternal(virBuffer *buf,
>          virBufferAddLit(buf, "</disks>\n");
>      }
>  
> +    if (def->nrevertdisks) {

Always use explicit comparison against 0 for non-pointer, non-booleans.

> +        virBufferAddLit(buf, "<revertDisks>\n");
> +        virBufferAdjustIndent(buf, 2);
> +        for (i = 0; i < def->nrevertdisks; i++) {
> +            if (virDomainSnapshotDiskDefFormat(buf, &def->revertdisks[i], xmlopt) < 0)
> +                return -1;
> +        }
> +        virBufferAdjustIndent(buf, -2);
> +        virBufferAddLit(buf, "</revertDisks>\n");

Also preferrably use virXMLFormatElement.

> +    }
> +
>      if (def->parent.dom) {
>          if (virDomainDefFormatInternal(def->parent.dom, xmlopt,
>                                         buf, domainflags) < 0)

Reviewed-by: Peter Krempa <pkrempa@redhat.com>

On Mon, Apr 03, 2023 at 03:45:50PM +0200, Peter Krempa wrote:
> On Mon, Mar 13, 2023 at 16:42:05 +0100, Pavel Hrdina wrote:
> > This new element will hold the new disk overlay created when reverting
> > to non-leaf snapshot in order to remember the files libvirt created.
> > 
> > Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> > ---
> >  src/conf/schemas/domainsnapshot.rng |  7 +++++++
> >  src/conf/snapshot_conf.c            | 27 +++++++++++++++++++++++++++
> >  src/conf/snapshot_conf.h            |  5 +++++
> >  3 files changed, 39 insertions(+)
> 
> [...]
> 
> > diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> > index da1c694cb9..949104c1fd 100644
> > --- a/src/conf/snapshot_conf.c
> > +++ b/src/conf/snapshot_conf.c
> 
> [...]
> 
> > @@ -843,6 +859,17 @@ virDomainSnapshotDefFormatInternal(virBuffer *buf,
> >          virBufferAddLit(buf, "</disks>\n");
> >      }
> >  
> > +    if (def->nrevertdisks) {
> 
> Always use explicit comparison against 0 for non-pointer, non-booleans.
> 
> > +        virBufferAddLit(buf, "<revertDisks>\n");
> > +        virBufferAdjustIndent(buf, 2);
> > +        for (i = 0; i < def->nrevertdisks; i++) {
> > +            if (virDomainSnapshotDiskDefFormat(buf, &def->revertdisks[i], xmlopt) < 0)
> > +                return -1;
> > +        }
> > +        virBufferAdjustIndent(buf, -2);
> > +        virBufferAddLit(buf, "</revertDisks>\n");
> 
> Also preferrably use virXMLFormatElement.

I've basically copy&pasted what we do for the <disks> element. Will fix
it for this patch and post a separate patch fixing the other incorrect
comparisons and building xml.

> > +    }
> > +
> >      if (def->parent.dom) {
> >          if (virDomainDefFormatInternal(def->parent.dom, xmlopt,
> >                                         buf, domainflags) < 0)
> 
> Reviewed-by: Peter Krempa <pkrempa@redhat.com>