conf: Use correct type for balloon stats period

We're parsing memballoon status period as unsigned int, but when we're
trying to set it, both we and qemu use signed int.  That means large
values will get wrapped around to negative one resulting in error.
Basically the same problem as commit e3a7b874 was dealing with when
updating live domain.

QEMU changed the accepted value to int64 in commit 1f9296b5, but even
values as INT_MAX don't make sense since the value passed means seconds.
Hence adding capability flag for this change isn't worth it.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1140958

Signed-off-by: Luyao Huang <lhuang@redhat.com>
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 40e2b2979e1bc39e54b9054444cf8ded9fa5036b..8d989155419cda5182366c1b73b1899961d73400 100644 (file)
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -5618,18 +5618,17 @@ qemu-kvm -net nic,model=? /dev/null
       <dt><code>period</code></dt>
       <dd>
         <p>
-          The optional <code>period</code> allows the QEMU virtio memory
-          balloon driver to provide statistics through the <code>virsh
-          dommemstat [domain]</code> command. By default, collection is
-          not enabled. In order to enable, use the <code>virsh dommemstat
-          [domain] --period [number]</code> command or <code>virsh edit</code>
-          command to add the option to the XML definition.
-          The <code>virsh dommemstat</code> will accept the options
-          <code>--live</code>, <code>--current</code>, or <code>--config</code>.
-          If an option is not provided, the change for a running domain will
-          only be made to the active guest.
-          If the QEMU driver is not at the right
-          revision, the attempt to set the period will fail.
+          The optional <code>period</code> allows the QEMU virtio memory balloon
+          driver to provide statistics through the <code>virsh dommemstat
+          [domain]</code> command. By default, collection is not enabled. In
+          order to enable, use the <code>virsh dommemstat [domain] --period
+          [number]</code> command or <code>virsh edit</code> command to add the
+          option to the XML definition.  The <code>virsh dommemstat</code> will
+          accept the options <code>--live</code>, <code>--current</code>,
+          or <code>--config</code>.  If an option is not provided, the change
+          for a running domain will only be made to the active guest.  If the
+          QEMU driver is not at the right revision, the attempt to set the
+          period will fail.  Large values (e.g. many years) might be ignored.
           <span class='since'>Since 1.1.1, requires QEMU 1.5</span>
         </p>
       </dd>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3865854a5ca843ea891882d92cec5c4bf4f3bf31..c75b543593ac10f61d11496092fdac4a6fbc6a57 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10492,6 +10492,7 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
     char *model;
     virDomainMemballoonDefPtr def;
     xmlNodePtr save = ctxt->node;
+    unsigned int period = 0;
 
     if (VIR_ALLOC(def) < 0)
         return NULL;
@@ -10510,12 +10511,16 @@ virDomainMemballoonDefParseXML(xmlNodePtr node,
     }
 
     ctxt->node = node;
-    if (virXPathUInt("string(./stats/@period)", ctxt, &def->period) < -1) {
+    if (virXPathUInt("string(./stats/@period)", ctxt, &period) < -1) {
         virReportError(VIR_ERR_XML_ERROR, "%s",
                        _("invalid statistics collection period"));
         goto error;
     }
 
+    def->period = period;
+    if (def->period < 0)
+        def->period = 0;
+
     if (def->model == VIR_DOMAIN_MEMBALLOON_MODEL_NONE)
         VIR_DEBUG("Ignoring device address for none model Memballoon");
     else if (virDomainDeviceInfoParseXML(node, NULL, &def->info, flags) < 0)
@@ -18839,7 +18844,7 @@ virDomainMemballoonDefFormat(virBufferPtr buf,
     virBufferAdjustIndent(&childrenBuf, indent + 2);
 
     if (def->period)
-        virBufferAsprintf(&childrenBuf, "<stats period='%u'/>\n", def->period);
+        virBufferAsprintf(&childrenBuf, "<stats period='%i'/>\n", def->period);
 
     if (virDomainDeviceInfoNeedsFormat(&def->info, flags) &&
         virDomainDeviceInfoFormat(&childrenBuf, &def->info, flags) < 0) {

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4b437b440c6a4842e8c9fcf95a49372d0c0d9836..4eb7742887e7a25cc1ca5ebbc637f29e555bd6bf 100644 (file)
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1560,7 +1560,7 @@ enum {
 struct _virDomainMemballoonDef {
     int model;
     virDomainDeviceInfo info;
-    unsigned int period; /* seconds between collections */
+    int period; /* seconds between collections */
 };
 
 struct _virDomainNVRAMDef {

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 5bb7e5b390d8d3f8e104cc34adf2a6ec82f92acd..ae315df09e0570a2800ac97e433ee1893a9404ce 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4309,7 +4309,7 @@ int qemuProcessStart(virConnectPtr conn,
     virCommandPtr cmd = NULL;
     struct qemuProcessHookData hookData;
     unsigned long cur_balloon;
-    unsigned int period = 0;
+    int period = 0;
     size_t i;
     bool rawio_set = false;
     char *nodeset = NULL;