[libvirt] [PATCH] qemu: fix return value issue
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@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; } cleanup: -- 1.7.1
On Mon, Aug 01, 2011 at 05:05:28PM +0800, ajia@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@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: -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 08/01/2011 05:57 PM, Daniel Veillard wrote:
On Mon, Aug 01, 2011 at 05:05:28PM +0800, ajia@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@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
participants (3)
-
ajia -
ajia＠redhat.com
-
Daniel Veillard