Rename/remove virDomainObjListFindBy{UUID|ID}Ref

[libvirt] [PATCH v2 2/2] conf: Rework/rename virDomainObjListFindByIDRef

Posted by John Ferlan 7 years, 9 months ago

Rework the code such that virDomainObjListFindByID will always
return a locked/ref counted object so that the callers can
always do the same cleanup logic to call virDomainObjEndAPI.
Makes accessing the objects much more consistent.

NB:
There were 2 callers (lxcDomainLookupByID and qemuDomainLookupByID)
that were already using the ByID name, but not virDomainObjEndAPI -
these were changed as well in this update/patch.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 src/bhyve/bhyve_driver.c    |  2 +-
 src/conf/virdomainobjlist.c | 35 +++++++++--------------------------
 src/conf/virdomainobjlist.h |  2 --
 src/libvirt_private.syms    |  1 -
 src/libxl/libxl_domain.c    |  2 +-
 src/libxl/libxl_driver.c    |  2 +-
 src/lxc/lxc_driver.c        |  3 +--
 src/openvz/openvz_driver.c  |  2 +-
 src/qemu/qemu_driver.c      |  5 ++---
 src/test/test_driver.c      |  2 +-
 src/uml/uml_driver.c        |  2 +-
 src/vmware/vmware_driver.c  |  2 +-
 src/vz/vz_driver.c          |  2 +-
 13 files changed, 20 insertions(+), 42 deletions(-)

diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
index bd8c6b30ed..768578a43f 100644
--- a/src/bhyve/bhyve_driver.c
+++ b/src/bhyve/bhyve_driver.c
@@ -844,7 +844,7 @@ bhyveDomainLookupByID(virConnectPtr conn,
     virDomainObjPtr vm;
     virDomainPtr dom = NULL;
 
-    vm = virDomainObjListFindByIDRef(privconn->domains, id);
+    vm = virDomainObjListFindByID(privconn->domains, id);
 
     if (!vm) {
         virReportError(VIR_ERR_NO_DOMAIN,
diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
index 02b52465d0..d57ed10a5f 100644
--- a/src/conf/virdomainobjlist.c
+++ b/src/conf/virdomainobjlist.c
@@ -109,44 +109,27 @@ static int virDomainObjListSearchID(const void *payload,
     return want;
 }
 
-static virDomainObjPtr
-virDomainObjListFindByIDInternal(virDomainObjListPtr doms,
-                                 int id,
-                                 bool ref)
+
+virDomainObjPtr
+virDomainObjListFindByID(virDomainObjListPtr doms,
+                         int id)
 {
     virDomainObjPtr obj;
+
     virObjectRWLockRead(doms);
     obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id, NULL);
-    if (ref) {
-        virObjectRef(obj);
-        virObjectRWUnlock(doms);
-    }
+    virObjectRef(obj);
+    virObjectRWUnlock(doms);
     if (obj) {
         virObjectLock(obj);
         if (obj->removing) {
             virObjectUnlock(obj);
-            if (ref)
-                virObjectUnref(obj);
+            virObjectUnref(obj);
             obj = NULL;
         }
     }
-    if (!ref)
-        virObjectRWUnlock(doms);
-    return obj;
-}
-
-virDomainObjPtr
-virDomainObjListFindByID(virDomainObjListPtr doms,
-                         int id)
-{
-    return virDomainObjListFindByIDInternal(doms, id, false);
-}
 
-virDomainObjPtr
-virDomainObjListFindByIDRef(virDomainObjListPtr doms,
-                            int id)
-{
-    return virDomainObjListFindByIDInternal(doms, id, true);
+    return obj;
 }
 
 
diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h
index 1b77a95ba9..7e2dece3aa 100644
--- a/src/conf/virdomainobjlist.h
+++ b/src/conf/virdomainobjlist.h
@@ -34,8 +34,6 @@ virDomainObjListPtr virDomainObjListNew(void);
 
 virDomainObjPtr virDomainObjListFindByID(virDomainObjListPtr doms,
                                          int id);
-virDomainObjPtr virDomainObjListFindByIDRef(virDomainObjListPtr doms,
-                                            int id);
 virDomainObjPtr virDomainObjListFindByUUID(virDomainObjListPtr doms,
                                            const unsigned char *uuid);
 virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f6c76d4341..d2728749fb 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -952,7 +952,6 @@ virDomainObjListCollect;
 virDomainObjListConvert;
 virDomainObjListExport;
 virDomainObjListFindByID;
-virDomainObjListFindByIDRef;
 virDomainObjListFindByName;
 virDomainObjListFindByUUID;
 virDomainObjListForEach;
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index e401327df4..d4859d6707 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -450,7 +450,7 @@ libxlDomainShutdownThread(void *opaque)
 
     cfg = libxlDriverConfigGet(driver);
 
-    vm = virDomainObjListFindByIDRef(driver->domains, ev->domid);
+    vm = virDomainObjListFindByID(driver->domains, ev->domid);
     if (!vm) {
         VIR_INFO("Received event for unknown domain ID %d", ev->domid);
         goto cleanup;
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 24553c7272..55a93a489b 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -1082,7 +1082,7 @@ libxlDomainLookupByID(virConnectPtr conn, int id)
     virDomainObjPtr vm;
     virDomainPtr dom = NULL;
 
-    vm = virDomainObjListFindByIDRef(driver->domains, id);
+    vm = virDomainObjListFindByID(driver->domains, id);
     if (!vm) {
         virReportError(VIR_ERR_NO_DOMAIN, NULL);
         goto cleanup;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 8773a3fd45..ca01d369d5 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -262,8 +262,7 @@ static virDomainPtr lxcDomainLookupByID(virConnectPtr conn,
     dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return dom;
 }
 
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
index f5a5526d28..c10d6df663 100644
--- a/src/openvz/openvz_driver.c
+++ b/src/openvz/openvz_driver.c
@@ -342,7 +342,7 @@ static virDomainPtr openvzDomainLookupByID(virConnectPtr conn,
     virDomainPtr dom = NULL;
 
     openvzDriverLock(driver);
-    vm = virDomainObjListFindByIDRef(driver->domains, id);
+    vm = virDomainObjListFindByID(driver->domains, id);
     openvzDriverUnlock(driver);
 
     if (!vm) {
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8a5b049296..7484b00e23 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1529,7 +1529,7 @@ static virDomainPtr qemuDomainLookupByID(virConnectPtr conn,
     virDomainObjPtr vm;
     virDomainPtr dom = NULL;
 
-    vm  = virDomainObjListFindByID(driver->domains, id);
+    vm = virDomainObjListFindByID(driver->domains, id);
 
     if (!vm) {
         virReportError(VIR_ERR_NO_DOMAIN,
@@ -1543,8 +1543,7 @@ static virDomainPtr qemuDomainLookupByID(virConnectPtr conn,
     dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
 
  cleanup:
-    if (vm)
-        virObjectUnlock(vm);
+    virDomainObjEndAPI(&vm);
     return dom;
 }
 
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 249932de06..a1888c0c9f 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -1708,7 +1708,7 @@ static virDomainPtr testDomainLookupByID(virConnectPtr conn,
     virDomainPtr ret = NULL;
     virDomainObjPtr dom;
 
-    if (!(dom = virDomainObjListFindByIDRef(privconn->domains, id))) {
+    if (!(dom = virDomainObjListFindByID(privconn->domains, id))) {
         virReportError(VIR_ERR_NO_DOMAIN, NULL);
         return NULL;
     }
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 0bafd0902e..b50ba1ba64 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -1379,7 +1379,7 @@ static virDomainPtr umlDomainLookupByID(virConnectPtr conn,
     virDomainPtr dom = NULL;
 
     umlDriverLock(driver);
-    vm = virDomainObjListFindByIDRef(driver->domains, id);
+    vm = virDomainObjListFindByID(driver->domains, id);
     umlDriverUnlock(driver);
 
     if (!vm) {
diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
index f94d192fd1..21c10b6605 100644
--- a/src/vmware/vmware_driver.c
+++ b/src/vmware/vmware_driver.c
@@ -825,7 +825,7 @@ vmwareDomainLookupByID(virConnectPtr conn, int id)
     virDomainPtr dom = NULL;
 
     vmwareDriverLock(driver);
-    vm = virDomainObjListFindByIDRef(driver->domains, id);
+    vm = virDomainObjListFindByID(driver->domains, id);
     vmwareDriverUnlock(driver);
 
     if (!vm) {
diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
index 8f8720a037..d3fcae491a 100644
--- a/src/vz/vz_driver.c
+++ b/src/vz/vz_driver.c
@@ -555,7 +555,7 @@ vzDomainLookupByID(virConnectPtr conn, int id)
     virDomainPtr ret = NULL;
     virDomainObjPtr dom;
 
-    dom = virDomainObjListFindByIDRef(privconn->driver->domains, id);
+    dom = virDomainObjListFindByID(privconn->driver->domains, id);
 
     if (dom == NULL) {
         virReportError(VIR_ERR_NO_DOMAIN, NULL);
-- 
2.13.6

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

Re: [libvirt] [PATCH v2 2/2] conf: Rework/rename virDomainObjListFindByIDRef

Posted by Jim Fehlig 7 years, 9 months ago

On 04/20/2018 07:08 AM, John Ferlan wrote:
> Rework the code such that virDomainObjListFindByID will always
> return a locked/ref counted object so that the callers can
> always do the same cleanup logic to call virDomainObjEndAPI.
> Makes accessing the objects much more consistent.
> 
> NB:
> There were 2 callers (lxcDomainLookupByID and qemuDomainLookupByID)
> that were already using the ByID name, but not virDomainObjEndAPI -
> these were changed as well in this update/patch.
> 
> Signed-off-by: John Ferlan <jferlan@redhat.com>
> ---
>   src/bhyve/bhyve_driver.c    |  2 +-
>   src/conf/virdomainobjlist.c | 35 +++++++++--------------------------
>   src/conf/virdomainobjlist.h |  2 --
>   src/libvirt_private.syms    |  1 -
>   src/libxl/libxl_domain.c    |  2 +-
>   src/libxl/libxl_driver.c    |  2 +-
>   src/lxc/lxc_driver.c        |  3 +--
>   src/openvz/openvz_driver.c  |  2 +-
>   src/qemu/qemu_driver.c      |  5 ++---
>   src/test/test_driver.c      |  2 +-
>   src/uml/uml_driver.c        |  2 +-
>   src/vmware/vmware_driver.c  |  2 +-
>   src/vz/vz_driver.c          |  2 +-
>   13 files changed, 20 insertions(+), 42 deletions(-)

Reviewed-by: Jim Fehlig <jfehlig@suse.com>

Regards,
Jim

> 
> diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c
> index bd8c6b30ed..768578a43f 100644
> --- a/src/bhyve/bhyve_driver.c
> +++ b/src/bhyve/bhyve_driver.c
> @@ -844,7 +844,7 @@ bhyveDomainLookupByID(virConnectPtr conn,
>       virDomainObjPtr vm;
>       virDomainPtr dom = NULL;
>   
> -    vm = virDomainObjListFindByIDRef(privconn->domains, id);
> +    vm = virDomainObjListFindByID(privconn->domains, id);
>   
>       if (!vm) {
>           virReportError(VIR_ERR_NO_DOMAIN,
> diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c
> index 02b52465d0..d57ed10a5f 100644
> --- a/src/conf/virdomainobjlist.c
> +++ b/src/conf/virdomainobjlist.c
> @@ -109,44 +109,27 @@ static int virDomainObjListSearchID(const void *payload,
>       return want;
>   }
>   
> -static virDomainObjPtr
> -virDomainObjListFindByIDInternal(virDomainObjListPtr doms,
> -                                 int id,
> -                                 bool ref)
> +
> +virDomainObjPtr
> +virDomainObjListFindByID(virDomainObjListPtr doms,
> +                         int id)
>   {
>       virDomainObjPtr obj;
> +
>       virObjectRWLockRead(doms);
>       obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id, NULL);
> -    if (ref) {
> -        virObjectRef(obj);
> -        virObjectRWUnlock(doms);
> -    }
> +    virObjectRef(obj);
> +    virObjectRWUnlock(doms);
>       if (obj) {
>           virObjectLock(obj);
>           if (obj->removing) {
>               virObjectUnlock(obj);
> -            if (ref)
> -                virObjectUnref(obj);
> +            virObjectUnref(obj);
>               obj = NULL;
>           }
>       }
> -    if (!ref)
> -        virObjectRWUnlock(doms);
> -    return obj;
> -}
> -
> -virDomainObjPtr
> -virDomainObjListFindByID(virDomainObjListPtr doms,
> -                         int id)
> -{
> -    return virDomainObjListFindByIDInternal(doms, id, false);
> -}
>   
> -virDomainObjPtr
> -virDomainObjListFindByIDRef(virDomainObjListPtr doms,
> -                            int id)
> -{
> -    return virDomainObjListFindByIDInternal(doms, id, true);
> +    return obj;
>   }
>   
>   
> diff --git a/src/conf/virdomainobjlist.h b/src/conf/virdomainobjlist.h
> index 1b77a95ba9..7e2dece3aa 100644
> --- a/src/conf/virdomainobjlist.h
> +++ b/src/conf/virdomainobjlist.h
> @@ -34,8 +34,6 @@ virDomainObjListPtr virDomainObjListNew(void);
>   
>   virDomainObjPtr virDomainObjListFindByID(virDomainObjListPtr doms,
>                                            int id);
> -virDomainObjPtr virDomainObjListFindByIDRef(virDomainObjListPtr doms,
> -                                            int id);
>   virDomainObjPtr virDomainObjListFindByUUID(virDomainObjListPtr doms,
>                                              const unsigned char *uuid);
>   virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms,
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index f6c76d4341..d2728749fb 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -952,7 +952,6 @@ virDomainObjListCollect;
>   virDomainObjListConvert;
>   virDomainObjListExport;
>   virDomainObjListFindByID;
> -virDomainObjListFindByIDRef;
>   virDomainObjListFindByName;
>   virDomainObjListFindByUUID;
>   virDomainObjListForEach;
> diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> index e401327df4..d4859d6707 100644
> --- a/src/libxl/libxl_domain.c
> +++ b/src/libxl/libxl_domain.c
> @@ -450,7 +450,7 @@ libxlDomainShutdownThread(void *opaque)
>   
>       cfg = libxlDriverConfigGet(driver);
>   
> -    vm = virDomainObjListFindByIDRef(driver->domains, ev->domid);
> +    vm = virDomainObjListFindByID(driver->domains, ev->domid);
>       if (!vm) {
>           VIR_INFO("Received event for unknown domain ID %d", ev->domid);
>           goto cleanup;
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 24553c7272..55a93a489b 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -1082,7 +1082,7 @@ libxlDomainLookupByID(virConnectPtr conn, int id)
>       virDomainObjPtr vm;
>       virDomainPtr dom = NULL;
>   
> -    vm = virDomainObjListFindByIDRef(driver->domains, id);
> +    vm = virDomainObjListFindByID(driver->domains, id);
>       if (!vm) {
>           virReportError(VIR_ERR_NO_DOMAIN, NULL);
>           goto cleanup;
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 8773a3fd45..ca01d369d5 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -262,8 +262,7 @@ static virDomainPtr lxcDomainLookupByID(virConnectPtr conn,
>       dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
>   
>    cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>       return dom;
>   }
>   
> diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c
> index f5a5526d28..c10d6df663 100644
> --- a/src/openvz/openvz_driver.c
> +++ b/src/openvz/openvz_driver.c
> @@ -342,7 +342,7 @@ static virDomainPtr openvzDomainLookupByID(virConnectPtr conn,
>       virDomainPtr dom = NULL;
>   
>       openvzDriverLock(driver);
> -    vm = virDomainObjListFindByIDRef(driver->domains, id);
> +    vm = virDomainObjListFindByID(driver->domains, id);
>       openvzDriverUnlock(driver);
>   
>       if (!vm) {
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 8a5b049296..7484b00e23 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1529,7 +1529,7 @@ static virDomainPtr qemuDomainLookupByID(virConnectPtr conn,
>       virDomainObjPtr vm;
>       virDomainPtr dom = NULL;
>   
> -    vm  = virDomainObjListFindByID(driver->domains, id);
> +    vm = virDomainObjListFindByID(driver->domains, id);
>   
>       if (!vm) {
>           virReportError(VIR_ERR_NO_DOMAIN,
> @@ -1543,8 +1543,7 @@ static virDomainPtr qemuDomainLookupByID(virConnectPtr conn,
>       dom = virGetDomain(conn, vm->def->name, vm->def->uuid, vm->def->id);
>   
>    cleanup:
> -    if (vm)
> -        virObjectUnlock(vm);
> +    virDomainObjEndAPI(&vm);
>       return dom;
>   }
>   
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 249932de06..a1888c0c9f 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -1708,7 +1708,7 @@ static virDomainPtr testDomainLookupByID(virConnectPtr conn,
>       virDomainPtr ret = NULL;
>       virDomainObjPtr dom;
>   
> -    if (!(dom = virDomainObjListFindByIDRef(privconn->domains, id))) {
> +    if (!(dom = virDomainObjListFindByID(privconn->domains, id))) {
>           virReportError(VIR_ERR_NO_DOMAIN, NULL);
>           return NULL;
>       }
> diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
> index 0bafd0902e..b50ba1ba64 100644
> --- a/src/uml/uml_driver.c
> +++ b/src/uml/uml_driver.c
> @@ -1379,7 +1379,7 @@ static virDomainPtr umlDomainLookupByID(virConnectPtr conn,
>       virDomainPtr dom = NULL;
>   
>       umlDriverLock(driver);
> -    vm = virDomainObjListFindByIDRef(driver->domains, id);
> +    vm = virDomainObjListFindByID(driver->domains, id);
>       umlDriverUnlock(driver);
>   
>       if (!vm) {
> diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
> index f94d192fd1..21c10b6605 100644
> --- a/src/vmware/vmware_driver.c
> +++ b/src/vmware/vmware_driver.c
> @@ -825,7 +825,7 @@ vmwareDomainLookupByID(virConnectPtr conn, int id)
>       virDomainPtr dom = NULL;
>   
>       vmwareDriverLock(driver);
> -    vm = virDomainObjListFindByIDRef(driver->domains, id);
> +    vm = virDomainObjListFindByID(driver->domains, id);
>       vmwareDriverUnlock(driver);
>   
>       if (!vm) {
> diff --git a/src/vz/vz_driver.c b/src/vz/vz_driver.c
> index 8f8720a037..d3fcae491a 100644
> --- a/src/vz/vz_driver.c
> +++ b/src/vz/vz_driver.c
> @@ -555,7 +555,7 @@ vzDomainLookupByID(virConnectPtr conn, int id)
>       virDomainPtr ret = NULL;
>       virDomainObjPtr dom;
>   
> -    dom = virDomainObjListFindByIDRef(privconn->driver->domains, id);
> +    dom = virDomainObjListFindByID(privconn->driver->domains, id);
>   
>       if (dom == NULL) {
>           virReportError(VIR_ERR_NO_DOMAIN, NULL);
> 

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

[libvirt] [PATCH v2 1/2] conf: Rework/rename virDomainObjListFindByUUIDRef
[libvirt] [PATCH v2 2/2] conf: Rework/rename virDomainObjListFindByIDRef