[libvirt] [PATCH] util: Allow removing hash entries in virHashForEach

[Daniel Veillard]

On Thu, Mar 03, 2011 at 03:46:05PM +0100, Jiri Denemark wrote:
> On Thu, Mar 03, 2011 at 22:09:32 +0800, Daniel Veillard wrote:
> > > This fixes a possible crash of libvirtd during its startup. When qemu
> > > driver reconnects to running domains, it iterates over all domain
> > > objects in a hash. When reconnecting to an associated qemu monitor
> > > fails and the domain is transient, it's immediately removed from the
> > > hash. Despite the fact that it's explicitly forbidden to do so. If
> > > libvirtd is lucky enough, virHashForEach will access random memory when
> > > the callback finishes and the deamon will crash.
> > > 
> > > Since it's trivial to fix virHashForEach to allow removal of hash
> > > entries while iterating through them, I went this way instead of fixing
> > > qemuReconnectDomain callback (and possibly others) to avoid deleting the
> > > entries.
> > 
> >   ACK, removing should be safe then, but adding will still be
> > a big problem due to virHashGrow().
> 
> Yeah, that would be a much bigger issue.
> 
> I pushed the patch.
> 
> > It's a safety belt, but doesn't replace driving properly. I tend to
> > think we should try to detect reentrant behaviour, for example have
> > a counter increment each time an operation is done on the hash and
> > raise a warning at runtime if the counter changed around iter().
> 
> It would probably be easier to set a flag that we're iterating over the hash
> and let all other APIs fail in that case.

  Works too :-)

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/