From 4c96d5344961e60ef8b0150192fa2a5c2c8ca960 Mon Sep 17 00:00:00 2001

Message-Id: <4c96d5344961e60ef8b0150192fa2a5c2c8ca960@dist-git>

From: Jincheng Miao <jmiao@redhat.com>

Date: Mon, 8 Sep 2014 13:24:18 +0200

Subject: [PATCH] qemu: snapshot: Acquire job earlier on snapshot revert/delete

https://bugzilla.redhat.com/show_bug.cgi?id=1134154

The code would lookup the snapshot object before acquiring the job. This

could lead to a crash as one thread could delete the snapshot object,

while a second thread already had the reference.

Signed-off-by: Jincheng Miao <jmiao@redhat.com>

Signed-off-by: Peter Krempa <pkrempa@redhat.com>

(cherry picked from commit a4065dc3e7b17a5fa6916fca2fb28a0d723fe13d)

Signed-off-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/qemu/qemu_driver.c | 30 +++++++++++++++---------------

 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c

index 6a79f28..fb4b508 100644

--- a/src/qemu/qemu_driver.c

+++ b/src/qemu/qemu_driver.c

@@ -14018,9 +14018,12 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,

         goto cleanup;

     }

-    if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))

+    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)

         goto cleanup;

+    if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))

+        goto endjob;

+

     if (!vm->persistent &&

         snap->def->state != VIR_DOMAIN_RUNNING &&

         snap->def->state != VIR_DOMAIN_PAUSED &&

@@ -14029,13 +14032,13 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,

         virReportError(VIR_ERR_OPERATION_INVALID, "%s",

                        _("transient domain needs to request run or pause "

                          "to revert to inactive snapshot"));

-        goto cleanup;

+        goto endjob;

     }

     if (virDomainSnapshotIsExternal(snap)) {

         virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",

                        _("revert to external snapshot not supported yet"));

-        goto cleanup;

+        goto endjob;

     }

     if (!(flags & VIR_DOMAIN_SNAPSHOT_REVERT_FORCE)) {

@@ -14043,7 +14046,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,

             virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY,

                            _("snapshot '%s' lacks domain '%s' rollback info"),

                            snap->def->name, vm->def->name);

-            goto cleanup;

+            goto endjob;

         }

         if (virDomainObjIsActive(vm) &&

             !(snap->def->state == VIR_DOMAIN_RUNNING

@@ -14052,7 +14055,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,

                       VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED))) {

             virReportError(VIR_ERR_SNAPSHOT_REVERT_RISKY, "%s",

                            _("must respawn qemu to start inactive snapshot"));

-            goto cleanup;

+            goto endjob;

         }

     }

@@ -14061,7 +14064,7 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,

         vm->current_snapshot->def->current = false;

         if (qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot,

                                             cfg->snapshotDir) < 0)

-            goto cleanup;

+            goto endjob;

         vm->current_snapshot = NULL;

         /* XXX Should we restore vm->current_snapshot after this point

          * in the failure cases where we know there was no change?  */

@@ -14076,12 +14079,9 @@ static int qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,

     if (snap->def->dom) {

         config = virDomainDefCopy(snap->def->dom, caps, driver->xmlopt, true);

         if (!config)

-            goto cleanup;

+            goto endjob;

     }

-    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)

-        goto cleanup;

-

     switch ((virDomainState) snap->def->state) {

     case VIR_DOMAIN_RUNNING:

     case VIR_DOMAIN_PAUSED:

@@ -14391,9 +14391,12 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,

     if (virDomainSnapshotDeleteEnsureACL(snapshot->domain->conn, vm->def) < 0)

         goto cleanup;

-    if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))

+    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)

         goto cleanup;

+    if (!(snap = qemuSnapObjFromSnapshot(vm, snapshot)))

+        goto endjob;

+

     if (!metadata_only) {

         if (!(flags & VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY) &&

             virDomainSnapshotIsExternal(snap))

@@ -14406,13 +14409,10 @@ static int qemuDomainSnapshotDelete(virDomainSnapshotPtr snapshot,

             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,

                            _("deletion of %d external disk snapshots not "

                              "supported yet"), external);

-            goto cleanup;

+            goto endjob;

         }

     }

-    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)

-        goto cleanup;

-

     if (flags & (VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN |

                  VIR_DOMAIN_SNAPSHOT_DELETE_CHILDREN_ONLY)) {

         rem.driver = driver;

-- 

2.1.0