1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH v2] bhyve: Use virDomainObjListFindBy{UUID|ID}Ref

John Ferlan posted 1 patch 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180402131123.23670-1-jferlan@redhat.com
Test syntax-check passed
src/bhyve/bhyve_driver.c | 62 ++++++++++++++++++------------------------------
1 file changed, 23 insertions(+), 39 deletions(-)
[libvirt] [PATCH v2] bhyve: Use virDomainObjListFindBy{UUID|ID}Ref
Posted by John Ferlan 7 years, 1 month ago 
For bhyveDomObjFromDomain, bhyveDomainLookupByUUID, and
bhyveDomainLookupByID 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 bhyveDomainUndefine and bhyveDomainDestroy since the
virDomainObjListRemove will return an unlocked object, we need to
relock before making the EndAPI call.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---

 Patch 1:
https://www.redhat.com/archives/libvir-list/2018-March/msg00490.html

 of the larger series:
https://www.redhat.com/archives/libvir-list/2018-March/msg00489.html

Changes since v1:

 * From review, use virObjectLock after virDomainObjListRemove when
   calling in a path that would have returned a reffed and locked
   object since virDomainObjListRemove will return an unlocked, but
   reffed object.

 src/bhyve/bhyve_driver.c | 62 ++++++++++++++++++------------------------------
 1 file changed, 23 insertions(+), 39 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index 849d3abcd3..38e6442db7 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -168,7 +168,7 @@ bhyveDomObjFromDomain(virDomainPtr domain)
     bhyveConnPtr privconn = domain->conn->privateData;
     char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-    vm = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
+    vm = virDomainObjListFindByUUIDRef(privconn->domains, domain->uuid);
     if (!vm) {
         virUUIDFormat(domain->uuid, uuidstr);
         virReportError(VIR_ERR_NO_DOMAIN,
@@ -312,8 +312,7 @@ bhyveDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
     ret = 0;
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -338,8 +337,7 @@ bhyveDomainGetState(virDomainPtr domain,
     ret = 0;
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -359,8 +357,7 @@ bhyveDomainGetAutostart(virDomainPtr domain, int *autostart)
     ret = 0;
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -423,8 +420,7 @@ bhyveDomainSetAutostart(virDomainPtr domain, int autostart)
  cleanup:
     VIR_FREE(configFile);
     VIR_FREE(autostartLink);
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -443,8 +439,7 @@ bhyveDomainIsActive(virDomainPtr domain)
     ret = virDomainObjIsActive(obj);
 
  cleanup:
-    if (obj)
-        virObjectUnlock(obj);
+    virDomainObjEndAPI(&obj);
     return ret;
 }
 
@@ -463,8 +458,7 @@ bhyveDomainIsPersistent(virDomainPtr domain)
     ret = obj->persistent;
 
  cleanup:
-    if (obj)
-        virObjectUnlock(obj);
+    virDomainObjEndAPI(&obj);
     return ret;
 }
 
@@ -484,8 +478,7 @@ bhyveDomainGetOSType(virDomainPtr dom)
         goto cleanup;
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -512,8 +505,7 @@ bhyveDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
 
     virObjectUnref(caps);
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -624,14 +616,13 @@ bhyveDomainUndefine(virDomainPtr domain)
         vm->persistent = 0;
     } else {
         virDomainObjListRemove(privconn->domains, vm);
-        vm = NULL;
+        virObjectLock(vm);
     }
 
     ret = 0;
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     if (event)
         virObjectEventStateQueue(privconn->domainEventState, event);
     return ret;
