snapshot: only pass snapshot to qemu command line when reverting

Changing the current vm, and writing that change to the file
system, all before a new qemu starts, is risky; it's hard to
roll back if starting the new qemu fails for some reason.
Instead of abusing vm->current_snapshot and making the command
line generator decide whether the current snapshot warrants
using -loadvm, it is better to just directly pass a snapshot all
the way through the call chain if it is to be loaded.

This frees up the last use of snapshot->def->active for qemu's
use, so the next patch can repurpose that field for tracking
which snapshot is current.

* src/qemu/qemu_command.c (qemuBuildCommandLine): Don't use active
field of snapshot.
* src/qemu/qemu_process.c (qemuProcessStart): Add a parameter.
* src/qemu/qemu_process.h (qemuProcessStart): Update prototype.
* src/qemu/qemu_migration.c (qemuMigrationPrepareAny): Update
callers.
* src/qemu/qemu_driver.c (qemudDomainCreate)
(qemuDomainSaveImageStartVM, qemuDomainObjStart)
(qemuDomainRevertToSnapshot): Likewise.
(qemuDomainSnapshotSetCurrentActive)
(qemuDomainSnapshotSetCurrentInactive): Delete unused functions.

src/qemu/qemu_command.c

@@ -2866,7 +2866,7 @@ qemuBuildCommandLine(virConnectPtr conn,
                      virBitmapPtr qemuCaps,
                      const char *migrateFrom,
                      int migrateFd,
-                     virDomainSnapshotObjPtr current_snapshot,
+                     virDomainSnapshotObjPtr snapshot,
                      enum virVMOperationType vmop)
 {
     int i;
@@ -4782,9 +4782,8 @@ qemuBuildCommandLine(virConnectPtr conn,
         }
     }
 
-    if (current_snapshot && current_snapshot->def->active)
+    if (snapshot)
-        virCommandAddArgList(cmd, "-loadvm", current_snapshot->def->name,
+        virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL);
-                             NULL);
 
     if (def->namespaceData) {
         qemuDomainCmdlineDefPtr qemucmd;

src/qemu/qemu_driver.c

@@ -804,12 +804,6 @@ qemudShutdown(void) {
 }
 
 
-static int qemuDomainSnapshotSetCurrentActive(virDomainObjPtr vm,
-                                              char *snapshotDir);
-static int qemuDomainSnapshotSetCurrentInactive(virDomainObjPtr vm,
-                                                char *snapshotDir);
-
-
 static virDrvOpenStatus qemudOpen(virConnectPtr conn,
                                   virConnectAuthPtr auth ATTRIBUTE_UNUSED,
                                   unsigned int flags)
@@ -1297,7 +1291,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn, const char *xml,
     if (qemuProcessStart(conn, driver, vm, NULL,
                          (flags & VIR_DOMAIN_START_PAUSED) != 0,
                          (flags & VIR_DOMAIN_START_AUTODESTROY) != 0,
-                         -1, NULL, VIR_VM_OP_CREATE) < 0) {
+                         -1, NULL, NULL, VIR_VM_OP_CREATE) < 0) {
         virDomainAuditStart(vm, "booted", false);
         if (qemuDomainObjEndJob(driver, vm) > 0)
             virDomainRemoveInactive(&driver->domains,
@@ -3920,7 +3914,7 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
 
     /* Set the migration source and start it up. */
     ret = qemuProcessStart(conn, driver, vm, "stdio", true,
-                           false, *fd, path, VIR_VM_OP_RESTORE);
+                           false, *fd, path, NULL, VIR_VM_OP_RESTORE);
 
     if (intermediatefd != -1) {
         if (ret < 0) {
@@ -4462,7 +4456,7 @@ qemuDomainObjStart(virConnectPtr conn,
     }
 
     ret = qemuProcessStart(conn, driver, vm, NULL, start_paused,
-                           autodestroy, -1, NULL, VIR_VM_OP_CREATE);
+                           autodestroy, -1, NULL, NULL, VIR_VM_OP_CREATE);
     virDomainAuditStart(vm, "booted", ret >= 0);
     if (ret >= 0) {
         virDomainEventPtr event =
@@ -8318,32 +8312,6 @@ cleanup:
     return ret;
 }
 
-static int qemuDomainSnapshotSetCurrentActive(virDomainObjPtr vm,
-                                              char *snapshotDir)
-{
-    if (vm->current_snapshot) {
-        vm->current_snapshot->def->active = 1;
-
-        return qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot,
-                                               snapshotDir);
-    }
-
-    return 0;
-}
-
-static int qemuDomainSnapshotSetCurrentInactive(virDomainObjPtr vm,
-                                                char *snapshotDir)
-{
-    if (vm->current_snapshot) {
-        vm->current_snapshot->def->active = 0;
-
-        return qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot,
-                                               snapshotDir);
-    }
-
-    return 0;
-}
-
 
 static int qemuDomainSnapshotIsAllowed(virDomainObjPtr vm)
 {
@@ -8790,15 +8758,10 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
             if (rc < 0)
                 goto endjob;
         } else {
-            if (qemuDomainSnapshotSetCurrentActive(vm, driver->snapshotDir) < 0)
-                goto endjob;
-
             rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
-                                  false, false, -1, NULL, VIR_VM_OP_CREATE);
+                                  false, false, -1, NULL, vm->current_snapshot,
+                                  VIR_VM_OP_CREATE);
             virDomainAuditStart(vm, "from-snapshot", rc >= 0);
