On Thu, Mar 03, 2011 at 02:47:38PM -0500, Laine Stump wrote: > This was also found while investigating > > https://bugzilla.redhat.com/show_bug.cgi?id=670848 > > An EOF on a domain's monitor socket results in an event being queued > to handle the EOF. The handler calls qemuProcessHandleMonitorEOF. If > it is a transient domain, this leads to a call to > virDomainRemoveInactive, which removes the domain from the driver's > hashtable and unref's it. Nowhere in this code is the qemu driver lock > acquired. > > However, all modifications to the driver's domain hashtable *must* be > done while holding the driver lock, otherwise the hashtable can become > corrupt, and (even more likely) another thread could call a different > hashtable function and acquire a pointer to the domain that is in the > process of being destroyed. > > To prevent such a disaster, qemuProcessHandleMonitorEOF must get the > qemu driver lock *before* it gets the DomainObj's lock, and hold it > until it is finished with the DomainObj. This guarantees that nobody > else modifies the hashtable at the same time, and that anyone who had > already gotten the DomainObj from the hashtable prior to this call has > finished with it before we remove/destroy it. > --- > src/qemu/qemu_process.c | 5 +++-- > 1 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c > index 1d67b3c..6a65a33 100644 > --- a/src/qemu/qemu_process.c > +++ b/src/qemu/qemu_process.c > @@ -107,11 +107,13 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, > > VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name); > > + qemuDriverLock(driver); > virDomainObjLock(vm); > > if (!virDomainObjIsActive(vm)) { > VIR_DEBUG("Domain %p is not active, ignoring EOF", vm); > virDomainObjUnlock(vm); > + qemuDriverUnlock(driver); > return; > } > > @@ -137,10 +139,9 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED, > virDomainObjUnlock(vm); > > if (event) { > - qemuDriverLock(driver); > qemuDomainEventQueue(driver, event); > - qemuDriverUnlock(driver); > } > + qemuDriverUnlock(driver); > } ACK 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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list