[libvirt] [PATCH 2/7] qemu: Adjust EndAsyncJob for qemuDomainSaveInternal error path

[John Ferlan]

On 01/29/2015 08:43 AM, Martin Kletzander wrote:
> On Wed, Jan 28, 2015 at 05:25:01PM -0500, John Ferlan wrote:
>> Commit id '540c339a' to fix issues with reference counting and transient
>> domains moved the qemuDomainObjEndAsyncJob call prior to the attempt to
>> restart the guest CPU's resulting in an error:
>>
>>    error: Failed to save domain rhel70 to /tmp/pl/rhel70.save
>>    error: internal error: unexpected async job 3
>>
>> when (ret != 0) - eg, the error path from qemuDomainSaveMemory.
>>
>> This patch will adjust the logic to only call the EndAsyncJob after
>> we've tried to restart the guest CPUs. It also needs to adjust the
>> test for qemuDomainRemoveInactive to add the ret == 0 condition.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>> src/qemu/qemu_driver.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 190d472..0174c87 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -3208,7 +3208,6 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver,
>> virDomainPtr dom,
>>                                      VIR_DOMAIN_EVENT_STOPPED,
>>                                      VIR_DOMAIN_EVENT_STOPPED_SAVED);
>>  endjob:
>> -    qemuDomainObjEndAsyncJob(driver, vm);
>>     if (ret != 0) {
>>         if (was_running && virDomainObjIsActive(vm)) {
>>             virErrorPtr save_err = virSaveLastError();
>> @@ -3224,9 +3223,10 @@ qemuDomainSaveInternal(virQEMUDriverPtr driver,
>> virDomainPtr dom,
>>             virSetError(save_err);
>>             virFreeError(save_err);
>>         }
>> -    } else if (!vm->persistent) {
>> -        qemuDomainRemoveInactive(driver, vm);
>>     }
>> +    qemuDomainObjEndAsyncJob(driver, vm);
>> +    if (ret == 0 && !vm->persistent)
>> +        qemuDomainRemoveInactive(driver, vm);
>>
> 
> We must definitely keep the job until we're done with changing things
> with the domain.  Have I really messed this up so much?  ACK whether
> you squash it in or not.
> 

>From my re-review of the referenced commit, I believe this was the only
function where endjob:/cleanup: processing needed the job still around
(and only because this command stops the CPU's and error recovery is
best effort).

I've combined the two patches into one, adjusted the commit message and
pushed just these two as one.

Tks -

John