[libvirt] [PATCH] Move most of qemuProcessKill into virProcessKillPainfully

[Eric Blake]

On 09/26/2012 10:25 AM, Daniel P. Berrange wrote:
> 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.

That explanation makes sense, but then so does the conclusion - this
should be two patches ;)

-- 
Eric Blake   eblake at redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 617 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120926/62da0b95/attachment-0001.sig>