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

Re: [libvirt] [PATCH] avoid shutting down the vm twice



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 :|


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