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

[Pavel Hrdina]

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);
+
     if ((cur = virXPathNode("./source", ctxt)) &&
         virDomainStorageSourceParse(cur, ctxt, src, flags, xmlopt) < 0)
         return -1;
@@ -761,6 +763,9 @@ virDomainSnapshotDiskDefFormat(virBuffer *buf,
         virBufferAsprintf(&attrBuf, " snapshot='%s'",
                           virDomainSnapshotLocationTypeToString(disk->snapshot));
 
+    if (disk->invalid)
+        virBufferAddLit(&childBuf, "<invalid/>\n");
+
     if (disk->src->path || disk->src->format != 0) {
         g_auto(virBuffer) driverAttrBuf = VIR_BUFFER_INITIALIZER;
         g_auto(virBuffer) driverChildBuf = VIR_BUFFER_INIT_CHILD(&childBuf);
diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h
index 1f787f1a94..de6e420f77 100644
--- a/src/conf/snapshot_conf.h
+++ b/src/conf/snapshot_conf.h
@@ -52,6 +52,7 @@ typedef struct _virDomainSnapshotDiskDef virDomainSnapshotDiskDef;
 struct _virDomainSnapshotDiskDef {
     char *name;     /* name matching the <target dev='...' of the domain */
     virDomainSnapshotLocation snapshot;
+    bool invalid;
 
     /* details of wrapper external file. src is always non-NULL.
      * XXX optimize this to allow NULL for internal snapshots? */
diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 37ae3f04d0..29d147f834 100644
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -2288,6 +2288,12 @@ qemuSnapshotDeleteExternalPrepare(virDomainObj *vm,
         if (snapDisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NO)
             continue;
 
+        if (snapDisk->invalid) {
+            virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+                           _("snapshot is in invalid state"));
+            return NULL;
+        }
+
         data = g_new0(qemuSnapshotDeleteExternalData, 1);
         data->snapDisk = snapDisk;
 
@@ -2522,6 +2528,36 @@ qemuSnapshotJobFinishing(virDomainObj *vm,
 }
 
 
+static int
+qemuSnapshotSetInvalid(virDomainObj *vm,
+                       virDomainMomentObj *snap,
+                       virDomainSnapshotDiskDef *disk,
+                       bool invalid)
+{
+    ssize_t i;
+    qemuDomainObjPrivate *priv = vm->privateData;
+    virQEMUDriver *driver = priv->driver;
+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+    virDomainSnapshotDef *snapdef = NULL;
+
+    if (!snap)
+        return 0;
+
+    snapdef = virDomainSnapshotObjGetDef(snap);
+    if (!snapdef)
+        return 0;
+
+    for (i = 0; i < snapdef->ndisks; i++) {
+        virDomainSnapshotDiskDef *snapDisk = &(snapdef->disks[i]);
+
+        if (STREQ(snapDisk->name, disk->name))
+            snapDisk->invalid = invalid;
+    }
+
+    return qemuDomainSnapshotWriteMetadata(vm, snap, driver->xmlopt, cfg->snapshotDir);
+}
+
+
 static int
 qemuSnapshotDiscardExternal(virDomainObj *vm,
                             virQEMUDriver *driver,
@@ -2539,6 +2575,9 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
             autofinalize = VIR_TRISTATE_BOOL_YES;
         }
 
+        if (qemuSnapshotSetInvalid(vm, data->parentSnap, data->snapDisk, true) < 0)
+            return -1;
+
         if (qemuBlockCommitImpl(vm, driver,
                                 data->domDisk,
                                 data->parentDiskSrc,
@@ -2585,6 +2624,9 @@ qemuSnapshotDiscardExternal(virDomainObj *vm,
             return -1;
 
         qemuBlockJobSyncEnd(vm, data->job, VIR_ASYNC_JOB_NONE);
+
+        if (qemuSnapshotSetInvalid(vm, data->parentSnap, data->snapDisk, false) < 0)
+            return -1;
     }
 
     return 0;
-- 
2.37.2