[libvirt] [PATCH] qemu: fix return value issue

ajia ajia at redhat.com
Mon Aug 1 14:50:52 UTC 2011


On 08/01/2011 05:57 PM, Daniel Veillard wrote:
> On Mon, Aug 01, 2011 at 05:05:28PM +0800, ajia at redhat.com wrote:
>> whether or not previous return value is -1, the following codes will be
>> executed for a inactive guest in src/qemu/qemu_driver.c:
>> ret = virDomainSaveConfig(driver->configDir, persistentDef);
>> and if everything is okay, 'ret' is assigned to 0, the previous 'ret'
>> will be overwritten, this patch will fix this issue.
>>
>> * src/qemu/qemu_driver.c: avoid return value is overwritten when give a argument
>>    in out of blkio weight range for a inactive guest.
>>
>> * how to reproduce?
>>    % virsh blkiotune ${guestname} --weight 10
>>    % echo $?
>>
>>    Note: guest must be inactive, argument 10 in out of blkio weight range,
>>    however, virsh hasn't raised any error information, and return value is 0.
>>
>>    https://bugzilla.redhat.com/show_bug.cgi?id=726304
>>
>> Signed-off-by: Alex Jia<ajia at redhat.com>
>> ---
>>   src/qemu/qemu_driver.c |    4 +++-
>>   1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index b673fd5..aaccddf 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -5688,7 +5688,9 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom,
>>                   ret = -1;
>>               }
>>           }
>> -        ret = virDomainSaveConfig(driver->configDir, persistentDef);
>> +
>> +        if(virDomainSaveConfig(driver->configDir, persistentDef)<  0)
>> +            goto cleanup;
>>       }
>    Doesn't sound right either because then we fail to report if the
> SaveConfig operation fails
>
>    The following fixes the issues you raise and still report failues
> of saving config files:
>
> Daniel
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index b673fd5..ef6f34f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5688,7 +5688,8 @@ static int qemuDomainSetBlkioParameters(virDomainPtr dom,
>                   ret = -1;
>               }
>           }
> -        ret = virDomainSaveConfig(driver->configDir, persistentDef);
> +        if (virDomainSaveConfig(driver->configDir, persistentDef)<  0)
> +            ret = -1;
>       }
>
>   cleanup:
>
Yeah, agree, if everything is okay, 'ret' will be assigned to 0, so here 
should set 'ret' to -1
if virDomainSaveConfig is fail.

Daniel, need I commit v2 patch? or directly apply your modification.

BTW, another patch is the same issue, I will commit v2 patch.

Thanks,
Alex




More information about the libvir-list mailing list