[libvirt] [PATCH 0/5] Make virDomainObjListFindByName run in O(1)

[Daniel P. Berrange]

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 :|