From b69bbebbba543b5c3bb190815d0074a576aa4966 Mon Sep 17 00:00:00 2001
From: Chris Lalancette <clalance@redhat.com>
Date: Thu, 22 Apr 2010 12:01:56 -0400
Subject: [PATCH] Fix up the locking in the snapshot code.

In particular I was forgetting to take the qemuMonitorPrivatePtr
lock (via qemuDomainObjBeginJob), which would cause problems
if two users tried to access the same domain at the same time.
This patch also fixes a problem where I was forgetting to remove
a transient domain from the list of domains.

Thanks to Stephen Shaw for pointing out the problem and testing
out the initial patch.

Signed-off-by: Chris Lalancette <clalance@redhat.com>
---
 src/qemu/qemu_driver.c | 45 +++++++++++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8ccf0b7d31..e1b1af3d7c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10700,7 +10700,6 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
     virDomainSnapshotPtr snapshot = NULL;
     char uuidstr[VIR_UUID_STRING_BUFLEN];
     virDomainSnapshotDefPtr def;
-    qemuDomainObjPrivatePtr priv;
     const char *qemuimgarg[] = { NULL, "snapshot", "-c", NULL, NULL, NULL };
     int i;
 
@@ -10767,14 +10766,19 @@ static virDomainSnapshotPtr qemuDomainSnapshotCreateXML(virDomainPtr domain,
         }
     }
     else {
+        qemuDomainObjPrivatePtr priv;
+        int ret;
+
+        if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
+            goto cleanup;
         priv = vm->privateData;
         qemuDomainObjEnterMonitorWithDriver(driver, vm);
-        if (qemuMonitorCreateSnapshot(priv->mon, def->name) < 0) {
-            qemuDomainObjExitMonitorWithDriver(driver, vm);
-            goto cleanup;
-        }
+        ret = qemuMonitorCreateSnapshot(priv->mon, def->name);
         qemuDomainObjExitMonitorWithDriver(driver, vm);
-
+        if (qemuDomainObjEndJob(vm) == 0)
+            vm = NULL;
+        if (ret < 0)
+            goto cleanup;
     }
 
     snap->def->state = vm->state;
@@ -11047,18 +11051,18 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
             rc = qemuMonitorLoadSnapshot(priv->mon, snap->def->name);
             qemuDomainObjExitMonitorWithDriver(driver, vm);
             if (rc < 0)
-                goto cleanup;
+                goto endjob;
         }
         else {
             if (qemuDomainSnapshotSetActive(vm, driver->snapshotDir) < 0)
-                goto cleanup;
+                goto endjob;
 
             rc = qemudStartVMDaemon(snapshot->domain->conn, driver, vm, NULL,
                                     -1);
             if (qemuDomainSnapshotSetInactive(vm, driver->snapshotDir) < 0)
-                goto cleanup;
+                goto endjob;
             if (rc < 0)
-                goto cleanup;
+                goto endjob;
         }
 
         if (snap->def->state == VIR_DOMAIN_PAUSED) {
@@ -11073,7 +11077,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
             qemuDomainObjExitMonitorWithDriver(driver, vm);
             if (rc < 0) {
                 vm->state = state;
-                goto cleanup;
+                goto endjob;
             }
         }
 
@@ -11097,20 +11101,26 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
             event = virDomainEventNewFromObj(vm,
                                              VIR_DOMAIN_EVENT_STOPPED,
                                              VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT);
+            if (!vm->persistent) {
+                if (qemuDomainObjEndJob(vm) > 0)
+                    virDomainRemoveInactive(&driver->domains, vm);
+                vm = NULL;
+            }
         }
 
         if (qemuDomainSnapshotSetActive(vm, driver->snapshotDir) < 0)
-            goto cleanup;
+            goto endjob;
     }
 
     vm->state = snap->def->state;
 
     ret = 0;
 
-cleanup:
+endjob:
     if (vm && qemuDomainObjEndJob(vm) == 0)
         vm = NULL;
 
+cleanup:
     if (event)
         qemuDomainEventQueue(driver, event);
     if (vm)
@@ -11264,6 +11274,9 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
         goto cleanup;
     }
 
+    if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
+        goto cleanup;
+
     if (flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN) {
         rem.driver = driver;
         rem.vm = vm;
@@ -11272,11 +11285,15 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,
         virHashForEach(vm->snapshots.objs, qemuDomainSnapshotDiscardChildren,
                        &rem);
         if (rem.err < 0)
-            goto cleanup;
+            goto endjob;
     }
 
     ret = qemuDomainSnapshotDiscard(driver, vm, snap);
 
+endjob:
+    if (qemuDomainObjEndJob(vm) == 0)
+        vm = NULL;
+
 cleanup:
     if (vm)
         virDomainObjUnlock(vm);
-- 
GitLab