qemuSnapshotCreateActiveExternal: Don't thaw filesystems when freeze fails

If we didn't freeze any filesystems we should not even attempt thawing
them. Additionally 'guest-fsfreeze-freeze' fails if the filesystems are
already frozen, where thawing them may break users data integrity if
they used VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE accidentally after an
explicit virDomainFSFreeze and the next snapshot without that flag would
be taken with already thawed filesystems.

This effectively reverts 7c736bab06479ccec59df69fb79a5c06d112d8fb .
Libvirt nowadays checks whether the guest agent is connected and pings
it before issuing an command so it's very unlikely that we'd end up in a
situation where qemuSnapshotCreateActiveExternal froze filesystems and
didn't thaw them.

Additionally we now discourage the use of
VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE since users have better control if
they freeze the FS themselves.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c
index 115c2fc91bf18c31f5e61d3ad60879c2ff684d76..eacc8c6400c6d877abe87a781535ae979d105673 100644 (file)
--- a/src/qemu/qemu_snapshot.c
+++ b/src/qemu/qemu_snapshot.c
@@ -1403,7 +1403,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver,
     virDomainSnapshotDefPtr snapdef = virDomainSnapshotObjGetDef(snap);
     bool memory = snapdef->memory == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL;
     bool memory_unlink = false;
-    int thaw = 0; /* 1 if freeze succeeded, -1 if freeze failed */
+    bool thaw = false;
     bool pmsuspended = false;
     int compressed;
     g_autoptr(virCommand) compressor = NULL;
@@ -1415,7 +1415,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver,
      * The command will fail if the guest is paused or the guest agent
      * is not running, or is already quiesced.  */
     if (flags & VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) {
-        int freeze;
+        int frozen;
 
         if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_MODIFY) < 0)
             goto cleanup;
@@ -1425,16 +1425,14 @@ qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver,
             goto cleanup;
         }
 
-        freeze = qemuSnapshotFSFreeze(vm, NULL, 0);
+        frozen = qemuSnapshotFSFreeze(vm, NULL, 0);
         qemuDomainObjEndAgentJob(vm);
 
-        if (freeze < 0) {
-            /* the helper reported the error */
-            if (freeze == -2)
-                thaw = -1; /* the command is sent but agent failed */
+        if (frozen < 0)
             goto cleanup;
-        }
-        thaw = 1;
+
+        if (frozen > 0)
+            thaw = true;
     }
 
     /* We need to track what state the guest is in, since taking the
@@ -1527,7 +1525,7 @@ qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver,
                         QEMU_ASYNC_JOB_SNAPSHOT, 0);
         virDomainAuditStop(vm, "from-snapshot");
         resume = false;
-        thaw = 0;
+        thaw = false;
         virObjectEventStateQueue(driver->domainEventState, event);
     } else if (memory && pmsuspended) {
         /* qemu 1.3 is unable to save a domain in pm-suspended (S3)
@@ -1559,14 +1557,12 @@ qemuSnapshotCreateActiveExternal(virQEMUDriverPtr driver,
         ret = -1;
     }
 
-    if (thaw != 0 &&
+    if (thaw &&
         qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_MODIFY) >= 0 &&
         virDomainObjIsActive(vm)) {
-        if (qemuSnapshotFSThaw(vm, ret == 0 && thaw > 0) < 0) {
-            /* helper reported the error, if it was needed */
-            if (thaw > 0)
-                ret = -1;
-        }
+        /* report error only on an otherwise successful snapshot */
+        if (qemuSnapshotFSThaw(vm, ret == 0) < 0)
+            ret = -1;
 
         qemuDomainObjEndAgentJob(vm);
     }