]> git.ipfire.org Git - thirdparty/libvirt.git/commitdiff
conf: Clean up object referencing for Add and Remove
authorJohn Ferlan <jferlan@redhat.com>
Mon, 23 Apr 2018 14:40:48 +0000 (10:40 -0400)
committerJohn Ferlan <jferlan@redhat.com>
Thu, 3 May 2018 23:09:03 +0000 (19:09 -0400)
When adding a new object to the domain object list, there should
have been 2 virObjectRef calls made one for each list into which
the object was placed to match the 2 virObjectUnref calls that
would occur during Remove as part of virHashRemoveEntry when
virObjectFreeHashData is called when the element is removed from
the hash table as set up in virDomainObjListNew.

Some drivers (libxl, lxc, qemu, and vz) handled this inconsistency
by calling virObjectRef upon successful return from virDomainObjListAdd
in order to use virDomainObjEndAPI when done with the returned @vm.
While others (bhyve, openvz, test, and vmware) handled this via only
calling virObjectUnlock upon successful return from virDomainObjListAdd.

This patch will "unify" the approach to use virDomainObjEndAPI
for any @vm successfully returned from virDomainObjListAdd.

Because list removal is so tightly coupled with list addition,
this patch fixes the list removal algorithm to return the object
as entered - "locked and reffed".  This way, the callers can then
decide how to uniformly handle add/remove success and failure.
This removes the onus on the caller to "specially handle" the
@vm during removal processing.

The Add/Remove logic allows for some logic simplification such
as in libxl where we can Remove the @vm directly rather than
needing to set a @remove_dom boolean and removing after the
libxlDomainObjEndJob completes as the @vm is locked/reffed.

Signed-off-by: John Ferlan <jferlan@redhat.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
17 files changed:
src/bhyve/bhyve_driver.c
src/conf/virdomainobjlist.c
src/libxl/libxl_driver.c
src/libxl/libxl_migration.c
src/lxc/lxc_driver.c
src/lxc/lxc_process.c
src/openvz/openvz_conf.c
src/openvz/openvz_driver.c
src/qemu/qemu_domain.c
src/qemu/qemu_driver.c
src/qemu/qemu_migration.c
src/test/test_driver.c
src/uml/uml_driver.c
src/vmware/vmware_conf.c
src/vmware/vmware_driver.c
src/vz/vz_driver.c
src/vz/vz_sdk.c

index 768578a43f32dbc8ef6db857259574168e85fe3f..6001f0806cca3186464aa905376acbbdd2e4362d 100644 (file)
@@ -550,7 +550,6 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag
     if (virDomainSaveConfig(BHYVE_CONFIG_DIR, caps,
                             vm->newDef ? vm->newDef : vm->def) < 0) {
         virDomainObjListRemove(privconn->domains, vm);
-        vm = NULL;
         goto cleanup;
     }
 
@@ -566,8 +565,7 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag
     virObjectUnref(caps);
     virDomainDefFree(def);
     virDomainDefFree(oldDef);
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     if (event)
         virObjectEventStateQueue(privconn->domainEventState, event);
 
@@ -609,12 +607,10 @@ bhyveDomainUndefine(virDomainPtr domain)
                                               VIR_DOMAIN_EVENT_UNDEFINED,
                                               VIR_DOMAIN_EVENT_UNDEFINED_REMOVED);
 
-    if (virDomainObjIsActive(vm)) {
+    if (virDomainObjIsActive(vm))
         vm->persistent = 0;
-    } else {
+    else
         virDomainObjListRemove(privconn->domains, vm);
-        virObjectLock(vm);
-    }
 
     ret = 0;
 
@@ -958,10 +954,8 @@ bhyveDomainCreateXML(virConnectPtr conn,
                              VIR_DOMAIN_RUNNING_BOOTED,
                              start_flags) < 0) {
         /* If domain is not persistent, remove its data */
-        if (!vm->persistent) {
+        if (!vm->persistent)
             virDomainObjListRemove(privconn->domains, vm);
-            vm = NULL;
-        }
         goto cleanup;
     }
 
