Re: [PATCH 2/2] qemu: Fix updating balloon period in live XML

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

 



On 10/01/2014 01:55 AM, John Ferlan wrote:


On 09/22/2014 06:41 AM, Erik Skultety wrote:
Up until now, we set memballoon period in monitor successfully, however
we did not update domain definition structure, thus dumpxml was omitting
period attribute in memballoon element

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140960
---
  src/qemu/qemu_driver.c | 8 +++++++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ede8880..d73288a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2460,9 +2460,15 @@ static int qemuDomainSetMemoryStatsPeriod(virDomainPtr dom, int period,
          qemuDomainObjEnterMonitor(driver, vm);
          r = qemuMonitorSetMemoryStatsPeriod(priv->mon, period);
          qemuDomainObjExitMonitor(driver, vm);
-        if (r < 0)
+        if (r < 0) {
              virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                             _("unable to set balloon driver collection period"));
+            goto endjob;
+        }

I'm trying to remember if there was a reason for not jumping to error.
It probably has to do with "at some point in time" during development
this setting would/could be done through calls via qemu_process.c and
causing a failure through that path wasn't good.  Now since this only
accessible via a virsh command - I agree going to endjob is right...

If you care to walk the history - start here:

http://www.redhat.com/archives/libvir-list/2013-July/msg00770.html

I had a little look at the history you pointed out. From what I've found, the sub-element 'period' was introduced along with qemu driver-specific API and libvirt domain API in 10-part v3 patch series (http://www.redhat.com/archives/libvir-list/2013-July/msg00774.html). I detached HEAD before these commits were pushed and I didn't find any trace about memballoon period setting. All the APIs and necessary structures/members were introduced by the patch series mentioned above, is that correct? :) (in that case I guess it's fine to add the jump to 'endjob' label)
ACK (to what's here)

John
+
+        vm->def->memballoon->period = period;
+        if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0)
+            goto endjob;
      }

      if (flags & VIR_DOMAIN_AFFECT_CONFIG) {


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Erik

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]