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.