@@ -974,8 +968,7 @@ bhyveDomainCreateXML(virConnectPtr conn,
  cleanup:
     virObjectUnref(caps);
     virDomainDefFree(def);
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     if (event)
         virObjectEventStateQueue(privconn->domainEventState, event);
 
@@ -1007,10 +1000,8 @@ bhyveDomainDestroy(virDomainPtr dom)
                                               VIR_DOMAIN_EVENT_STOPPED,
                                               VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
 
-    if (!vm->persistent) {
+    if (!vm->persistent)
         virDomainObjListRemove(privconn->domains, vm);
-        virObjectLock(vm);
-    }
 
  cleanup:
     virDomainObjEndAPI(&vm);
index d1eb87e5cfaca359fb97f4cc6ca76c8465b74277..72064d7c663a7c275b23c71002b5cda2bef801ff 100644 (file)
@@ -226,9 +226,13 @@ virDomainObjListFindByName(virDomainObjListPtr doms,
  * Upon entry @vm should have at least 1 ref and be locked.
  *
  * Add the @vm into the @doms->objs and @doms->objsName hash
- * tables.
+ * tables. Once successfully added into a table, increase the
+ * reference count since upon removal in virHashRemoveEntry
+ * the virObjectUnref will be called since the hash tables were
+ * configured to call virObjectFreeHashData when the object is
+ * removed from the hash table.
  *
- * Returns 0 on success with 2 references and locked
+ * Returns 0 on success with 3 references and locked
  *        -1 on failure with 1 reference and locked
  */
 static int
@@ -240,15 +244,12 @@ virDomainObjListAddObjLocked(virDomainObjListPtr doms,
     virUUIDFormat(vm->def->uuid, uuidstr);
     if (virHashAddEntry(doms->objs, uuidstr, vm) < 0)
         return -1;
+    virObjectRef(vm);
 
     if (virHashAddEntry(doms->objsName, vm->def->name, vm) < 0) {
-        virObjectRef(vm);
         virHashRemoveEntry(doms->objs, uuidstr);
         return -1;
     }
-
-    /* Since domain is in two hash tables, increment the
-     * reference counter */
     virObjectRef(vm);
 
     return 0;
@@ -266,6 +267,9 @@ virDomainObjListAddObjLocked(virDomainObjListPtr doms,
  * the @def being added is assumed to represent a
  * live config, not a future inactive config
  *
+ * The returned @vm from this function will be locked and ref
+ * counted. The caller is expected to use virDomainObjEndAPI
+ * when it completes usage.
  */
 static virDomainObjPtr
 virDomainObjListAddLocked(virDomainObjListPtr doms,
@@ -311,9 +315,6 @@ virDomainObjListAddLocked(virDomainObjListPtr doms,
                               def,
                               !!(flags & VIR_DOMAIN_OBJ_LIST_ADD_LIVE),
                               oldDef);
-        /* XXX: Temporary until this API is fixed to return a locked and
-         *      refcnt'd object */
-        virObjectUnref(vm);
     } else {
         /* UUID does not match, but if a name matches, refuse it */
         if ((vm = virDomainObjListFindByNameLocked(doms, def->name))) {
@@ -371,7 +372,6 @@ virDomainObjListRemoveLocked(virDomainObjListPtr doms,
 
     virHashRemoveEntry(doms->objs, uuidstr);
     virHashRemoveEntry(doms->objsName, dom->def->name);
-    virObjectUnlock(dom);
 }
 
 
@@ -386,8 +386,7 @@ virDomainObjListRemoveLocked(virDomainObjListPtr doms,
  * no one else is either waiting for 'dom' or still using it.
  *
  * When this function returns, @dom will be removed from the hash
- * tables, unlocked, and returned with the refcnt that was present
- * upon entry.
+ * tables and returned with lock and refcnt that was present upon entry.
  */
 void
 virDomainObjListRemove(virDomainObjListPtr doms,
@@ -453,9 +452,11 @@ virDomainObjListRename(virDomainObjListPtr doms,
     if (virHashAddEntry(doms->objsName, new_name, dom) < 0)
         goto cleanup;
 
-    /* Okay, this is crazy. virHashAddEntry() does not increment
-     * the refcounter of @dom, but virHashRemoveEntry() does
-     * decrement it. We need to work around it. */
+    /* Increment the refcnt for @new_name. We're about to remove
+     * the @old_name which will cause the refcnt to be decremented
+     * via the virObjectUnref call made during the virObjectFreeHashData
+     * as a result of removing something from the object list hash
+     * table as set up during virDomainObjListNew. */
     virObjectRef(dom);
 
     rc = callback(dom, new_name, flags, opaque);
@@ -624,7 +625,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms,
         if (dom) {
             if (!liveStatus)
                 dom->persistent = 1;
-            virObjectUnlock(dom);
+            virDomainObjEndAPI(&dom);
         } else {
             VIR_ERROR(_("Failed to load config for domain '%s'"), entry->d_name);
         }
index 55a93a489beb847bd496bd5d39a57ddb9815a8b5..34adef8d48ca09a90546716914b4011d73104825 100644 (file)
@@ -450,13 +450,8 @@ libxlReconnectDomain(virDomainObjPtr vm,
 
  error:
     libxlDomainCleanup(driver, vm);
-    if (!vm->persistent) {
+    if (!vm->persistent)
         virDomainObjListRemoveLocked(driver->domains, vm);
-
-        /* virDomainObjListRemoveLocked leaves the object unlocked,
-         * lock it again to factorize more code. */
-        virObjectLock(vm);
-    }
     goto cleanup;
 }
 
@@ -605,7 +600,6 @@ libxlAddDom0(libxlDriverPrivatePtr driver)
                                    0,
                                    &oldDef)))
         goto cleanup;
-
     def = NULL;
 
     vm->persistent = 1;
@@ -626,8 +620,7 @@ libxlAddDom0(libxlDriverPrivatePtr driver)
     libxl_dominfo_dispose(&d_info);
     virDomainDefFree(def);
     virDomainDefFree(oldDef);
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     virObjectUnref(cfg);
     return ret;
 }
@@ -1043,23 +1036,18 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml,
                                    VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
                                    NULL)))
         goto cleanup;
-    virObjectRef(vm);
     def = NULL;
 
     if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) {
-        if (!vm->persistent) {
+        if (!vm->persistent)
             virDomainObjListRemove(driver->domains, vm);
-            virObjectLock(vm);
-        }
         goto cleanup;
     }
 
     if (libxlDomainStartNew(driver, vm,
                          (flags & VIR_DOMAIN_START_PAUSED) != 0) < 0) {
-        if (!vm->persistent) {
+        if (!vm->persistent)
             virDomainObjListRemove(driver->domains, vm);
-            virObjectLock(vm);
-        }
         goto endjob;
     }
 
@@ -1396,10 +1384,8 @@ 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;
 
@@ -1759,7 +1745,6 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml,
     libxlDriverPrivatePtr driver = dom->conn->privateData;
     virDomainObjPtr vm;
     int ret = -1;
-    bool remove_dom = false;
 
 #ifdef LIBXL_HAVE_NO_SUSPEND_RESUME
     virReportUnsupportedError();
@@ -1791,7 +1776,7 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml,
         goto endjob;
 
     if (!vm->persistent)
-        remove_dom = true;
+        virDomainObjListRemove(driver->domains, vm);
 
     ret = 0;
 
@@ -1799,10 +1784,6 @@ libxlDomainSaveFlags(virDomainPtr dom, const char *to, const char *dxml,
     libxlDomainObjEndJob(driver, vm);
 
  cleanup:
-    if (remove_dom && vm) {
-        virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
     virDomainObjEndAPI(&vm);
     return ret;
 }
@@ -1850,24 +1831,19 @@ libxlDomainRestoreFlags(virConnectPtr conn, const char *from,
                                    VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
                                    NULL)))
         goto cleanup;
