From 3c89868c5fef3d0cfbc40d0185447d13a6242620 Mon Sep 17 00:00:00 2001
From: Jim Fehlig <jfehlig@suse.com>
Date: Fri, 16 Mar 2018 15:15:07 -0600
Subject: [PATCH] libxl: lock virDomainObj after ListRemove

Most libxl driver API use the pattern of lock and add a ref to
virDomainObj, perform API, then decrement ref and unlock in
virDomainEndAPI. In some cases the API may call
virDomainObjListRemove, which unlocks the virDomainObj. Relock
the object in such cases so EndAPI is called with a locked object.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: John Ferlan <jferlan@redhat.com>
---
 src/libxl/libxl_driver.c    | 22 +++++++++++++---------
 src/libxl/libxl_migration.c |  7 +++++--
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 89432d03c6..ac61fb3221 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1056,7 +1056,7 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml,
     if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) {
         if (!vm->persistent) {
             virDomainObjListRemove(driver->domains, vm);
-            vm = NULL;
+            virObjectLock(vm);
         }
         goto cleanup;
     }
@@ -1065,7 +1065,7 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml,
                          (flags & VIR_DOMAIN_START_PAUSED) != 0) < 0) {
         if (!vm->persistent) {
             virDomainObjListRemove(driver->domains, vm);
-            vm = NULL;
+            virObjectLock(vm);
             goto cleanup;
         }
         goto endjob;
@@ -1417,8 +1417,10 @@ libxlDomainDestroyFlags(virDomainPtr dom,
                                      VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
 
     libxlDomainCleanup(driver, vm);
-    if (!vm->persistent)
+    if (!vm->persistent) {
         virDomainObjListRemove(driver->domains, vm);
+        virObjectLock(vm);
+    }
 
     ret = 0;
 
@@ -1822,7 +1824,7 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml,
  cleanup:
     if (remove_dom && vm) {
         virDomainObjListRemove(driver->domains, vm);
-        vm = NULL;
+        virObjectLock(vm);
     }
     virDomainObjEndAPI(&vm);
     return ret;
@@ -1877,7 +1879,7 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
     if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) {
         if (!vm->persistent) {
             virDomainObjListRemove(driver->domains, vm);
-            vm = NULL;
+            virObjectLock(vm);
         }
         goto cleanup;
     }
@@ -1885,8 +1887,10 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
     ret = libxlDomainStartRestore(driver, vm,
                                   (flags & VIR_DOMAIN_SAVE_PAUSED) != 0,
                                   fd, hdr.version);
-    if (ret < 0 && !vm->persistent)
+    if (ret < 0 && !vm->persistent) {
         virDomainObjListRemove(driver->domains, vm);
+        virObjectLock(vm);
+    }
 
     libxlDomainObjEndJob(driver, vm);
 
@@ -1995,7 +1999,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags)
  cleanup:
     if (remove_dom && vm) {
         virDomainObjListRemove(driver->domains, vm);
-        vm = NULL;
+        virObjectLock(vm);
     }
     virDomainObjEndAPI(&vm);
     if (event)
@@ -2056,7 +2060,7 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags)
  cleanup:
     if (remove_dom && vm) {
         virDomainObjListRemove(driver->domains, vm);
-        vm = NULL;
+        virObjectLock(vm);
     }
     virDomainObjEndAPI(&vm);
     VIR_FREE(name);
@@ -2880,7 +2884,7 @@ libxlDomainUndefineFlags(virDomainPtr dom,
         vm->persistent = 0;
     } else {
         virDomainObjListRemove(driver->domains, vm);
-        vm = NULL;
+        virObjectLock(vm);
     }
 
     ret = 0;
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 7dc39ae025..324a1adde9 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -299,7 +299,7 @@ libxlDoMigrateReceive(void *opaque)
  cleanup:
     if (remove_dom) {
         virDomainObjListRemove(driver->domains, vm);
-        vm = NULL;
+        virObjectLock(vm);
     }
     virDomainObjEndAPI(&vm);
 }
@@ -1336,8 +1336,11 @@ libxlDomainMigrationFinish(virConnectPtr dconn,
                              VIR_DOMAIN_SHUTOFF_FAILED);
         event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
                                          VIR_DOMAIN_EVENT_STOPPED_FAILED);
-        if (!vm->persistent)
+        if (!vm->persistent) {
             virDomainObjListRemove(driver->domains, vm);
+            /* Caller passed a locked vm and expects the same on return */
+            virObjectLock(vm);
+        }
     }
 
     if (event)
-- 
GitLab