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

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



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


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