-    virObjectRef(vm);
     def = NULL;
 
     if (libxlDomainObjBeginJob(driver, vm, LIBXL_JOB_MODIFY) < 0) {
-        if (!vm->persistent) {
+        if (!vm->persistent)
             virDomainObjListRemove(driver->domains, vm);
-            virObjectLock(vm);
-        }
         goto cleanup;
     }
 
     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);
 
@@ -1893,7 +1869,6 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags)
     libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
     virDomainObjPtr vm;
     virObjectEventPtr event = NULL;
-    bool remove_dom = false;
     bool paused = false;
     int ret = -1;
 
@@ -1951,7 +1926,7 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags)
         event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED,
                                          VIR_DOMAIN_EVENT_STOPPED_CRASHED);
         if (!vm->persistent)
-            remove_dom = true;
+            virDomainObjListRemove(driver->domains, vm);
     }
 
     ret = 0;
@@ -1972,10 +1947,6 @@ libxlDomainCoreDump(virDomainPtr dom, const char *to, unsigned int flags)
     libxlDomainObjEndJob(driver, vm);
 
  cleanup:
-    if (remove_dom && vm) {
-        virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
     virDomainObjEndAPI(&vm);
     if (event)
         libxlDomainEventQueue(driver, event);
@@ -1990,7 +1961,6 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags)
     virDomainObjPtr vm = NULL;
     char *name = NULL;
     int ret = -1;
-    bool remove_dom = false;
 
     virCheckFlags(0, -1);
 
@@ -2023,7 +1993,7 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags)
         goto endjob;
 
     if (!vm->persistent)
-        remove_dom = true;
+        virDomainObjListRemove(driver->domains, vm);
 
     ret = 0;
 
@@ -2031,10 +2001,6 @@ libxlDomainManagedSave(virDomainPtr dom, unsigned int flags)
     libxlDomainObjEndJob(driver, vm);
 
  cleanup:
-    if (remove_dom && vm) {
-        virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
     virDomainObjEndAPI(&vm);
     VIR_FREE(name);
     return ret;
@@ -2765,16 +2731,14 @@ libxlDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag
                                    0,
                                    &oldDef)))
         goto cleanup;
-
-    virObjectRef(vm);
     def = NULL;
+
     vm->persistent = 1;
 
     if (virDomainSaveConfig(cfg->configDir,
                             cfg->caps,
                             vm->newDef ? vm->newDef : vm->def) < 0) {
         virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
         goto cleanup;
     }
 
@@ -2851,12 +2815,10 @@ libxlDomainUndefineFlags(virDomainPtr dom,
     event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_UNDEFINED,
                                      VIR_DOMAIN_EVENT_UNDEFINED_REMOVED);
 
-    if (virDomainObjIsActive(vm)) {
+    if (virDomainObjIsActive(vm))
         vm->persistent = 0;
-    } else {
+    else
         virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
 
     ret = 0;
 
index d37a4a687ab4e0533dde50ba60b89836723c2ef2..206b878292f6a5ca5dbf7e111809de358104580e 100644 (file)
@@ -265,7 +265,6 @@ libxlDoMigrateDstReceive(void *opaque)
     int recvfd = args->recvfd;
     size_t i;
     int ret;
-    bool remove_dom = 0;
 
     virObjectRef(vm);
     virObjectLock(vm);
@@ -280,7 +279,7 @@ libxlDoMigrateDstReceive(void *opaque)
                                   args->migcookie->xenMigStreamVer);
 
     if (ret < 0 && !vm->persistent)
