Re: [libvirt] [PATCH 0/5] Make virDomainObjListFindByName run in O(1)
On Fri, Apr 24, 2015 at 09:16:05AM +0200, Michal Privoznik wrote: On 24.04.2015 07:40, zhang bo wrote: On 2015/4/24 1:14, Michal Privoznik wrote: As discussed here: https://www.redhat.com/archives/libvir-list/2015-April/msg01135.html Michal Privoznik (5): virDomainObjListAddLocked: s/false/NULL/ for @oldDef virDomainObjListNew: Use virObjectFreeHashData Introduce virDomainObjEndAPI virDomainObjListFindByName: Return referenced object virDomainObjList: Introduce yet another hash table src/bhyve/bhyve_driver.c | 3 +- src/conf/domain_conf.c | 83 +++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 10 ++--- src/lxc/lxc_driver.c | 3 +- src/openvz/openvz_driver.c | 11 +++-- src/parallels/parallels_driver.c | 3 +- src/qemu/qemu_domain.c | 7 +-- src/qemu/qemu_driver.c | 14 ++ src/test/test_driver.c | 93 +--- src/uml/uml_driver.c | 15 +++ 12 files changed, 110 insertions(+), 135 deletions(-) BTW, we just dealt with virDomainObjListFindByName() here, without caring about virDomainObjListFindByID(). virDomainObjListFindByID() is now called by umlDomainDestroyFlags() and umlDomainShutdownFlags(), if we simultaneously shutdown multiple vms on hypervisor UML, then similar problem comes out: some guest maybe unable to shutdown immediately, until other guests get shutoff, and even 'virsh list' blocks. so, shall we: 1 don't take such problem as a problem 2 use virDomainObjListFindByName() instead of virDomainObjListFindByID() there. In fact I'd prefer FindByUUID() everywhere except for those few places where other lookup functions are necessary (e.g. virDomainLookupByID()). 3 add a third hash table for domid No. even two hash tables duplicate information enough. If we use virDomainObjListFindByName() there, and forbids developers from using **ByID(), it seems unacceptable for any developers. Why? If we add a third hash table, it may become not easy to maintain the high-complexity codes. Is there a better solution other than adding more hash tables? The ultimate solution would be to have multi key hash table, so that any item from tuple (uuid, name, id) would suffice to find domain object. I think we should just ignore the FindByID method - nothing inside libvirt should use it - as you say we should always use UUID except in the couple of places we use name. The public LookupByID method is discouraged now we have the bulk-list APIs. So I don't think there's any compelling reason to have a 3rd hash for ID lookup. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] Make virDomainObjListFindByName run in O(1)
On 23.04.2015 19:14, Michal Privoznik wrote: As discussed here: https://www.redhat.com/archives/libvir-list/2015-April/msg01135.html Michal Privoznik (5): virDomainObjListAddLocked: s/false/NULL/ for @oldDef virDomainObjListNew: Use virObjectFreeHashData Introduce virDomainObjEndAPI virDomainObjListFindByName: Return referenced object virDomainObjList: Introduce yet another hash table src/bhyve/bhyve_driver.c | 3 +- src/conf/domain_conf.c | 83 +++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 10 ++--- src/lxc/lxc_driver.c | 3 +- src/openvz/openvz_driver.c | 11 +++-- src/parallels/parallels_driver.c | 3 +- src/qemu/qemu_domain.c | 7 +-- src/qemu/qemu_driver.c | 14 ++ src/test/test_driver.c | 93 +--- src/uml/uml_driver.c | 15 +++ 12 files changed, 110 insertions(+), 135 deletions(-) Thanks for all the reviews. I've pushed this. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] Make virDomainObjListFindByName run in O(1)
On 24.04.2015 07:40, zhang bo wrote: On 2015/4/24 1:14, Michal Privoznik wrote: As discussed here: https://www.redhat.com/archives/libvir-list/2015-April/msg01135.html Michal Privoznik (5): virDomainObjListAddLocked: s/false/NULL/ for @oldDef virDomainObjListNew: Use virObjectFreeHashData Introduce virDomainObjEndAPI virDomainObjListFindByName: Return referenced object virDomainObjList: Introduce yet another hash table src/bhyve/bhyve_driver.c | 3 +- src/conf/domain_conf.c | 83 +++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 10 ++--- src/lxc/lxc_driver.c | 3 +- src/openvz/openvz_driver.c | 11 +++-- src/parallels/parallels_driver.c | 3 +- src/qemu/qemu_domain.c | 7 +-- src/qemu/qemu_driver.c | 14 ++ src/test/test_driver.c | 93 +--- src/uml/uml_driver.c | 15 +++ 12 files changed, 110 insertions(+), 135 deletions(-) BTW, we just dealt with virDomainObjListFindByName() here, without caring about virDomainObjListFindByID(). virDomainObjListFindByID() is now called by umlDomainDestroyFlags() and umlDomainShutdownFlags(), if we simultaneously shutdown multiple vms on hypervisor UML, then similar problem comes out: some guest maybe unable to shutdown immediately, until other guests get shutoff, and even 'virsh list' blocks. so, shall we: 1 don't take such problem as a problem 2 use virDomainObjListFindByName() instead of virDomainObjListFindByID() there. In fact I'd prefer FindByUUID() everywhere except for those few places where other lookup functions are necessary (e.g. virDomainLookupByID()). 3 add a third hash table for domid No. even two hash tables duplicate information enough. If we use virDomainObjListFindByName() there, and forbids developers from using **ByID(), it seems unacceptable for any developers. Why? If we add a third hash table, it may become not easy to maintain the high-complexity codes. Is there a better solution other than adding more hash tables? The ultimate solution would be to have multi key hash table, so that any item from tuple (uuid, name, id) would suffice to find domain object. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/5] Make virDomainObjListFindByName run in O(1)
As discussed here: https://www.redhat.com/archives/libvir-list/2015-April/msg01135.html Michal Privoznik (5): virDomainObjListAddLocked: s/false/NULL/ for @oldDef virDomainObjListNew: Use virObjectFreeHashData Introduce virDomainObjEndAPI virDomainObjListFindByName: Return referenced object virDomainObjList: Introduce yet another hash table src/bhyve/bhyve_driver.c | 3 +- src/conf/domain_conf.c | 83 +++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 10 ++--- src/lxc/lxc_driver.c | 3 +- src/openvz/openvz_driver.c | 11 +++-- src/parallels/parallels_driver.c | 3 +- src/qemu/qemu_domain.c | 7 +-- src/qemu/qemu_driver.c | 14 ++ src/test/test_driver.c | 93 +--- src/uml/uml_driver.c | 15 +++ 12 files changed, 110 insertions(+), 135 deletions(-) -- 2.0.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] Make virDomainObjListFindByName run in O(1)
On 2015/4/24 1:14, Michal Privoznik wrote: As discussed here: https://www.redhat.com/archives/libvir-list/2015-April/msg01135.html Michal Privoznik (5): virDomainObjListAddLocked: s/false/NULL/ for @oldDef virDomainObjListNew: Use virObjectFreeHashData Introduce virDomainObjEndAPI virDomainObjListFindByName: Return referenced object virDomainObjList: Introduce yet another hash table src/bhyve/bhyve_driver.c | 3 +- src/conf/domain_conf.c | 83 +++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 10 ++--- src/lxc/lxc_driver.c | 3 +- src/openvz/openvz_driver.c | 11 +++-- src/parallels/parallels_driver.c | 3 +- src/qemu/qemu_domain.c | 7 +-- src/qemu/qemu_driver.c | 14 ++ src/test/test_driver.c | 93 +--- src/uml/uml_driver.c | 15 +++ 12 files changed, 110 insertions(+), 135 deletions(-) BTW, we just dealt with virDomainObjListFindByName() here, without caring about virDomainObjListFindByID(). virDomainObjListFindByID() is now called by umlDomainDestroyFlags() and umlDomainShutdownFlags(), if we simultaneously shutdown multiple vms on hypervisor UML, then similar problem comes out: some guest maybe unable to shutdown immediately, until other guests get shutoff, and even 'virsh list' blocks. so, shall we: 1 don't take such problem as a problem 2 use virDomainObjListFindByName() instead of virDomainObjListFindByID() there. 3 add a third hash table for domid If we use virDomainObjListFindByName() there, and forbids developers from using **ByID(), it seems unacceptable for any developers. If we add a third hash table, it may become not easy to maintain the high-complexity codes. Is there a better solution other than adding more hash tables? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] Make virDomainObjListFindByName run in O(1)
On 2015/4/24 1:14, Michal Privoznik wrote: As discussed here: https://www.redhat.com/archives/libvir-list/2015-April/msg01135.html Michal Privoznik (5): virDomainObjListAddLocked: s/false/NULL/ for @oldDef virDomainObjListNew: Use virObjectFreeHashData Introduce virDomainObjEndAPI virDomainObjListFindByName: Return referenced object virDomainObjList: Introduce yet another hash table src/bhyve/bhyve_driver.c | 3 +- src/conf/domain_conf.c | 83 +++ src/conf/domain_conf.h | 2 + src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 10 ++--- src/lxc/lxc_driver.c | 3 +- src/openvz/openvz_driver.c | 11 +++-- src/parallels/parallels_driver.c | 3 +- src/qemu/qemu_domain.c | 7 +-- src/qemu/qemu_driver.c | 14 ++ src/test/test_driver.c | 93 +--- src/uml/uml_driver.c | 15 +++ 12 files changed, 110 insertions(+), 135 deletions(-) Great. I tested the patch series, the migration downtime problem disappears. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
