There's no need to check if @dom exists before trying to call virDomainObjListRemove since it must exist due to prior checks. Additionally, if we do remove the @dom, then set it to NULL so that the virObjectUnlock isn't referencing something that is deleted. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/uml/uml_driver.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index ab7fa7f273..ff2e7ac66b 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -747,8 +747,10 @@ static int umlProcessAutoDestroyDom(void *payload, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); - if (dom && !dom->persistent) + if (!dom->persistent) { virDomainObjListRemove(data->driver->domains, dom); + dom = NULL; + } if (dom) virObjectUnlock(dom); -- 2.13.6

On Mon, Apr 02, 2018 at 09:06:13AM -0400, John Ferlan wrote: Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; Matches what we do everywhere else, but ewwww if the caller owns the lock, and the 'dom' hasn't been freed entirely, it is pretty evil semantics to have the lock be lost. Why can't virDomainObjListRemove leave the lock state unchanged. It already unlocks, then locks, then unlocks again. A final lock would avoid such suprising semantics Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Rather than repeat code throughout, create and use a couple of accessors in order to lookup by UUID. This will also generate a common error message including the failed uuidstr for lookup rather than just returning nothing in some instances. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/uml/uml_driver.c | 244 +++++++++++++++------------------------------------ 1 file changed, 70 insertions(+), 174 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index b84585d728..ad526a1e6b 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -163,6 +163,39 @@ umlVMDriverUnlock(void) umlDriverUnlock(uml_driver); } + +static virDomainObjPtr +umlDomObjFromDomainLocked(struct uml_driver *driver, + const unsigned char *uuid) +{ + virDomainObjPtr vm; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (!(vm = virDomainObjListFindByUUID(driver->domains, uuid))) { + virUUIDFormat(uuid, uuidstr); + + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + return NULL; + } + + return vm; +} + + +static virDomainObjPtr +umlDomObjFromDomain(struct uml_driver *driver, + const unsigned char *uuid) +{ + virDomainObjPtr vm; + + umlDriverLock(driver); + vm = umlDomObjFromDomainLocked(driver, uuid); + umlDriverUnlock(driver); + return vm; +} + + static virNWFilterCallbackDriver umlCallbackDriver = { .name = "UML", .vmFilterRebuild = umlVMFilterRebuild, @@ -1376,20 +1409,14 @@ static virDomainPtr umlDomainLookupByID(virConnectPtr conn, } static virDomainPtr umlDomainLookupByUUID(virConnectPtr conn, - const unsigned char *uuid) + const unsigned char *uuid) { struct uml_driver *driver = (struct uml_driver *)conn->privateData; virDomainObjPtr vm; virDomainPtr dom = NULL; - umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, uuid); - umlDriverUnlock(driver); - - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, NULL); - goto cleanup; - } + if (!(vm = umlDomObjFromDomain(driver, uuid))) + return NULL; if (virDomainLookupByUUIDEnsureACL(conn, vm->def) < 0) goto cleanup; @@ -1435,13 +1462,8 @@ static int umlDomainIsActive(virDomainPtr dom) virDomainObjPtr obj; int ret = -1; - umlDriverLock(driver); - obj = virDomainObjListFindByUUID(driver->domains, dom->uuid); - umlDriverUnlock(driver); - if (!obj) { - virReportError(VIR_ERR_NO_DOMAIN, NULL); - goto cleanup; - } + if (!(obj = umlDomObjFromDomain(driver, dom->uuid))) + return -1; if (virDomainIsActiveEnsureACL(dom->conn, obj->def) < 0) goto cleanup; @@ -1461,13 +1483,8 @@ static int umlDomainIsPersistent(virDomainPtr dom) virDomainObjPtr obj; int ret = -1; - umlDriverLock(driver); - obj = virDomainObjListFindByUUID(driver->domains, dom->uuid); - umlDriverUnlock(driver); - if (!obj) { - virReportError(VIR_ERR_NO_DOMAIN, NULL); - goto cleanup; - } + if (!(obj = umlDomObjFromDomain(driver, dom->uuid))) + return -1; if (virDomainIsPersistentEnsureACL(dom->conn, obj->def) < 0) goto cleanup; @@ -1486,13 +1503,8 @@ static int umlDomainIsUpdated(virDomainPtr dom) virDomainObjPtr obj; int ret = -1; - umlDriverLock(driver); - obj = virDomainObjListFindByUUID(driver->domains, dom->uuid); - umlDriverUnlock(driver); - if (!obj) { - virReportError(VIR_ERR_NO_DOMAIN, NULL); - goto cleanup; - } + if (!(obj = umlDomObjFromDomain(driver, dom->uuid))) + return -1; if (virDomainIsUpdatedEnsureACL(dom->conn, obj->def) < 0) goto cleanup; @@ -1645,14 +1657,8 @@ static int umlDomainShutdownFlags(virDomainPtr dom, virCheckFlags(0, -1); - umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - umlDriverUnlock(driver); - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching id %d"), dom->id); - goto cleanup; - } + if (!(vm = umlDomObjFromDomain(driver, dom->uuid))) + return -1; if (virDomainShutdownFlagsEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; @@ -1691,12 +1697,8 @@ umlDomainDestroyFlags(virDomainPtr dom, virCheckFlags(0, -1); umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching id %d"), dom->id); - goto cleanup; - } + if (!(vm = umlDomObjFromDomainLocked(driver, dom->uuid))) + return -1; if (virDomainDestroyFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -1734,14 +1736,8 @@ static char *umlDomainGetOSType(virDomainPtr dom) { virDomainObjPtr vm; char *type = NULL; - umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - umlDriverUnlock(driver); - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, "%s", - _("no domain with matching uuid")); - goto cleanup; - } + if (!(vm = umlDomObjFromDomain(driver, dom->uuid))) + return NULL; if (virDomainGetOSTypeEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -1763,18 +1759,8 @@ umlDomainGetMaxMemory(virDomainPtr dom) virDomainObjPtr vm; unsigned long long ret = 0; - umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - umlDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; - } + if (!(vm = umlDomObjFromDomain(driver, dom->uuid))) + return -1; if (virDomainGetMaxMemoryEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -1793,18 +1779,8 @@ static int umlDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) virDomainObjPtr vm; int ret = -1; - umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - umlDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; - } + if (!(vm = umlDomObjFromDomain(driver, dom->uuid))) + return -1; if (virDomainSetMaxMemoryEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -1830,18 +1806,8 @@ static int umlDomainSetMemory(virDomainPtr dom, unsigned long newmem) virDomainObjPtr vm; int ret = -1; - umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - umlDriverUnlock(driver); - - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); - goto cleanup; - } + if (!(vm = umlDomObjFromDomain(driver, dom->uuid))) + return -1; if (virDomainSetMemoryEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -1874,15 +1840,8 @@ static int umlDomainGetInfo(virDomainPtr dom, virDomainObjPtr vm; int ret = -1; - umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - umlDriverUnlock(driver); - - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, "%s", - _("no domain with matching uuid")); - goto cleanup; - } + if (!(vm = umlDomObjFromDomain(driver, dom->uuid))) + return -1; if (virDomainGetInfoEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -1923,15 +1882,8 @@ umlDomainGetState(virDomainPtr dom, virCheckFlags(0, -1); - umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - umlDriverUnlock(driver); - - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, "%s", - _("no domain with matching uuid")); - goto cleanup; - } + if (!(vm = umlDomObjFromDomain(driver, dom->uuid))) + return -1; if (virDomainGetStateEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -1956,14 +1908,8 @@ static char *umlDomainGetXMLDesc(virDomainPtr dom, /* Flags checked by virDomainDefFormat */ umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - umlDriverUnlock(driver); - - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, "%s", - _("no domain with matching uuid")); + if (!(vm = umlDomObjFromDomainLocked(driver, dom->uuid))) goto cleanup; - } if (virDomainGetXMLDescEnsureACL(dom->conn, vm->def, flags) < 0) goto cleanup; @@ -2023,13 +1969,8 @@ static int umlDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) virNWFilterReadLockFilterUpdates(); umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, "%s", - _("no domain with matching uuid")); + if (!(vm = umlDomObjFromDomainLocked(driver, dom->uuid))) goto cleanup; - } if (virDomainCreateWithFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -2123,12 +2064,8 @@ static int umlDomainUndefineFlags(virDomainPtr dom, virCheckFlags(0, -1); umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, "%s", - _("no domain with matching uuid")); + if (!(vm = umlDomObjFromDomainLocked(driver, dom->uuid))) goto cleanup; - } if (virDomainUndefineFlagsEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -2221,14 +2158,8 @@ static int umlDomainAttachDevice(virDomainPtr dom, const char *xml) umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = umlDomObjFromDomainLocked(driver, dom->uuid))) goto cleanup; - } if (virDomainAttachDeviceEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -2341,14 +2272,8 @@ static int umlDomainDetachDevice(virDomainPtr dom, const char *xml) int ret = -1; umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - if (!vm) { - char uuidstr[VIR_UUID_STRING_BUFLEN]; - virUUIDFormat(dom->uuid, uuidstr); - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = umlDomObjFromDomainLocked(driver, dom->uuid))) goto cleanup; - } if (virDomainDetachDeviceEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -2411,13 +2336,8 @@ static int umlDomainGetAutostart(virDomainPtr dom, int ret = -1; umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, "%s", - _("no domain with matching uuid")); + if (!(vm = umlDomObjFromDomainLocked(driver, dom->uuid))) goto cleanup; - } if (virDomainGetAutostartEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -2441,13 +2361,8 @@ static int umlDomainSetAutostart(virDomainPtr dom, int ret = -1; umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, "%s", - _("no domain with matching uuid")); + if (!(vm = umlDomObjFromDomainLocked(driver, dom->uuid))) goto cleanup; - } if (virDomainSetAutostartEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -2517,15 +2432,8 @@ umlDomainBlockPeek(virDomainPtr dom, virCheckFlags(0, -1); - umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - umlDriverUnlock(driver); - - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, "%s", - _("no domain with matching uuid")); - goto cleanup; - } + if (!(vm = umlDomObjFromDomain(driver, dom->uuid))) + return -1; if (virDomainBlockPeekEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -2581,7 +2489,6 @@ umlDomainOpenConsole(virDomainPtr dom, { struct uml_driver *driver = dom->conn->privateData; virDomainObjPtr vm = NULL; - char uuidstr[VIR_UUID_STRING_BUFLEN]; int ret = -1; virDomainChrDefPtr chr = NULL; size_t i; @@ -2589,13 +2496,8 @@ umlDomainOpenConsole(virDomainPtr dom, virCheckFlags(0, -1); umlDriverLock(driver); - virUUIDFormat(dom->uuid, uuidstr); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, - _("no domain with matching uuid '%s'"), uuidstr); + if (!(vm = umlDomObjFromDomainLocked(driver, dom->uuid))) goto cleanup; - } if (virDomainOpenConsoleEnsureACL(dom->conn, vm->def) < 0) goto cleanup; @@ -2932,14 +2834,8 @@ umlDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) virCheckFlags(0, -1); - umlDriverLock(driver); - vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); - umlDriverUnlock(driver); - - if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, NULL); - goto cleanup; - } + if (!(vm = umlDomObjFromDomain(driver, dom->uuid))) + return -1; if (virDomainHasManagedSaveImageEnsureACL(dom->conn, vm->def) < 0) goto cleanup; -- 2.13.6

