3:24 a.m.
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
3:24 a.m.
New subject: [libvirt] [PATCH 1/2] uml: s/virDomainObjListFindByID/virDomainObjListFindByUUID/
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(a)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
4:15 a.m.
New subject: [libvirt] [PATCH 1/2] uml: s/virDomainObjListFindByID/virDomainObjListFindByUUID/
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(a)redhat.com>
---
src/uml/uml_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
ACK,
Peter
6:31 a.m.
New subject: [libvirt] [PATCH 1/2] uml: s/virDomainObjListFindByID/virDomainObjListFindByUUID/
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(a)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
3:24 a.m.
New subject: [libvirt] [PATCH 2/2] libxl: s/virDomainObjListFindByID/virDomainObjListFindByUUID/
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(a)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
2:09 p.m.
New subject: [libvirt] [PATCH 2/2] libxl: s/virDomainObjListFindByID/virDomainObjListFindByUUID/
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(a)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;
3531
days inactive
3531
days old
5 comments
3 participants
participants (3)
-
Jim Fehlig -
Michal Privoznik -
Peter Krempa