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

Re: [libvirt] [PATCH] Fix cleanup on VM state after failed QEMU startup



On Mon, Feb 14, 2011 at 06:01:49PM +0800, Wen Congyang wrote:
> At 02/14/2011 05:38 PM, Daniel P. Berrange Write:
> > Commit 9962e406c664ed5521f5aca500c860a331cb3979 introduced a
> > problem where if the VM failed to startup, it would not be
> > correctly cleaned up. Amongst other things the SELinux
> > security label would not be removed, which prevents the VM
> > from ever starting again.
> > 
> > The virDomainIsActive() check at the start of qemudShutdownVMDaemon
> > checks for vm->def->id not being -1. By moving the assignment of the
> > VM id to the start of qemudStartVMDaemon, we can ensure cleanup will
> > occur on failure
> > 
> > * src/qemu/qemu_driver.c: Move initialization of 'vm->def->id'
> >   so that qemudShutdownVMDaemon() will process the shutdown
> > ---
> >  src/qemu/qemu_driver.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 21d7779..d7c806b 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -2652,6 +2652,8 @@ static int qemudStartVMDaemon(virConnectPtr conn,
> >      if (virDomainObjSetDefTransient(driver->caps, vm, true) < 0)
> >          goto cleanup;
> >  
> > +    vm->def->id = driver->nextvmid++;
> > +
> 
> The function virDomainObjSetDefTransient() does not need cleanup?

Only if it is successful, which is what this does.

> I think we can init 'vm->def->id' after we check if the domain
> is started.

Nope, we need to be able to reverse the results of every function
run in qemudStartVMDaemon, regardless of whether we actually get
as far as the virCommandRun step.

> 
> >      /* Must be run before security labelling */
> >      DEBUG0("Preparing host devices");
> >      if (qemuPrepareHostDevices(driver, vm->def) < 0)
> > @@ -2818,7 +2820,6 @@ static int qemudStartVMDaemon(virConnectPtr conn,
> >      }
> >  
> >      DEBUG0("Building emulator command line");
> > -    vm->def->id = driver->nextvmid++;
> >      if (!(cmd = qemuBuildCommandLine(conn, driver, vm->def, priv->monConfig,
> >                                       priv->monJSON != 0, qemuCmdFlags,
> >                                       migrateFrom, stdin_fd,
> 

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]