@@ -803,7 +794,7 @@ bhyveDomainLookupByUUID(virConnectPtr conn,
     virDomainObjPtr vm;
     virDomainPtr dom = NULL;
 
-    vm = virDomainObjListFindByUUID(privconn->domains, uuid);
+    vm = virDomainObjListFindByUUIDRef(privconn->domains, uuid);
 
     if (!vm) {
         char uuidstr[VIR_UUID_STRING_BUFLEN];
@@ -819,8 +810,7 @@ bhyveDomainLookupByUUID(virConnectPtr conn,
     dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return dom;
 }
 
@@ -857,7 +847,7 @@ bhyveDomainLookupByID(virConnectPtr conn,
     virDomainObjPtr vm;
     virDomainPtr dom = NULL;
 
-    vm = virDomainObjListFindByID(privconn->domains, id);
+    vm = virDomainObjListFindByIDRef(privconn->domains, id);
 
     if (!vm) {
         virReportError(VIR_ERR_NO_DOMAIN,
@@ -871,8 +861,7 @@ bhyveDomainLookupByID(virConnectPtr conn,
     dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return dom;
 }
 
@@ -913,8 +902,7 @@ bhyveDomainCreateWithFlags(virDomainPtr dom,
                                                   VIR_DOMAIN_EVENT_STARTED_BOOTED);
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     if (event)
         virObjectEventStateQueue(privconn->domainEventState, event);
     return ret;
@@ -1024,12 +1012,11 @@ bhyveDomainDestroy(virDomainPtr dom)
 
     if (!vm->persistent) {
         virDomainObjListRemove(privconn->domains, vm);
-        vm = NULL;
+        virObjectLock(vm);
     }
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     if (event)
         virObjectEventStateQueue(privconn->domainEventState, event);
     return ret;
@@ -1056,8 +1043,7 @@ bhyveDomainShutdown(virDomainPtr dom)
     ret = virBhyveProcessShutdown(vm);
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -1100,8 +1086,7 @@ bhyveDomainOpenConsole(virDomainPtr dom,
     ret = 0;
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -1143,7 +1128,7 @@ bhyveDomainSetMetadata(virDomainPtr dom,
 
  cleanup:
     virObjectUnref(caps);
-    virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -1165,7 +1150,7 @@ bhyveDomainGetMetadata(virDomainPtr dom,
     ret = virDomainObjGetMetadata(vm, type, uri, flags);
 
  cleanup:
-    virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
@@ -1548,8 +1533,7 @@ bhyveDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags)
     ret = 0;
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return ret;
 }
 
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] bhyve: Use virDomainObjListFindBy{UUID|ID}Ref
Posted by John Ferlan 7 years, 1 month ago 
ping?

Tks, -

John

On 04/02/2018 09:11 AM, John Ferlan wrote:
> For bhyveDomObjFromDomain, bhyveDomainLookupByUUID, and
> bhyveDomainLookupByID 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 bhyveDomainUndefine and bhyveDomainDestroy since the
> virDomainObjListRemove will return an unlocked object, we need to
> relock before making the EndAPI call.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> 
>  Patch 1:
> https://www.redhat.com/archives/libvir-list/2018-March/msg00490.html
> 
>  of the larger series:
> https://www.redhat.com/archives/libvir-list/2018-March/msg00489.html
> 
> Changes since v1:
> 
>  * From review, use virObjectLock after virDomainObjListRemove when
>    calling in a path that would have returned a reffed and locked
>    object since virDomainObjListRemove will return an unlocked, but
>    reffed object.
> 
>  src/bhyve/bhyve_driver.c | 62 ++++++++++++++++++------------------------------
>  1 file changed, 23 insertions(+), 39 deletions(-)
> 
> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> index 849d3abcd3..38e6442db7 100644
> --- a/src/bhyve/bhyve_driver.c
> +++ b/src/bhyve/bhyve_driver.c
> @@ -168,7 +168,7 @@ bhyveDomObjFromDomain(virDomainPtr domain)
>      bhyveConnPtr privconn = domain->conn->privateData;
>      char uuidstr[VIR_UUID_STRING_BUFLEN];
>  
> -    vm = virDomainObjListFindByUUID(privconn->domains, domain->uuid);
> +    vm = virDomainObjListFindByUUIDRef(privconn->domains, domain->uuid);
>      if (!vm) {
>          virUUIDFormat(domain->uuid, uuidstr);
>          virReportError(VIR_ERR_NO_DOMAIN,
> @@ -312,8 +312,7 @@ bhyveDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info)
>      ret = 0;
>  
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>  
> @@ -338,8 +337,7 @@ bhyveDomainGetState(virDomainPtr domain,
>      ret = 0;
>  
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>  
> @@ -359,8 +357,7 @@ bhyveDomainGetAutostart(virDomainPtr domain, int *autostart)
>      ret = 0;
>  
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>  
> @@ -423,8 +420,7 @@ bhyveDomainSetAutostart(virDomainPtr domain, int autostart)
>   cleanup:
>      VIR_FREE(configFile);
>      VIR_FREE(autostartLink);
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>  
> @@ -443,8 +439,7 @@ bhyveDomainIsActive(virDomainPtr domain)
>      ret = virDomainObjIsActive(obj);
>  
>   cleanup:
> -    if (obj)
> -        virObjectUnlock(obj);
> +    virDomainObjEndAPI(&obj);
>      return ret;
>  }
>  
> @@ -463,8 +458,7 @@ bhyveDomainIsPersistent(virDomainPtr domain)
>      ret = obj->persistent;
>  
>   cleanup:
> -    if (obj)
> -        virObjectUnlock(obj);
> +    virDomainObjEndAPI(&obj);
>      return ret;
>  }
>  
> @@ -484,8 +478,7 @@ bhyveDomainGetOSType(virDomainPtr dom)
>          goto cleanup;
>  
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>  
> @@ -512,8 +505,7 @@ bhyveDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
>  
>      virObjectUnref(caps);
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>  
> @@ -624,14 +616,13 @@ bhyveDomainUndefine(virDomainPtr domain)
>          vm->persistent = 0;
>      } else {
>          virDomainObjListRemove(privconn->domains, vm);
> -        vm = NULL;
> +        virObjectLock(vm);
>      }
>  
>      ret = 0;
>  
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      if (event)
>          virObjectEventStateQueue(privconn->domainEventState, event);
>      return ret;
> @@ -803,7 +794,7 @@ bhyveDomainLookupByUUID(virConnectPtr conn,
>      virDomainObjPtr vm;
>      virDomainPtr dom = NULL;
>  
> -    vm = virDomainObjListFindByUUID(privconn->domains, uuid);
> +    vm = virDomainObjListFindByUUIDRef(privconn->domains, uuid);
>  
>      if (!vm) {
>          char uuidstr[VIR_UUID_STRING_BUFLEN];
> @@ -819,8 +810,7 @@ bhyveDomainLookupByUUID(virConnectPtr conn,
>      dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
>  
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return dom;
>  }
>  
> @@ -857,7 +847,7 @@ bhyveDomainLookupByID(virConnectPtr conn,
>      virDomainObjPtr vm;
>      virDomainPtr dom = NULL;
>  
> -    vm = virDomainObjListFindByID(privconn->domains, id);
> +    vm = virDomainObjListFindByIDRef(privconn->domains, id);
>  
>      if (!vm) {
>          virReportError(VIR_ERR_NO_DOMAIN,
> @@ -871,8 +861,7 @@ bhyveDomainLookupByID(virConnectPtr conn,
>      dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
>  
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return dom;
>  }
>  
> @@ -913,8 +902,7 @@ bhyveDomainCreateWithFlags(virDomainPtr dom,
>                                                    VIR_DOMAIN_EVENT_STARTED_BOOTED);
>  
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      if (event)
>          virObjectEventStateQueue(privconn->domainEventState, event);
>      return ret;
> @@ -1024,12 +1012,11 @@ bhyveDomainDestroy(virDomainPtr dom)
>  
>      if (!vm->persistent) {
>          virDomainObjListRemove(privconn->domains, vm);
> -        vm = NULL;
> +        virObjectLock(vm);
>      }
>  
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      if (event)
>          virObjectEventStateQueue(privconn->domainEventState, event);
>      return ret;
> @@ -1056,8 +1043,7 @@ bhyveDomainShutdown(virDomainPtr dom)
>      ret = virBhyveProcessShutdown(vm);
>  
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>  
> @@ -1100,8 +1086,7 @@ bhyveDomainOpenConsole(virDomainPtr dom,
>      ret = 0;
>  
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>  
> @@ -1143,7 +1128,7 @@ bhyveDomainSetMetadata(virDomainPtr dom,
>  
>   cleanup:
>      virObjectUnref(caps);
> -    virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>  
> @@ -1165,7 +1150,7 @@ bhyveDomainGetMetadata(virDomainPtr dom,
>      ret = virDomainObjGetMetadata(vm, type, uri, flags);
>  
>   cleanup:
> -    virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>  
> @@ -1548,8 +1533,7 @@ bhyveDomainHasManagedSaveImage(virDomainPtr domain, unsigned int flags)
>      ret = 0;
>  
>   cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>      return ret;
>  }
>  
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] bhyve: Use virDomainObjListFindBy{UUID|ID}Ref
Posted by Michal Privoznik 7 years, 1 month ago 
On 04/02/2018 03:11 PM, John Ferlan wrote:
> For bhyveDomObjFromDomain, bhyveDomainLookupByUUID, and
> bhyveDomainLookupByID 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 bhyveDomainUndefine and bhyveDomainDestroy since the
> virDomainObjListRemove will return an unlocked object, we need to
> relock before making the EndAPI call.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
> 
>  Patch 1:
> https://www.redhat.com/archives/libvir-list/2018-March/msg00490.html
> 
>  of the larger series:
> https://www.redhat.com/archives/libvir-list/2018-March/msg00489.html
> 
> Changes since v1:
> 
>  * From review, use virObjectLock after virDomainObjListRemove when
>    calling in a path that would have returned a reffed and locked
>    object since virDomainObjListRemove will return an unlocked, but
>    reffed object.
> 
>  src/bhyve/bhyve_driver.c | 62 ++++++++++++++++++------------------------------
>  1 file changed, 23 insertions(+), 39 deletions(-)

ACK

Michal

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

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.