Re: [libvirt] [PATCH] Move most of qemuProcessKill into virProcessKillPainfully
On Wed, Sep 26, 2012 at 10:07:42AM -0600, Eric Blake wrote:
> +++ b/src/qemu/qemu_driver.c
> @@ -2006,13 +2006,11 @@ qemuDomainDestroyFlags(virDomainPtr dom,
> * it now means the job will be released
> */
> if (flags & VIR_DOMAIN_DESTROY_GRACEFUL) {
> - if (qemuProcessKill(driver, vm, 0) < 0) {
> - virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> - _("failed to kill qemu process with
SIGTERM"));
> + if (qemuProcessKill(driver, vm, 0) < 0)
> goto cleanup;
> - }
> } else {
> - ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE));
> + if (qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE) < 0)
> + goto cleanup;
Why are you changing this from ignore_value() into a cleanup path on
failure?
Hmm, this is a semantic change, so I guess I ought to have it as a
separate patch. The virDomainDestroy() function is supposed to
guarantee that the domain has been killed. In the current code
we were returning 0, even if qemuProcessKill failed to kill the
process even with SIGKILL. If even SIGKILL fails, we really need
to return an error code so apps can see that virDomainDestroy
failed, and not think everything was fine.
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 :|