[PATCH v2] domain: fix parsing of memory tunables on 32-bit machines

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

 



Commit 6c9a8a4 (Oct 2014) exposed a long-standing issue on 32-bit
machines: code related to virDomainSetMemoryParameters has always
been documented as using a 64-bit limit, but it was implemented by
calling virDomainParseMemory which enforced an 'unsigned long'
limit.  Since VIR_DOMAIN_MEMORY_PARAM_UNLIMITED capped to a
long is -1, but virDomainParseScaledValue no longer accepts
negative values, an attempt to use 2^53-1 as a hard memory limit
started failing the testsuite.  However, the problem with capping
things artificially low has existed for much longer - ever since
commits 4888f0fb and 2e22f23 (Mar 2012) switched internal tracking
from 'unsigned long' to 'unsigned long long' (prior to that time,
the cap was a side-effect of the choice of types).  We _have_ to
cap the balloon memory values, (no thanks to baked in 'unsigned long'
of API such as virDomainSetMaxMemory or virDomainGetInfo with no
counterpart API that guarantees 64-bit access to those numbers)
but memory parameters have never needed the artificial limit.

At any rate, the solution is to make the parser function gain a
parameter, and only do the reduced 32-bit cap for the values that
are constrained due to API.

* src/conf/domain_conf.h (_virDomainMemtune): Add comments.
* src/conf/domain_conf.c (virDomainParseMemory): Add parameter.
(virDomainDefParseXML): Adjust callers.

Signed-off-by: Eric Blake <eblake@xxxxxxxxxx>
---
 src/conf/domain_conf.c | 25 ++++++++++++++-----------
 src/conf/domain_conf.h | 14 ++++++++------
 2 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 39befb0..c594325 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6373,19 +6373,22 @@ virDomainParseScaledValue(const char *xpath,


 /* Parse a memory element located at XPATH within CTXT, and store the
- * result into MEM.  If REQUIRED, then the value must exist;
- * otherwise, the value is optional.  The value is in blocks of 1024.
- * Return 0 on success, -1 on failure after issuing error.  */
+ * result into MEM, in blocks of 1024.  If REQUIRED, then the value
+ * must exist; otherwise, the value is optional.  The value must not
+ * exceed VIR_DOMAIN_MEMORY_PARAM_UNLIMITED once scaled; additionally,
+ * if CAPPED is true, the value must fit within an unsigned long (only
+ * matters on 32-bit platforms).  Return 0 on success, -1 on failure
+ * after issuing error.  */
 static int
 virDomainParseMemory(const char *xpath, xmlXPathContextPtr ctxt,
-                     unsigned long long *mem, bool required)
+                     unsigned long long *mem, bool required, bool capped)
 {
     int ret = -1;
     unsigned long long bytes, max;

     /* On 32-bit machines, our bound is 0xffffffff * KiB. On 64-bit
      * machines, our bound is off_t (2^63).  */
-    if (sizeof(unsigned long) < sizeof(long long))
+    if (capped && sizeof(unsigned long) < sizeof(long long))
         max = 1024ull * ULONG_MAX;
     else
         max = LLONG_MAX;
@@ -12215,11 +12218,11 @@ virDomainDefParseXML(xmlDocPtr xml,

     /* Extract domain memory */
     if (virDomainParseMemory("./memory[1]", ctxt,
-                             &def->mem.max_balloon, true) < 0)
+                             &def->mem.max_balloon, true, true) < 0)
         goto error;

     if (virDomainParseMemory("./currentMemory[1]", ctxt,
-                             &def->mem.cur_balloon, false) < 0)
+                             &def->mem.cur_balloon, false, true) < 0)
         goto error;

     /* and info about it */
@@ -12339,19 +12342,19 @@ virDomainDefParseXML(xmlDocPtr xml,

     /* Extract other memory tunables */
     if (virDomainParseMemory("./memtune/hard_limit[1]", ctxt,
-                             &def->mem.hard_limit, false) < 0)
+                             &def->mem.hard_limit, false, false) < 0)
         goto error;

     if (virDomainParseMemory("./memtune/soft_limit[1]", ctxt,
-                             &def->mem.soft_limit, false) < 0)
+                             &def->mem.soft_limit, false, false) < 0)
         goto error;

     if (virDomainParseMemory("./memtune/min_guarantee[1]", ctxt,
-                             &def->mem.min_guarantee, false) < 0)
+                             &def->mem.min_guarantee, false, false) < 0)
         goto error;

     if (virDomainParseMemory("./memtune/swap_hard_limit[1]", ctxt,
-                             &def->mem.swap_hard_limit, false) < 0)
+                             &def->mem.swap_hard_limit, false, false) < 0)
         goto error;

     n = virXPathULong("string(./vcpu[1])", ctxt, &count);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9908d88..fbb3b2f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1966,8 +1966,10 @@ typedef struct _virDomainMemtune virDomainMemtune;
 typedef virDomainMemtune *virDomainMemtunePtr;

 struct _virDomainMemtune {
-    unsigned long long max_balloon; /* in kibibytes */
-    unsigned long long cur_balloon; /* in kibibytes */
+    unsigned long long max_balloon; /* in kibibytes, capped at ulong thanks
+                                       to virDomainGetMaxMemory */
+    unsigned long long cur_balloon; /* in kibibytes, capped at ulong thanks
+                                       to virDomainGetInfo */

     virDomainHugePagePtr hugepages;
     size_t nhugepages;
@@ -1975,10 +1977,10 @@ struct _virDomainMemtune {
     bool nosharepages;
     bool locked;
     int dump_core; /* enum virTristateSwitch */
-    unsigned long long hard_limit; /* in kibibytes */
-    unsigned long long soft_limit; /* in kibibytes */
-    unsigned long long min_guarantee; /* in kibibytes */
-    unsigned long long swap_hard_limit; /* in kibibytes */
+    unsigned long long hard_limit; /* in kibibytes, limit at off_t bytes */
+    unsigned long long soft_limit; /* in kibibytes, limit at off_t bytes */
+    unsigned long long min_guarantee; /* in kibibytes, limit at off_t bytes */
+    unsigned long long swap_hard_limit; /* in kibibytes, limit at off_t bytes */
 };

 typedef struct _virDomainPowerManagement virDomainPowerManagement;
-- 
1.9.3

--
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]