-        remove_dom = true;
+        virDomainObjListRemove(driver->domains, vm);
 
     /* Remove all listen socks from event handler, and close them. */
     for (i = 0; i < nsocks; i++) {
@@ -296,10 +295,6 @@ libxlDoMigrateDstReceive(void *opaque)
     libxlDomainObjEndJob(driver, vm);
 
  cleanup:
-    if (remove_dom) {
-        virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
     virDomainObjEndAPI(&vm);
 }
 
@@ -581,9 +576,8 @@ libxlDomainMigrationDstPrepareTunnel3(virConnectPtr dconn,
                                    VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
                                    NULL)))
         goto error;
-
-    virObjectRef(vm);
     *def = NULL;
+
     priv = vm->privateData;
 
     if (taint_hook) {
@@ -633,10 +627,8 @@ libxlDomainMigrationDstPrepareTunnel3(virConnectPtr dconn,
     VIR_FORCE_CLOSE(dataFD[0]);
     virObjectUnref(args);
     /* Remove virDomainObj from domain list */
-    if (vm) {
+    if (vm)
         virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
 
  done:
     virDomainObjEndAPI(&vm);
@@ -680,9 +672,8 @@ libxlDomainMigrationDstPrepare(virConnectPtr dconn,
                                    VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
                                    NULL)))
         goto error;
-
-    virObjectRef(vm);
     *def = NULL;
+
     priv = vm->privateData;
 
     if (taint_hook) {
@@ -807,10 +798,8 @@ libxlDomainMigrationDstPrepare(virConnectPtr dconn,
         priv->migrationPort = 0;
     }
     /* Remove virDomainObj from domain list */
-    if (vm) {
+    if (vm)
         virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
 
  done:
     VIR_FREE(xmlout);
@@ -1336,11 +1325,8 @@ libxlDomainMigrationDstFinish(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)
@@ -1392,11 +1378,8 @@ libxlDomainMigrationSrcConfirm(libxlDriverPrivatePtr driver,
     if (flags & VIR_MIGRATE_UNDEFINE_SOURCE)
         virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm);
 
-    if (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE)) {
+    if (!vm->persistent || (flags & VIR_MIGRATE_UNDEFINE_SOURCE))
         virDomainObjListRemove(driver->domains, vm);
-        /* Caller passed a locked vm and expects the same on return */
-        virObjectLock(vm);
-    }
 
     ret = 0;
 
index dd4e28fc7f704d848587fe5da2c2713055747a60..a451a0e552235abb6003a361bdebc950c1fc52fe 100644 (file)
@@ -478,14 +478,12 @@ lxcDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
                                    0, &oldDef)))
         goto cleanup;
 
-    virObjectRef(vm);
     def = NULL;
     vm->persistent = 1;
 
     if (virDomainSaveConfig(cfg->configDir, driver->caps,
                             vm->newDef ? vm->newDef : vm->def) < 0) {
         virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
         goto cleanup;
     }
 
@@ -546,12 +544,10 @@ static int lxcDomainUndefineFlags(virDomainPtr dom,
                                      VIR_DOMAIN_EVENT_UNDEFINED,
                                      VIR_DOMAIN_EVENT_UNDEFINED_REMOVED);
 
-    if (virDomainObjIsActive(vm)) {
+    if (virDomainObjIsActive(vm))
         vm->persistent = 0;
-    } else {
+    else
         virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
 
     ret = 0;
 
@@ -1233,14 +1229,11 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn,
                                    VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
                                    NULL)))
         goto cleanup;
-    virObjectRef(vm);
     def = NULL;
 
     if (virLXCDomainObjBeginJob(driver, vm, LXC_JOB_MODIFY) < 0) {
-        if (!vm->persistent) {
+        if (!vm->persistent)
             virDomainObjListRemove(driver->domains, vm);
-            virObjectLock(vm);
-        }
         goto cleanup;
     }
 
@@ -1250,10 +1243,8 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn,
                            VIR_DOMAIN_RUNNING_BOOTED) < 0) {
         virDomainAuditStart(vm, "booted", false);
         virLXCDomainObjEndJob(driver, vm);
-        if (!vm->persistent) {
+        if (!vm->persistent)
             virDomainObjListRemove(driver->domains, vm);
-            virObjectLock(vm);
-        }
         goto cleanup;
     }
 
@@ -1523,10 +1514,8 @@ lxcDomainDestroyFlags(virDomainPtr dom,
 
  endjob:
     virLXCDomainObjEndJob(driver, vm);
-    if (!vm->persistent) {
+    if (!vm->persistent)
         virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
 
  cleanup:
     virDomainObjEndAPI(&vm);
index 0c5b24affa1e3c6d21ab03d0fe0fa0019d4f70bc..cc6ed12526c12477b1d360370893304fa6e68a0b 100644 (file)
@@ -671,10 +671,8 @@ static void virLXCProcessMonitorEOFNotify(virLXCMonitorPtr mon,
         } else {
             VIR_DEBUG("Stop event has already been sent");
         }
-        if (!vm->persistent) {
+        if (!vm->persistent)
             virDomainObjListRemove(driver->domains, vm);
-            vm = NULL;
-        }
     } else {
         int ret = virLXCProcessReboot(driver, vm);
         virDomainAuditStop(vm, "reboot");
@@ -685,15 +683,15 @@ static void virLXCProcessMonitorEOFNotify(virLXCMonitorPtr mon,
             event = virDomainEventLifecycleNewFromObj(vm,
                                              VIR_DOMAIN_EVENT_STOPPED,
                                              priv->stopReason);
-            if (!vm->persistent) {
+            if (!vm->persistent)
                 virDomainObjListRemove(driver->domains, vm);
-                vm = NULL;
-            }
         }
     }
 
-    if (vm)
-        virObjectUnlock(vm);
+    /* NB: virLXCProcessConnectMonitor will perform the virObjectRef(vm)
+     * before adding monitorCallbacks. Since we are now done with the @vm
+     * we can Unref/Unlock */
+    virDomainObjEndAPI(&vm);
     if (event)
         virObjectEventStateQueue(driver->domainEventState, event);
 }