-            if (qemuDomainSnapshotSetCurrentInactive(vm,
-                                                     driver->snapshotDir) < 0)
-                goto endjob;
             if (rc < 0)
                 goto endjob;
         }
@@ -8845,9 +8808,6 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
                 goto cleanup;
             }
         }
-
-        if (qemuDomainSnapshotSetCurrentActive(vm, driver->snapshotDir) < 0)
-            goto endjob;
     }
 
     ret = 0;

src/qemu/qemu_migration.c

@@ -1034,7 +1034,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
      * -incoming $migrateFrom
      */
     if (qemuProcessStart(dconn, driver, vm, migrateFrom, true,
-                         true, dataFD[0], NULL,
+                         true, dataFD[0], NULL, NULL,
                          VIR_VM_OP_MIGRATE_IN_START) < 0) {
         virDomainAuditStart(vm, "migrated", false);
         /* Note that we don't set an error here because qemuProcessStart

src/qemu/qemu_process.c

@@ -2614,6 +2614,7 @@ int qemuProcessStart(virConnectPtr conn,
                      bool autodestroy,
                      int stdin_fd,
                      const char *stdin_path,
+                     virDomainSnapshotObjPtr snapshot,
                      enum virVMOperationType vmop)
 {
     int ret;
@@ -2816,16 +2817,9 @@ int qemuProcessStart(virConnectPtr conn,
     VIR_DEBUG("Building emulator command line");
     if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig,
                                      priv->monJSON != 0, priv->qemuCaps,
-                                     migrateFrom, stdin_fd,
+                                     migrateFrom, stdin_fd, snapshot, vmop)))
-                                     vm->current_snapshot, vmop)))
         goto cleanup;
 
-#if 0
-    /* XXX */
-    if (qemuDomainSnapshotSetCurrentInactive(vm, driver->snapshotDir) < 0)
-        goto cleanup;
-#endif
-
     /* now that we know it is about to start call the hook if present */
     if (virHookPresent(VIR_HOOK_DRIVER_QEMU)) {
         char *xml = virDomainDefFormat(vm->def, 0);

src/qemu/qemu_process.h

@@ -52,6 +52,7 @@ int qemuProcessStart(virConnectPtr conn,
                      bool autodestroy,
                      int stdin_fd,
                      const char *stdin_path,
+                     virDomainSnapshotObjPtr snapshot,
                      enum virVMOperationType vmop);
 
 void qemuProcessStop(struct qemud_driver *driver,