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

[Martin Kletzander]

On Thu, Oct 30, 2014 at 02:00:34PM -0600, Eric Blake wrote:
>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

Oh, this is why it needs to be capped.  That makes sense.  And great
to have it in the comment next to those variables!

>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 at redhat.com>
>---
> 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.  */

The last sentence (Return ...) could be in its own paragraph even
though our documentation is not created for these functions.

ACK with that and safe for freeze and also build-breaker for 32-bit
machines.

Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20141031/0d7b7fd6/attachment-0001.sig>