@@ -803,7 +801,8 @@ static virLXCMonitorPtr virLXCProcessConnectMonitor(virLXCDriverPtr driver,
         goto cleanup;
 
     /* Hold an extra reference because we can't allow 'vm' to be
-     * deleted while the monitor is active */
+     * deleted while the monitor is active. This will be unreffed
+     * during EOFNotify processing. */
     virObjectRef(vm);
 
     monitor = virLXCMonitorNew(vm, cfg->stateDir, &monitorCallbacks);
index 72ef63a3555e21b05624a932ac3935c351bf5502..5ed2b423cb4fc0af3d1dbb42f21845aacad8e461 100644 (file)
@@ -611,7 +611,7 @@ int openvzLoadDomains(struct openvz_driver *driver)
         /* XXX OpenVZ doesn't appear to have concept of a transient domain */
         dom->persistent = 1;
 
-        virObjectUnlock(dom);
+        virDomainObjEndAPI(&dom);
         dom = NULL;
         def = NULL;
     }
index c10d6df6633bfad76fd30af6e0cf9413fcdb236b..14295dfda0f6e0074c6f3014669d3835affce61d 100644 (file)
@@ -982,8 +982,7 @@ openvzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla
 
  cleanup:
     virDomainDefFree(vmdef);
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     openvzDriverUnlock(driver);
     return dom;
 }
@@ -1071,8 +1070,7 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml,
 
  cleanup:
     virDomainDefFree(vmdef);
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     openvzDriverUnlock(driver);
     return dom;
 }
@@ -1151,12 +1149,10 @@ openvzDomainUndefineFlags(virDomainPtr dom,
     if (virRun(prog, NULL) < 0)
         goto cleanup;
 
-    if (virDomainObjIsActive(vm)) {
+    if (virDomainObjIsActive(vm))
         vm->persistent = 0;
-    } else {
+    else
         virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
 
     ret = 0;
 
@@ -2232,16 +2228,13 @@ openvzDomainMigratePrepare3Params(virConnectPtr dconn,
 
  error:
     virDomainDefFree(def);
-    if (vm) {
+    if (vm)
         virDomainObjListRemove(driver->domains, vm);
-        vm = NULL;
-    }
 
  done:
     VIR_FREE(my_hostname);
     virURIFree(uri);
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -2396,7 +2389,6 @@ openvzDomainMigrateConfirm3Params(virDomainPtr domain,
     VIR_DEBUG("Domain '%s' successfully migrated", vm->def->name);
 
     virDomainObjListRemove(driver->domains, vm);
-    virObjectLock(vm);
 
     ret = 0;
 
index 0d752e1464fde5517a6aec03951a796571678bf1..727ea33f144a598b1178ee121161807a13d30a03 100644 (file)
@@ -7167,24 +7167,9 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver,
         VIR_FREE(snapDir);
     }
 
-    virObjectRef(vm);
-
     virDomainObjListRemove(driver->domains, vm);
-    /*
-     * virDomainObjListRemove() leaves the domain unlocked so it can
-     * be unref'd for other drivers that depend on that, but we still
-     * need to reset a job and we have a reference from the API that
-     * called this function.  So we need to lock it back.  This is
-     * just a workaround for the qemu driver.
-     *
-     * XXX: Ideally, the global handling of domain objects and object
-     *      lists would be refactored so we don't need hacks like
-     *      this, but since that requires refactor of all drivers,
-     *      it's a work for another day.
-     */
-    virObjectLock(vm);
+
     virObjectUnref(cfg);
-    virObjectUnref(vm);
 }
 
 
index 82e1566c79726485818331d7dc47172c46a9b420..83fc191085505bb3952ef0417bcfc03ce2395e2e 100644 (file)
@@ -1765,7 +1765,6 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn,
                                    VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
                                    NULL)))
         goto cleanup;
-    virObjectRef(vm);
     def = NULL;
 
     if (qemuProcessBeginJob(driver, vm, VIR_DOMAIN_JOB_OPERATION_START,
@@ -6737,7 +6736,6 @@ qemuDomainRestoreFlags(virConnectPtr conn,
                                    VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
                                    NULL)))
         goto cleanup;
-    virObjectRef(vm);
     def = NULL;
 
     if (flags & VIR_DOMAIN_SAVE_RUNNING)
@@ -7411,9 +7409,8 @@ qemuDomainDefineXMLFlags(virConnectPtr conn,
                                    driver->xmlopt,
                                    0, &oldDef)))
         goto cleanup;
-
-    virObjectRef(vm);
     def = NULL;
+
     if (qemuDomainHasBlockjob(vm, true)) {
         virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, "%s",
                        _("domain has active block job"));
