[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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

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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]