[libvirt] [PATCH 6/6] conf: Clean up object referencing for Add and Remove
John Ferlan
jferlan at redhat.com
Tue Apr 24 12:28:09 UTC 2018
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 at redhat.com>
---
src/bhyve/bhyve_driver.c | 21 +++++----------
src/conf/virdomainobjlist.c | 33 +++++++++++------------
src/libxl/libxl_driver.c | 64 +++++++++------------------------------------
src/libxl/libxl_migration.c | 31 +++++-----------------
src/lxc/lxc_driver.c | 21 ++++-----------
src/lxc/lxc_process.c | 17 ++++++------
src/openvz/openvz_conf.c | 2 +-
src/openvz/openvz_driver.c | 20 +++++---------
src/qemu/qemu_domain.c | 17 +-----------
src/qemu/qemu_driver.c | 6 +----
src/qemu/qemu_migration.c | 3 +--
src/test/test_driver.c | 56 ++++++++++++---------------------------
src/uml/uml_driver.c | 37 +++++++-------------------
src/vmware/vmware_conf.c | 3 +--
src/vmware/vmware_driver.c | 17 ++++--------
src/vz/vz_driver.c | 1 -
src/vz/vz_sdk.c | 3 ---
17 files changed, 99 insertions(+), 253 deletions(-)
diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 768578a43f..6001f0806c 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -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);
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 5725040552..a30b3908f0 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -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);
}
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 55a93a489b..34adef8d48 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -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;
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index d37a4a687a..206b878292 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -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;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index ca01d369d5..5fbc3fe666 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -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);
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index e911d88b56..e3849e00f0 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -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);
diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c
index a25eaf570e..c2eda94f1c 100644
--- a/src/openvz/openvz_conf.c
+++ b/src/openvz/openvz_conf.c
@@ -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;
}
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index c10d6df663..14295dfda0 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -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;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 326c939c85..b0f72c4a62 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -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);
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7484b00e23..d67ef93fde 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1763,7 +1763,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,
@@ -6735,7 +6734,6 @@ qemuDomainRestoreFlags(virConnectPtr conn,
VIR_DOMAIN_OBJ_LIST_ADD_CHECK_LIVE,
NULL)))
goto cleanup;
- virObjectRef(vm);
def = NULL;
if (flags & VIR_DOMAIN_SAVE_RUNNING)
@@ -7409,9 +7407,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"));
@@ -16407,7 +16404,6 @@ static virDomainPtr qemuDomainQemuAttach(virConnectPtr conn,
NULL)))
goto cleanup;
- virObjectRef(vm);
def = NULL;
if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) {
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 88b8253fa9..d4bbbe4c84 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -2279,9 +2279,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;
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index a1888c0c9f..4c701ca7d0 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -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;
@@ -2794,8 +2775,7 @@ static virDomainPtr testDomainDefineXMLFlags(virConnectPtr conn,
cleanup:
virDomainDefFree(def);
virDomainDefFree(oldDef);
- if (dom)
- virObjectUnlock(dom);
+ virDomainObjEndAPI(&dom);
testObjectEventQueue(privconn, event);
return ret;
}
@@ -3062,12 +3042,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;
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index b50ba1ba64..fd109dcd9c 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -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;
diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c
index b9f18e6ac4..e1d6956df0 100644
--- a/src/vmware/vmware_conf.c
+++ b/src/vmware/vmware_conf.c
@@ -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;
diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index 21c10b6605..f94b3252fe 100644
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -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;
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index d3fcae491a..48a9a866d9 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -3161,7 +3161,6 @@ vzDomainMigratePerformStep(virDomainObjPtr dom,
goto cleanup;
virDomainObjListRemove(driver->domains, dom);
- virObjectLock(dom);
ret = 0;
diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c
index bb8f15e4fe..cb82b9d582 100644
--- a/src/vz/vz_sdk.c
+++ b/src/vz/vz_sdk.c
@@ -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:
--
2.13.6
More information about the libvir-list
mailing list