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

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(a)redhat.com&gt; --- 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);