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@xxxxxxxxxx>
> ---
> 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.
