[libvirt RFC 23/24] qemu_snapshot: when deleting snapshot invalidate parent snapshot

[Peter Krempa]

On Tue, Aug 23, 2022 at 18:32:26 +0200, Pavel Hrdina wrote:
> When deleting external snapshots the operation may fail at any point
> which could lead to situation that some disks finished the block commit
> operation but for some disks it failed and the libvirt job ends.
> 
> In order to make sure that the qcow2 images are in consistent state
> introduce new element "<invalid/>" that will mark the disk in snapshot
> metadata as invalid until the snapshot delete is completed successfully.
> 
> This will prevent deleting snapshot with the invalid disk and in future
> reverting to snapshot with the invalid disk.
> 
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  src/conf/snapshot_conf.c |  5 +++++
>  src/conf/snapshot_conf.h |  1 +
>  src/qemu/qemu_snapshot.c | 42 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 48 insertions(+)
> 
> diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> index ae635edd08..155da42862 100644
> --- a/src/conf/snapshot_conf.c
> +++ b/src/conf/snapshot_conf.c
> @@ -158,6 +158,8 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node,
>          return -1;
>      }
>  
> +    def->invalid = !!virXPathNode("./invalid", ctxt);

You shouldn't allow parsing this unless a snapshot is being
redefined/reloaded from disk.

> +
>      if ((cur = virXPathNode("./source", ctxt)) &&
>          virDomainStorageSourceParse(cur, ctxt, src, flags, xmlopt) < 0)
>          return -1;

Once again please make sure to document non-obvious behaviour.

In this patch it's specifically where the disks of the _parent_ snapshot
are marked as invalid. I've overlooked that first.