[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 02/23/2017 04:57 PM, Marc Hartmayer wrote:
> On Thu, Feb 23, 2017 at 03:33 PM +0100, Michal Privoznik <mprivozn redhat com> 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 linux vnet ibm com>
>>> Reviewed-by: Boris Fiuczynski <fiuczy linux vnet ibm com>
>>> ---
>>>  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)

Ah, that's a good point. On the other hand, we need to call
virDomainObjRemoveTransientDef(). So I guess the proper solution is to:

if (qemuDomainSetPrivatePaths(driver, vm) < 0) {
  virDomainObjRemoveTransientDef(vm);
  goto cleanup;
}

I'll post the patch shortly.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]