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