@@ -16420,7 +16417,6 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn,
                                    NULL)))
         goto cleanup;
 
-    virObjectRef(vm);
     def = NULL;
 
     if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) {
index 2bdd58bb503f0f57cdfcc1d5d160d9de37012eef..7602a304c57f7a5c7367b069b1822bfc4b6cb63e 100644 (file)
@@ -2280,9 +2280,8 @@ qemuMigrationDstPrepareAny(virQEMUDriverPtr driver,
                                    VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
                                    NULL)))
         goto cleanup;
-
-    virObjectRef(vm);
     *def = NULL;
+
     priv = vm->privateData;
     if (VIR_STRDUP(priv->origname, origname) < 0)
         goto cleanup;
index 9089dc9a2522cf8fccb7f66ce8467889f130da13..467587b19fa88f074f617c2d33789f8b49a34ea6 100644 (file)
@@ -893,7 +893,7 @@ testParseDomains(testDriverPtr privconn,
     int num, ret = -1;
     size_t i;
     xmlNodePtr *nodes = NULL;
-    virDomainObjPtr obj;
+    virDomainObjPtr obj = NULL;
 
     num = virXPathNodeSet("/node/domain", ctxt, &nodes);
     if (num < 0)
@@ -921,10 +921,8 @@ testParseDomains(testDriverPtr privconn,
             goto error;
         }
 
-        if (testParseDomainSnapshots(privconn, obj, file, ctxt) < 0) {
-            virObjectUnlock(obj);
+        if (testParseDomainSnapshots(privconn, obj, file, ctxt) < 0)
             goto error;
-        }
 
         nsdata = def->namespaceData;
         obj->persistent = !nsdata->transient;
@@ -932,20 +930,19 @@ testParseDomains(testDriverPtr privconn,
 
         if (nsdata->runstate != VIR_DOMAIN_SHUTOFF) {
             if (testDomainStartState(privconn, obj,
-                                     VIR_DOMAIN_RUNNING_BOOTED) < 0) {
-                virObjectUnlock(obj);
+                                     VIR_DOMAIN_RUNNING_BOOTED) < 0)
                 goto error;
-            }
         } else {
             testDomainShutdownState(NULL, obj, 0);
         }
         virDomainObjSetState(obj, nsdata->runstate, 0);
 
-        virObjectUnlock(obj);
+        virDomainObjEndAPI(&obj);
     }
 
     ret = 0;
  error:
+    virDomainObjEndAPI(&obj);
     VIR_FREE(nodes);
     return ret;
 }
