For vmwareDomObjFromDomainLocked and vmwareDomainLookupByID
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.

The LookupByName already returns the ref counted and locked object,
so this will make things more consistent.

For vmwareDomainUndefineFlags and vmwareDomainShutdownFlags since
virDomainObjListRemove will return an unlocked object, we need to
relock before making the EndAPI call.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/vmware/vmware_driver.c | 53 +++++++++++++++++-----------------------------
 1 file changed, 19 insertions(+), 34 deletions(-)

diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index d17fdfe3be..643c85d4f8 100644
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -67,7 +67,7 @@ vmwareDomObjFromDomainLocked(struct vmware_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,
@@ -510,13 +510,12 @@ vmwareDomainShutdownFlags(virDomainPtr dom,
 
     if (!vm->persistent) {
         virDomainObjListRemove(driver->domains, vm);
-        vm = NULL;
+        virObjectLock(vm);
     }
 
     ret = 0;
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     vmwareDriverUnlock(driver);
     return ret;
 }
@@ -577,8 +576,7 @@ vmwareDomainSuspend(virDomainPtr dom)
     ret = 0;
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -619,8 +617,7 @@ vmwareDomainResume(virDomainPtr dom)
     ret = 0;
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -660,8 +657,7 @@ vmwareDomainReboot(virDomainPtr dom, unsigned int flags)
     ret = 0;
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -772,8 +768,7 @@ vmwareDomainCreateWithFlags(virDomainPtr dom,
     ret = vmwareStartVM(driver, vm);
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     vmwareDriverUnlock(driver);
     return ret;
 }
@@ -811,14 +806,13 @@ vmwareDomainUndefineFlags(virDomainPtr dom,
         vm->persistent = 0;
     } else {
         virDomainObjListRemove(driver->domains, vm);
-        vm = NULL;
+        virObjectLock(vm);
     }
 
     ret = 0;
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     vmwareDriverUnlock(driver);
     return ret;
 }
@@ -837,7 +831,7 @@ vmwareDomainLookupByID(virConnectPtr conn, int id)
     virDomainPtr dom = NULL;
 
     vmwareDriverLock(driver);
-    vm = virDomainObjListFindByID(driver->domains, id);
+    vm = virDomainObjListFindByIDRef(driver->domains, id);
     vmwareDriverUnlock(driver);
 
     if (!vm) {
@@ -849,8 +843,7 @@ vmwareDomainLookupByID(virConnectPtr conn, int id)
     dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return dom;
 }
 
@@ -866,8 +859,7 @@ vmwareDomainGetOSType(virDomainPtr dom)
 
     ignore_value(VIR_STRDUP(ret, virDomainOSTypeToString(vm->def->os.type)));
 
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -884,8 +876,7 @@ vmwareDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid)
 
     dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return dom;
 }
 
@@ -925,8 +916,7 @@ vmwareDomainIsActive(virDomainPtr dom)
 
     ret = virDomainObjIsActive(obj);
 
-    if (obj)
-        virObjectUnlock(obj);
+    virDomainObjEndAPI(&obj);
     return ret;
 }
 
@@ -943,8 +933,7 @@ vmwareDomainIsPersistent(virDomainPtr dom)
 
     ret = obj->persistent;
 
-    if (obj)
-        virObjectUnlock(obj);
+    virDomainObjEndAPI(&obj);
     return ret;
 }
 
@@ -964,8 +953,7 @@ vmwareDomainGetXMLDesc(virDomainPtr dom, unsigned int flags)
     ret = virDomainDefFormat(vm->def, driver->caps,
                              virDomainDefFormatConvertXMLFlags(flags));
 
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -1097,8 +1085,7 @@ vmwareDomainGetInfo(virDomainPtr dom, virDomainInfoPtr info)
     ret = 0;
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -1124,8 +1111,7 @@ vmwareDomainGetState(virDomainPtr dom,
     ret = 0;
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -1167,8 +1153,7 @@ vmwareDomainHasManagedSaveImage(virDomainPtr dom, unsigned int flags)
 
     ret = 0;
 
-    if (obj)
-        virObjectUnlock(obj);
+    virDomainObjEndAPI(&obj);
     return ret;
 }
 
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Apr 02, 2018 at 09:16:40AM -0400, John Ferlan wrote:
> For vmwareDomObjFromDomainLocked and vmwareDomainLookupByID
> 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.
> 
> The LookupByName already returns the ref counted and locked object,
> so this will make things more consistent.
> 
> For vmwareDomainUndefineFlags and vmwareDomainShutdownFlags since
> virDomainObjListRemove will return an unlocked object, we need to
> relock before making the EndAPI call.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>  src/vmware/vmware_driver.c | 53 +++++++++++++++++-----------------------------
>  1 file changed, 19 insertions(+), 34 deletions(-)

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list