Re: [libvirt] [PATCH 1/2] qemu: Fix incorrect jump labels in error paths

On Thu, Feb 23, 2017 at 03:33 PM +0100, Michal Privoznik <mprivozn(a)redhat.com&gt; wrote: > On 02/23/2017 10:44 AM, Marc Hartmayer wrote: >> Fix incorrect jump labels in error paths as the stop jump is only >> needed if the driver has already changed the state. For example >> 'virAtomicIntInc(&driver->nactive)' will be 'reverted' in the >> qemuProcessStop call. >> >> Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com&gt; >> Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; >> --- >> src/qemu/qemu_process.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c >> index ea10fff..a57d136 100644 >> --- a/src/qemu/qemu_process.c >> +++ b/src/qemu/qemu_process.c >> @@ -4755,9 +4755,12 @@ qemuProcessInit(virQEMUDriverPtr driver, >> */ >> VIR_DEBUG("Setting current domain def as transient"); >> if (virDomainObjSetDefTransient(caps, driver->xmlopt, vm) < 0) >> - goto stop; >> + goto cleanup; >> >> - if (!(flags & VIR_QEMU_PROCESS_START_PRETEND)) { >> + if (flags & VIR_QEMU_PROCESS_START_PRETEND) { >> + if (qemuDomainSetPrivatePaths(driver, vm) < 0) >> + goto cleanup; > > This should have been goto stop; After SetDefTransien() succeeds, > everything must goto stop in order to call qemuProcessStop which undoes > the SetDefTransient(). I will fix it before pushing. Hmm why? In qemuProcessStop() the driver->nactive will be decreased and I think that is definitely not what we want if we haven't increased that value... (for the VIR_QEMU_PROCESS_START_PRETEND path)