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.