[libvirt] [PATCHv3 05/43] snapshot: only pass snapshot to qemu command line when reverting

Eric Blake eblake at redhat.com
Wed Aug 24 15:22:22 UTC 2011


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   |    7 ++---
 src/qemu/qemu_driver.c    |   47 ++++----------------------------------------
 src/qemu/qemu_migration.c |    2 +-
 src/qemu/qemu_process.c   |   10 +-------
 src/qemu/qemu_process.h   |    1 +
 5 files changed, 12 insertions(+), 55 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 287ad8d..61fc2c7 100644
--- a/src/qemu/qemu_command.c
+++ b/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)
-        virCommandAddArgList(cmd, "-loadvm", current_snapshot->def->name,
-                             NULL);
+    if (snapshot)
+        virCommandAddArgList(cmd, "-loadvm", snapshot->def->name, NULL);

     if (def->namespaceData) {
         qemuDomainCmdlineDefPtr qemucmd;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d712bf2..c4183de 100644
--- a/src/qemu/qemu_driver.c
+++ b/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)
 {
@@ -8861,17 +8829,12 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
                 goto endjob;
             }
         } else {
-            if (qemuDomainSnapshotSetCurrentActive(vm, driver->snapshotDir) < 0)
-                goto endjob;
-
             rc = qemuProcessStart(snapshot->domain->conn, driver, vm, NULL,
-                                  true, false, -1, NULL, VIR_VM_OP_CREATE);
+                                  true, false, -1, NULL, vm->current_snapshot,
+                                  VIR_VM_OP_CREATE);
             virDomainAuditStart(vm, "from-snapshot", rc >= 0);
             if (rc < 0)
                 goto endjob;
-            if (qemuDomainSnapshotSetCurrentInactive(vm,
-                                                     driver->snapshotDir) < 0)
-                goto endjob;
         }

         /* Touch up domain state.  */
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index a84faf6..b88d9c0 100644
--- a/src/qemu/qemu_migration.c
+++ b/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
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 616c8e2..f691bbb 100644
--- a/src/qemu/qemu_process.c
+++ b/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,
-                                     vm->current_snapshot, vmop)))
+                                     migrateFrom, stdin_fd, 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);
diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
index e9b910d..96ba3f3 100644
--- a/src/qemu/qemu_process.h
+++ b/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,
-- 
1.7.4.4




More information about the libvir-list mailing list