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;
+ } else {
vm->def->id = qemuDriverAllocateID(driver);
qemuDomainSetFakeReboot(driver, vm, false);
virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP);
@@ -4770,10 +4773,10 @@ qemuProcessInit(virQEMUDriverPtr driver,
VIR_HOOK_QEMU_OP_PREPARE,
VIR_HOOK_SUBOP_BEGIN) < 0)
goto stop;
- }
- if (qemuDomainSetPrivatePaths(driver, vm) < 0)
- goto cleanup;
+ if (qemuDomainSetPrivatePaths(driver, vm) < 0)
+ goto stop;
+ }
ret = 0;
--
2.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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.
> + } else {
> vm->def->id = qemuDriverAllocateID(driver);
> qemuDomainSetFakeReboot(driver, vm, false);
> virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP);
> @@ -4770,10 +4773,10 @@ qemuProcessInit(virQEMUDriverPtr driver,
> VIR_HOOK_QEMU_OP_PREPARE,
> VIR_HOOK_SUBOP_BEGIN) < 0)
> goto stop;
> - }
>
> - if (qemuDomainSetPrivatePaths(driver, vm) < 0)
> - goto cleanup;
> + if (qemuDomainSetPrivatePaths(driver, vm) < 0)
> + goto stop;
> + }
>
> ret = 0;
>
>
ACKed and pushed.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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)
>
>> + } else {
>> vm->def->id = qemuDriverAllocateID(driver);
>> qemuDomainSetFakeReboot(driver, vm, false);
>> virDomainObjSetState(vm, VIR_DOMAIN_PAUSED, VIR_DOMAIN_PAUSED_STARTING_UP);
>> @@ -4770,10 +4773,10 @@ qemuProcessInit(virQEMUDriverPtr driver,
>> VIR_HOOK_QEMU_OP_PREPARE,
>> VIR_HOOK_SUBOP_BEGIN) < 0)
>> goto stop;
>> - }
>>
>> - if (qemuDomainSetPrivatePaths(driver, vm) < 0)
>> - goto cleanup;
>> + if (qemuDomainSetPrivatePaths(driver, vm) < 0)
>> + goto stop;
>> + }
>>
>> ret = 0;
>>
>>
>
> ACKed and pushed.
>
> Michal
>
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2026 Red Hat, Inc.