[libvirt] [PATCH 1/8] Fix up the locking in the snapshot code.

[Chris Lalancette <clalance@xxxxxxxxxx>]

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@xxxxxxxxxx>
---
 src/qemu/qemu_driver.c |   45 +++++++++++++++++++++++++++++++--------------
 1 files changed, 31 insertions(+), 14 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1ef15dd..2f889d9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10690,7 +10690,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;
 
@@ -10757,14 +10756,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;
@@ -11037,18 +11041,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) {
@@ -11060,7 +11064,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
             rc = qemuMonitorStopCPUs(priv->mon);
             qemuDomainObjExitMonitorWithDriver(driver, vm);
             if (rc < 0)
-                goto cleanup;
+                goto endjob;
         }
 
         event = virDomainEventNewFromObj(vm,
@@ -11083,20 +11087,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)
@@ -11250,6 +11260,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;
@@ -11258,11 +11271,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);
-- 
1.6.6.1

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list