Rather than an empty failed to find, let's provide a bit more knowledge about what we failed to find by using the name string or the id value. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/uml/uml_driver.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index ad526a1e6b..b81da7cb2e 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1393,7 +1393,8 @@ static virDomainPtr umlDomainLookupByID(virConnectPtr conn, umlDriverUnlock(driver); if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, NULL); + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching id '%d'"), id); goto cleanup; } @@ -1441,7 +1442,8 @@ static virDomainPtr umlDomainLookupByName(virConnectPtr conn, umlDriverUnlock(driver); if (!vm) { - virReportError(VIR_ERR_NO_DOMAIN, NULL); + virReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching name '%s'"), name); goto cleanup; } -- 2.13.6

For umlDomObjFromDomainLocked and umlDomainLookupByID let's return a locked and referenced @vm object so that callers can then use the common and more consistent virDomainObjEndAPI in order to handle cleanup rather than needing to know that the returned object is locked and calling virObjectUnlock. This means for some consumers we need to relock the @dom after a virDomainObjListRemove, but before calling virDomainObjEndAPI. The LookupByName already returns the ref counted and locked object, so this will make things more consistent. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/uml/uml_driver.c | 91 ++++++++++++++++++---------------------------------- 1 file changed, 32 insertions(+), 59 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index b81da7cb2e..93d8a9f57c 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -171,7 +171,7 @@ umlDomObjFromDomainLocked(struct uml_driver *driver, virDomainObjPtr vm; char uuidstr[VIR_UUID_STRING_BUFLEN]; - if (!(vm = virDomainObjListFindByUUID(driver->domains, uuid))) { + if (!(vm = virDomainObjListFindByUUIDRef(driver->domains, uuid))) { virUUIDFormat(uuid, uuidstr); virReportError(VIR_ERR_NO_DOMAIN, @@ -773,8 +773,7 @@ static int umlProcessAutoDestroyDom(void *payload, return 0; } - if (!(dom = virDomainObjListFindByUUID(data->driver->domains, - uuid))) { + if (!(dom = virDomainObjListFindByUUIDRef(data->driver->domains, uuid))) { VIR_DEBUG("No domain object to kill"); return 0; } @@ -788,11 +787,10 @@ static int umlProcessAutoDestroyDom(void *payload, if (!dom->persistent) { virDomainObjListRemove(data->driver->domains, dom); - dom = NULL; + virObjectLock(dom); } - if (dom) - virObjectUnlock(dom); + virDomainObjEndAPI(&dom); if (event) umlDomainEventQueue(data->driver, event); virHashRemoveEntry(data->driver->autodestroy, uuidstr); @@ -1389,7 +1387,7 @@ static virDomainPtr umlDomainLookupByID(virConnectPtr conn, virDomainPtr dom = NULL; umlDriverLock(driver); - vm = virDomainObjListFindByID(driver->domains, id); + vm = virDomainObjListFindByIDRef(driver->domains, id); umlDriverUnlock(driver); if (!vm) { @@ -1404,8 +1402,7 @@ static virDomainPtr umlDomainLookupByID(virConnectPtr conn, dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return dom; } @@ -1425,8 +1422,7 @@ static virDomainPtr umlDomainLookupByUUID(virConnectPtr conn, dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return dom; } @@ -1473,8 +1469,7 @@ static int umlDomainIsActive(virDomainPtr dom) ret = virDomainObjIsActive(obj); cleanup: - if (obj) - virObjectUnlock(obj); + virDomainObjEndAPI(&obj); return ret; } @@ -1494,8 +1489,7 @@ static int umlDomainIsPersistent(virDomainPtr dom) ret = obj->persistent; cleanup: - if (obj) - virObjectUnlock(obj); + virDomainObjEndAPI(&obj); return ret; } @@ -1514,8 +1508,7 @@ static int umlDomainIsUpdated(virDomainPtr dom) ret = obj->updated; cleanup: - if (obj) - virObjectUnlock(obj); + virDomainObjEndAPI(&obj); return ret; } @@ -1676,8 +1669,7 @@ static int umlDomainShutdownFlags(virDomainPtr dom, cleanup: VIR_FREE(info); - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return ret; } @@ -1711,15 +1703,13 @@ umlDomainDestroyFlags(virDomainPtr dom, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_DESTROYED); if (!vm->persistent) { - virDomainObjListRemove(driver->domains, - vm); - vm = NULL; + virDomainObjListRemove(driver->domains, vm); + virObjectLock(vm); } ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); if (event) umlDomainEventQueue(driver, event); umlDriverUnlock(driver); @@ -1748,8 +1738,7 @@ static char *umlDomainGetOSType(virDomainPtr dom) { goto cleanup; cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return type; } @@ -1770,8 +1759,7 @@ umlDomainGetMaxMemory(virDomainPtr dom) ret = virDomainDefGetMemoryTotal(vm->def); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return ret; } @@ -1797,8 +1785,7 @@ static int umlDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax) ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return ret; } @@ -1830,8 +1817,7 @@ static int umlDomainSetMemory(virDomainPtr dom, unsigned long newmem) ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return ret; } @@ -1866,8 +1852,7 @@ static int umlDomainGetInfo(virDomainPtr dom, ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return ret; } @@ -1894,8 +1879,7 @@ umlDomainGetState(virDomainPtr dom, ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return ret; } @@ -1921,8 +1905,7 @@ static char *umlDomainGetXMLDesc(virDomainPtr dom, virDomainDefFormatConvertXMLFlags(flags)); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return ret; } @@ -1986,8 +1969,7 @@ static int umlDomainCreateWithFlags(virDomainPtr dom, unsigned int flags) VIR_DOMAIN_EVENT_STARTED_BOOTED); cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); if (event) umlDomainEventQueue(driver, event); umlDriverUnlock(driver); @@ -2034,8 +2016,7 @@ 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); + virDomainObjListRemove(driver->domains, vm); vm = NULL; goto cleanup; } @@ -2085,14 +2066,13 @@ static int umlDomainUndefineFlags(virDomainPtr dom, vm->persistent = 0; } else { virDomainObjListRemove(driver->domains, vm); - vm = NULL; + virObjectLock(vm); } ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); umlDriverUnlock(driver); return ret; } @@ -2198,8 +2178,7 @@ static int umlDomainAttachDevice(virDomainPtr dom, const char *xml) cleanup: virDomainDeviceDefFree(dev); - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); umlDriverUnlock(driver); return ret; } @@ -2306,8 +2285,7 @@ static int umlDomainDetachDevice(virDomainPtr dom, const char *xml) cleanup: virDomainDeviceDefFree(dev); - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); umlDriverUnlock(driver); return ret; } @@ -2348,8 +2326,7 @@ static int umlDomainGetAutostart(virDomainPtr dom, ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); umlDriverUnlock(driver); return ret; } @@ -2413,8 +2390,7 @@ static int umlDomainSetAutostart(virDomainPtr dom, cleanup: VIR_FREE(configFile); VIR_FREE(autostartLink); - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); umlDriverUnlock(driver); return ret; } @@ -2477,8 +2453,7 @@ umlDomainBlockPeek(virDomainPtr dom, cleanup: VIR_FORCE_CLOSE(fd); - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return ret; } @@ -2545,8 +2520,7 @@ umlDomainOpenConsole(virDomainPtr dom, ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); umlDriverUnlock(driver); return ret; } @@ -2845,8 +2819,7 @@ umlDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags) ret = 0; cleanup: - if (vm) - virObjectUnlock(vm); + virDomainObjEndAPI(&vm); return ret; } -- 2.13.6