On Mon, Aug 01, 2011 at 05:05:28PM +0800, ajia(a)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(a)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