[libvirt] [PATCH 0/2] Prefer virDomainObjListFindByUUID wherever possible
This should go on the top of [1] so that both virDomainObjListFindByUUID and virDomainObjListFindByName are both O(1). Then, the only function left that is more time complex is virDomainObjListFindByID. Unfortunately, we can't drop it completely, but we can use it less. 1: https://www.redhat.com/archives/libvir-list/2015-April/msg01168.html Michal Privoznik (2): uml: s/virDomainObjListFindByID/virDomainObjListFindByUUID/ libxl: s/virDomainObjListFindByID/virDomainObjListFindByUUID/ src/libxl/libxl_domain.c | 2 +- src/uml/uml_driver.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) -- 2.0.5
ListFindByID() still requires to step through items in the hash table (in the worst case scenario through all of them), lock each one and compare whether we've found what we're looking for. This is suboptimal as locking a domain object means we need to wait for the current API running over the object to finish. Unfortunately, we can't drop the function completely because we have this public API virDomainLookupByID which we can't drop. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/uml/uml_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 9cee541..af94f63 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1670,7 +1670,7 @@ static int umlDomainShutdownFlags(virDomainPtr dom, virCheckFlags(0, -1); umlDriverLock(driver); - vm = virDomainObjListFindByID(driver->domains, dom->id); + vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); umlDriverUnlock(driver); if (!vm) { virReportError(VIR_ERR_NO_DOMAIN, @@ -1715,7 +1715,7 @@ umlDomainDestroyFlags(virDomainPtr dom, virCheckFlags(0, -1); umlDriverLock(driver); - vm = virDomainObjListFindByID(driver->domains, dom->id); + vm = virDomainObjListFindByUUID(driver->domains, dom->uuid); if (!vm) { virReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching id %d"), dom->id); -- 2.0.5
On Fri, Apr 24, 2015 at 10:24:14 +0200, Michal Privoznik wrote:
ListFindByID() still requires to step through items in the hash table (in the worst case scenario through all of them), lock each one and compare whether we've found what we're looking for. This is suboptimal as locking a domain object means we need to wait for the current API running over the object to finish.
Unfortunately, we can't drop the function completely because we have this public API virDomainLookupByID which we can't drop.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/uml/uml_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ACK, Peter
On 24.04.2015 11:15, Peter Krempa wrote:
On Fri, Apr 24, 2015 at 10:24:14 +0200, Michal Privoznik wrote:
ListFindByID() still requires to step through items in the hash table (in the worst case scenario through all of them), lock each one and compare whether we've found what we're looking for. This is suboptimal as locking a domain object means we need to wait for the current API running over the object to finish.
Unfortunately, we can't drop the function completely because we have this public API virDomainLookupByID which we can't drop.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/uml/uml_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ACK,
Peter
Thanks, I've pushed this one. Jim, can you please look at 2/2? Seems like you're the right guy for libxl driver. Michal
Like in previous commit, lets use FindByUUID() instead of ListFindByID(). The latter is suboptimal as it needs to iterate over each item in the domain object list, lock it and compare the IDs. If an object is already locked, we must wait until it's unlocked. During this wait, we keep the whole list locked, and potentially block other thread trying to access the list. There's no such problem with ListFindByUUID() since UUID is the key to the hash table, therefore it can be looked up without need for locking each single domain object. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libxl/libxl_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 3039427..55d1313 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -494,7 +494,7 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) if (xl_reason == LIBXL_SHUTDOWN_REASON_SUSPEND) goto error; - vm = virDomainObjListFindByID(driver->domains, event->domid); + vm = virDomainObjListFindByUUID(driver->domains, event->domuuid.uuid); if (!vm) { VIR_INFO("Received event for unknown domain ID %d", event->domid); goto error; -- 2.0.5
Michal Privoznik wrote:
Like in previous commit, lets use FindByUUID() instead of ListFindByID(). The latter is suboptimal as it needs to iterate over each item in the domain object list, lock it and compare the IDs. If an object is already locked, we must wait until it's unlocked. During this wait, we keep the whole list locked, and potentially block other thread trying to access the list.
There's no such problem with ListFindByUUID() since UUID is the key to the hash table, therefore it can be looked up without need for locking each single domain object.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/libxl/libxl_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 3039427..55d1313 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -494,7 +494,7 @@ libxlDomainEventHandler(void *data, VIR_LIBXL_EVENT_CONST libxl_event *event) if (xl_reason == LIBXL_SHUTDOWN_REASON_SUSPEND) goto error;
- vm = virDomainObjListFindByID(driver->domains, event->domid); + vm = virDomainObjListFindByUUID(driver->domains, event->domuuid.uuid);
Sadly, this won't work. event->domuuid is not populated when libxl invokes the event handler Breakpoint 1, libxlDomainEventHandler (data=0x7fffe0012310, event=0x55555588abb0) at libxl/libxl_domain.c:477 (gdb) p event->domuuid $2 = {uuid = '\000' <repeats 15 times>} Regards, Jim
if (!vm) { VIR_INFO("Received event for unknown domain ID %d", event->domid); goto error;
participants (3)
-
Jim Fehlig -
Michal Privoznik -
Peter Krempa