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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list