Xen devel - Apr 2013 - [PATCH] xl: extend autoballoon xl.conf option with an "auto" option

From: David Vrabel <david.vrabel@citrix.com>

autoballoon=1 is not recommened if dom0_mem was used to reduce the
amount of dom0 memory.  Instead of requiring users to change xl.conf
if they do this, extend the autoballoon option with a new choice:
"auto".

With autoballoon="auto", autoballooning will be disabled if dom0_mem
was used on the Xen command line.

For consistency, accept "on" and "off" as valid autoballoon
options (1
and 0 are still accepted).

The default remains "on" so it is recommended that packagers provide
an xl.cong with autoballoon="auto".

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
 docs/man/xl.conf.pod.5 |   21 ++++++++++++++-------
 tools/examples/xl.conf |    2 +-
 tools/libxl/xl.c       |   31 +++++++++++++++++++++++++++++--
 3 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 82c6b20..959f494 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -45,15 +45,22 @@ The semantics of each C<KEY> defines which form of
C<VALUE> is required.
 
 =over 4
 
-=item B<autoballoon=BOOLEAN>
+=item B<autoballoon="off"|"on"|"auto">
 
-If disabled then C<xl> will not attempt to reduce the amount of memory
-assigned to domain 0 in order to create free memory when starting a
-new domain. You are strongly recommended to set this to C<0>
-(C<False>) if you use the C<dom0_mem> hypervisor command line to
-reduce the amount of memory given to domain 0 by default.
+If set to "on" then C<xl> will automatically reduce the amount
of
+memory assigned to domain 0 in order to free memory for new domains.
 
-Default: C<1>
+If set to "off" then C<xl> will not automatically reduce the
amount of
+domain 0 memory.
+
+If set to "auto" then auto-ballooning will be disabled if the
+C<dom0_mem> option was provided on the Xen command line.
+
+You are strongly recommended to set this to C<"off"> (or
C<"auto">) if
+you use the C<dom0_mem> hypervisor command line to reduce the amount
+of memory given to domain 0 by default.
+
+Default: C<"on">
 
 =item B<run_hotplug_scripts=BOOLEAN>
 
diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
index 28ab796..b3db5db 100644
--- a/tools/examples/xl.conf
+++ b/tools/examples/xl.conf
@@ -2,7 +2,7 @@
 
 # automatically balloon down dom0 when xen doesn''t have enough free
 # memory to create a domain
-#autoballoon=1
+#autoballoon="auto"
 
 # full path of the lockfile used by xl during domain creation
 #lockfile="/var/lock/xl"
diff --git a/tools/libxl/xl.c b/tools/libxl/xl.c
index ecbcd3b..bddb7be 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -47,6 +47,24 @@ enum output_format default_output_format =
OUTPUT_FORMAT_JSON;
 
 static xentoollog_level minmsglevel = XTL_PROGRESS;
 
+/* Default autoballoon value based on presence of dom0_mem Xen command
+   line option. */
+static int auto_autoballoon(void)
+{
+    const libxl_version_info *info;
+
+    info = libxl_get_version_info(ctx);
+    if (!info) {
+        fprintf(stderr, "libxl_get_version_info failed.\n");
+        return 1; /* default to autoballooning */
+    }
+
+    if (strstr(info->commandline, "dom0_mem"))
+        return 0;
+
+    return 1;
+}
+
 static void parse_global_config(const char *configfile,
                               const char *configfile_data,
                               int configfile_len)
@@ -68,8 +86,17 @@ static void parse_global_config(const char *configfile,
         exit(1);
     }
 
-    if (!xlu_cfg_get_long (config, "autoballoon", &l, 0))
-        autoballoon = l;
+    if (!xlu_cfg_get_string(config, "autoballoon", &buf, 0)) {
+        printf("opt\n");
+        if (!strcmp(buf, "on") || !strcmp(buf, "1"))
+            autoballoon = 1;
+        else if (!strcmp(buf, "off") || !strcmp(buf, "0"))
+            autoballoon = 0;
+        else if (!strcmp(buf, "auto"))
+            autoballoon = auto_autoballoon();
+        else
+            fprintf(stderr, "invalid autoballoon option");
+    }
 
     if (!xlu_cfg_get_long (config, "run_hotplug_scripts", &l, 0))
         run_hotplug_scripts = l;
-- 
1.7.2.5

David Vrabel writes ("[PATCH] xl: extend autoballoon xl.conf option with an
"auto" option"):
> autoballoon=1 is not recommened if dom0_mem was used to reduce the
> amount of dom0 memory.  Instead of requiring users to change xl.conf
> if they do this, extend the autoballoon option with a new choice:
> "auto".
I really like the principle of this patch.

But:
> +
> +Default: C<"on">
I would like to see another patch changing the default.
> +    if (strstr(info->commandline, "dom0_mem"))
> +        return 0;
Sadly your command line parsing needs to be more sophisticated than
this.  Consider:

 xen blah vmlinuz blah initrd.gz blah init=/etc/init.try-without-dom0_mem

Ian.

On Thu, 4 Apr 2013, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
> 
> autoballoon=1 is not recommened if dom0_mem was used to reduce the
> amount of dom0 memory.  Instead of requiring users to change xl.conf
> if they do this, extend the autoballoon option with a new choice:
> "auto".
> 
> With autoballoon="auto", autoballooning will be disabled if
dom0_mem
> was used on the Xen command line.
> 
> For consistency, accept "on" and "off" as valid
autoballoon options (1
> and 0 are still accepted).
> 
> The default remains "on" so it is recommended that packagers
provide
> an xl.cong with autoballoon="auto".
Even though I dislike having to parse the Xen command line to solve this
issue, having seen how many problems the autoballoon option can cause, I
am very supportive of this change.


> diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
> index 28ab796..b3db5db 100644
> --- a/tools/examples/xl.conf
> +++ b/tools/examples/xl.conf
> @@ -2,7 +2,7 @@
>  
>  # automatically balloon down dom0 when xen doesn''t have enough
free
>  # memory to create a domain
> -#autoballoon=1
> +#autoballoon="auto"
please update the comment on this file too

On Thu, 2013-04-04 at 14:30 +0100, Ian Jackson wrote:
> David Vrabel writes ("[PATCH] xl: extend autoballoon xl.conf option
with an "auto" option"):
> > autoballoon=1 is not recommened if dom0_mem was used to reduce the
> > amount of dom0 memory.  Instead of requiring users to change xl.conf
> > if they do this, extend the autoballoon option with a new choice:
> > "auto".
> 
> I really like the principle of this patch.
> 
> But:
> 
> > +
> > +Default: C<"on">
> 
> I would like to see another patch changing the default.
> 
> > +    if (strstr(info->commandline, "dom0_mem"))
> > +        return 0;
> 
> Sadly your command line parsing needs to be more sophisticated than
> this.  Consider:
> 
>  xen blah vmlinuz blah initrd.gz blah init=/etc/init.try-without-dom0_mem
You are correct although your example isn''t because the Xen command
line
in this case would only include the first "blah" (or else your
hypothetical bootloader which implements this syntax is doing something
non-intuitive).

Ian.