On Wed, Feb 02, 2011 at 02:18:47PM +0100, Jiri Denemark wrote: > > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > > > index 6140f0f..c527bb7 100644 > > > --- a/src/qemu/qemu_driver.c > > > +++ b/src/qemu/qemu_driver.c > > > @@ -2972,7 +2972,11 @@ cleanup: > > > * pretend we never started it */ > > > virCommandFree(cmd); > > > VIR_FORCE_CLOSE(logfile); > > > - qemudShutdownVMDaemon(driver, vm, 0); > > > + /* The vm may be cloesd in other thread, so we should check whether the > > > > s/cloesd/closed/ > > > > > + * vm is active before shutdown. > > > + */ > > > + if (virDomainObjIsActive(vm)) > > > + qemudShutdownVMDaemon(driver, vm, 0); > > > > I'm still playing with this patch, but at first glance, it is making > > sense to me. > > The patch makes sense to me, since we may unlock the domain object several > times before we get to the cleanup code. Thus the state may have changed by > the time we get there. > > Eric, what is the result of you playing with the patch? Is it ok to ack and > push it? This patch is only protecting one caller of qemuShutdownVMDaemon. I think it'd be worth moving it to be in the qemuShutdownVMDaemon function itself, so all callers are protected from mistakes / races. 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