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