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

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(a)redhat.com&gt; > --- > 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