@@ -1678,10 +1675,8 @@ testDomainCreateXML(virConnectPtr conn, const char *xml,
     def = NULL;
 
     if (testDomainStartState(privconn, dom, VIR_DOMAIN_RUNNING_BOOTED) < 0) {
-        if (!dom->persistent) {
+        if (!dom->persistent)
             virDomainObjListRemove(privconn->domains, dom);
-            dom = NULL;
-        }
         goto cleanup;
     }
 
@@ -1692,8 +1687,7 @@ testDomainCreateXML(virConnectPtr conn, const char *xml,
     ret = virGetDomain(conn, dom->def->name, dom->def->uuid, dom->def->id);
 
  cleanup:
-    if (dom)
-        virObjectUnlock(dom);
+    virDomainObjEndAPI(&dom);
     testObjectEventQueue(privconn, event);
     virDomainDefFree(def);
     testDriverUnlock(privconn);
@@ -1787,10 +1781,8 @@ static int testDomainDestroyFlags(virDomainPtr domain,
                                      VIR_DOMAIN_EVENT_STOPPED,
                                      VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
 
-    if (!privdom->persistent) {
+    if (!privdom->persistent)
         virDomainObjListRemove(privconn->domains, privdom);
-        virObjectLock(privdom);
-    }
 
     ret = 0;
  cleanup:
@@ -1888,10 +1880,8 @@ static int testDomainShutdownFlags(virDomainPtr domain,
                                      VIR_DOMAIN_EVENT_STOPPED,
                                      VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
 
-    if (!privdom->persistent) {
+    if (!privdom->persistent)
         virDomainObjListRemove(privconn->domains, privdom);
-        virObjectLock(privdom);
-    }
 
     ret = 0;
  cleanup:
@@ -1957,10 +1947,8 @@ static int testDomainReboot(virDomainPtr domain,
                                          VIR_DOMAIN_EVENT_STOPPED,
                                          VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
 
-        if (!privdom->persistent) {
+        if (!privdom->persistent)
             virDomainObjListRemove(privconn->domains, privdom);
-            virObjectLock(privdom);
-        }
     }
 
     ret = 0;
@@ -2095,10 +2083,8 @@ testDomainSaveFlags(virDomainPtr domain, const char *path,
                                      VIR_DOMAIN_EVENT_STOPPED,
                                      VIR_DOMAIN_EVENT_STOPPED_SAVED);
 
-    if (!privdom->persistent) {
+    if (!privdom->persistent)
         virDomainObjListRemove(privconn->domains, privdom);
-        virObjectLock(privdom);
-    }
 
     ret = 0;
  cleanup:
@@ -2200,10 +2186,8 @@ testDomainRestoreFlags(virConnectPtr conn,
     def = NULL;
 
     if (testDomainStartState(privconn, dom, VIR_DOMAIN_RUNNING_RESTORED) < 0) {
-        if (!dom->persistent) {
+        if (!dom->persistent)
             virDomainObjListRemove(privconn->domains, dom);
-            dom = NULL;
-        }
         goto cleanup;
     }
 
@@ -2216,8 +2200,7 @@ testDomainRestoreFlags(virConnectPtr conn,
     virDomainDefFree(def);
     VIR_FREE(xml);
     VIR_FORCE_CLOSE(fd);
-    if (dom)
-        virObjectUnlock(dom);
+    virDomainObjEndAPI(&dom);
     testObjectEventQueue(privconn, event);
     return ret;
 }
@@ -2280,10 +2263,8 @@ static int testDomainCoreDumpWithFormat(virDomainPtr domain,
         event = virDomainEventLifecycleNewFromObj(privdom,
                                          VIR_DOMAIN_EVENT_STOPPED,
                                          VIR_DOMAIN_EVENT_STOPPED_CRASHED);
-        if (!privdom->persistent) {
+        if (!privdom->persistent)
             virDomainObjListRemove(privconn->domains, privdom);
-            virObjectLock(privdom);
-        }
     }
 
     ret = 0;
@@ -2800,8 +2781,7 @@ static virDomainPtr testDomainDefineXMLFlags(virConnectPtr conn,
  cleanup:
     virDomainDefFree(def);
     virDomainDefFree(oldDef);
-    if (dom)
-        virObjectUnlock(dom);
+    virDomainObjEndAPI(&dom);
     testObjectEventQueue(privconn, event);
     return ret;
 }
@@ -3068,12 +3048,10 @@ static int testDomainUndefineFlags(virDomainPtr domain,
                                      VIR_DOMAIN_EVENT_UNDEFINED_REMOVED);
     privdom->hasManagedSave = false;
 
-    if (virDomainObjIsActive(privdom)) {
+    if (virDomainObjIsActive(privdom))
         privdom->persistent = 0;
-    } else {
+    else
         virDomainObjListRemove(privconn->domains, privdom);
-        virObjectLock(privdom);
-    }
 
     ret = 0;
 
index b0774b288e978446dd57352e915edf7159b7311e..53ec64e10f102d6161a6fce91777456273be3476 100644 (file)
@@ -379,10 +379,8 @@ umlInotifyEvent(int watch,
             event = virDomainEventLifecycleNewFromObj(dom,
                                              VIR_DOMAIN_EVENT_STOPPED,
                                              VIR_DOMAIN_EVENT_STOPPED_SHUTDOWN);
-            if (!dom->persistent) {
+            if (!dom->persistent)
                 virDomainObjListRemove(driver->domains, dom);
-                virObjectLock(dom);
-            }
         } else if (e.mask & (IN_CREATE | IN_MODIFY)) {
             VIR_DEBUG("Got inotify domain startup '%s'", name);
             if (virDomainObjIsActive(dom)) {
@@ -412,10 +410,8 @@ umlInotifyEvent(int watch,
                 event = virDomainEventLifecycleNewFromObj(dom,
                                                  VIR_DOMAIN_EVENT_STOPPED,
                                                  VIR_DOMAIN_EVENT_STOPPED_FAILED);
-                if (!dom->persistent) {
+                if (!dom->persistent)
                     virDomainObjListRemove(driver->domains, dom);
-                    virObjectLock(dom);
-                }
             } else if (umlIdentifyChrPTY(driver, dom) < 0) {
                 VIR_WARN("Could not identify character devices for new domain");
                 umlShutdownVMDaemon(driver, dom,
@@ -424,10 +420,8 @@ umlInotifyEvent(int watch,
                 event = virDomainEventLifecycleNewFromObj(dom,
                                                  VIR_DOMAIN_EVENT_STOPPED,
                                                  VIR_DOMAIN_EVENT_STOPPED_FAILED);
-                if (!dom->persistent) {
+                if (!dom->persistent)
                     virDomainObjListRemove(driver->domains, dom);
-                    virObjectLock(dom);
-                }
             }
         }
         virDomainObjEndAPI(&dom);
@@ -785,10 +779,8 @@ static int umlProcessAutoDestroyDom(void *payload,
                                      VIR_DOMAIN_EVENT_STOPPED,
                                      VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
 
-    if (!dom->persistent) {
+    if (!dom->persistent)
         virDomainObjListRemove(data->driver->domains, dom);
-        virObjectLock(dom);
-    }
 
     virDomainObjEndAPI(&dom);
     if (event)
@@ -1609,10 +1601,8 @@ static virDomainPtr umlDomainCreateXML(virConnectPtr conn, const char *xml,
     if (umlStartVMDaemon(conn, driver, vm,
                          (flags & VIR_DOMAIN_START_AUTODESTROY)) < 0) {
         virDomainAuditStart(vm, "booted", false);
-        if (!vm->persistent) {
+        if (!vm->persistent)
             virDomainObjListRemove(driver->domains, vm);
-            vm = NULL;
-        }
         goto cleanup;
     }
     virDomainAuditStart(vm, "booted", true);
@@ -1624,8 +1614,7 @@ static virDomainPtr umlDomainCreateXML(virConnectPtr conn, const char *xml,
 
  cleanup:
     virDomainDefFree(def);
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     if (event)
         umlDomainEventQueue(driver, event);
     umlDriverUnlock(driver);
@@ -1694,10 +1683,8 @@ umlDomainDestroyFlags(virDomainPtr dom,
     event = virDomainEventLifecycleNewFromObj(vm,
                                      VIR_DOMAIN_EVENT_STOPPED,
                                      VIR_DOMAIN_EVENT_STOPPED_DESTROYED);
-    if (!vm->persistent) {
+    if (!vm->persistent)
         virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
     ret = 0;
 
  cleanup:
@@ -2009,7 +1996,6 @@ umlDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
     if (virDomainSaveConfig(driver->configDir, driver->caps,
                             vm->newDef ? vm->newDef : vm->def) < 0) {
         virDomainObjListRemove(driver->domains, vm);
-        vm = NULL;
         goto cleanup;
     }
 
@@ -2017,8 +2003,7 @@ umlDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags)
 
  cleanup:
     virDomainDefFree(def);
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     umlDriverUnlock(driver);
     return dom;
 }
@@ -2054,12 +2039,10 @@ static int umlDomainUndefineFlags(virDomainPtr dom,
     if (virDomainDeleteConfig(driver->configDir, driver->autostartDir, vm) < 0)
         goto cleanup;
 
-    if (virDomainObjIsActive(vm)) {
+    if (virDomainObjIsActive(vm))
         vm->persistent = 0;
-    } else {
+    else
         virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
 
     ret = 0;
 
index b9f18e6ac464272bf071a8ecf605576de58abdfb..e1d6956df0be13d5b9f8776193ba9ea1e9386f7f 100644 (file)
@@ -186,10 +186,9 @@ vmwareLoadDomains(struct vmware_driver *driver)
                              VIR_DOMAIN_RUNNING_UNKNOWN);
         vm->persistent = 1;
 
-        virObjectUnlock(vm);
+        virDomainObjEndAPI(&vm);
 
         vmdef = NULL;
-        vm = NULL;
     }
 
     ret = 0;
index 21c10b6605fd89b4cda629efef611efc1ab97dcd..f94b3252fe750c1ea9376f15ea8ee76956b3605f 100644 (file)
@@ -502,10 +502,8 @@ vmwareDomainShutdownFlags(virDomainPtr dom,
     if (vmwareStopVM(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN) < 0)
         goto cleanup;
 
-    if (!vm->persistent) {
+    if (!vm->persistent)
         virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
 
     ret = 0;
  cleanup:
@@ -717,10 +715,8 @@ vmwareDomainCreateXML(virConnectPtr conn, const char *xml,
     vmdef = NULL;
 
     if (vmwareStartVM(driver, vm) < 0) {
-        if (!vm->persistent) {
+        if (!vm->persistent)
             virDomainObjListRemove(driver->domains, vm);
-            vm = NULL;
-        }
         goto cleanup;
     }
 
@@ -730,8 +726,7 @@ vmwareDomainCreateXML(virConnectPtr conn, const char *xml,
     virDomainDefFree(vmdef);
     VIR_FREE(vmx);
     VIR_FREE(vmxPath);
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     vmwareDriverUnlock(driver);
     return dom;
 }
@@ -796,12 +791,10 @@ vmwareDomainUndefineFlags(virDomainPtr dom,
     if (vmwareUpdateVMStatus(driver, vm) < 0)
         goto cleanup;
 
-    if (virDomainObjIsActive(vm)) {
+    if (virDomainObjIsActive(vm))
         vm->persistent = 0;
-    } else {
+    else
         virDomainObjListRemove(driver->domains, vm);
-        virObjectLock(vm);
-    }
 
     ret = 0;
 
index d3fcae491a493e4c1d27e195d730330697878d4b..48a9a866d9b81f8c2f5d25ade32abd399bdc4c7d 100644 (file)
@@ -3161,7 +3161,6 @@ vzDomainMigratePerformStep(virDomainObjPtr dom,
         goto cleanup;
 
     virDomainObjListRemove(driver->domains, dom);
-    virObjectLock(dom);
 
     ret = 0;
 
index bb8f15e4fed15eb3a3ae44540638c0a30af01b01..cb82b9d582f18b4a595b6985b3f57d8d3048b652 100644 (file)
@@ -1969,7 +1969,6 @@ prlsdkLoadDomain(vzDriverPtr driver,
             goto error;
         }
 
-        virObjectRef(dom);
         pdom = dom->privateData;
         pdom->sdkdom = sdkdom;
         PrlHandle_AddRef(sdkdom);
@@ -2235,7 +2234,6 @@ prlsdkHandleVmRemovedEvent(vzDriverPtr driver,
                     VIR_DOMAIN_EVENT_UNDEFINED_REMOVED);
 
     virDomainObjListRemove(driver->domains, dom);
-    virObjectLock(dom);
     virDomainObjEndAPI(&dom);
     return;
 }
@@ -4334,7 +4332,6 @@ prlsdkUnregisterDomain(vzDriverPtr driver, virDomainObjPtr dom, unsigned int fla
                     VIR_DOMAIN_EVENT_UNDEFINED_REMOVED);
 
     virDomainObjListRemove(driver->domains, dom);
-    virObjectLock(dom);
 
     ret = 0;
  cleanup: