Xen devel - Mar 2013 - [PATCH v2 0/2] libxl: memory management patches

Hi,

Here are two small but important libxl/xl memory management patches:
  - libxl: xl mem-max et consortes must update static-max in xenstore too,
  - xl: Add mem_set_enforce_limit option to xl.conf file.

Daniel

xl mem-max et consortes must update static-max in xenstore too.
Without this patch there is no chance to increase memory reservation
for given domain above memory limit defined in config file. It means
that memory hotplug is practicaly unusable without this patch.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad@kernel.org>
---
 tools/libxl/libxl.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 572c2c6..6f4cf09 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3484,6 +3484,9 @@ int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid,
uint32_t max_memkb)
         goto out;
     }
 
+    libxl__xs_write(gc, XBT_NULL, libxl__sprintf(gc,
"%s/memory/static-max",
+			    dompath), "%"PRIu32, max_memkb);
+
     rc = 0;
 out:
     GC_FREE;
-- 
1.7.10.4

Add mem_set_enforce_limit option to xl.conf file.
It gives a chance to align xl mem-set behavior
to xm mem-set behavior. Default xl mem-set
behavior is not changed.

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 docs/man/xl.conf.pod.5   |    9 +++++++++
 tools/examples/xl.conf   |    4 ++++
 tools/libxl/xl.c         |    4 ++++
 tools/libxl/xl.h         |    1 +
 tools/libxl/xl_cmdimpl.c |    2 +-
 5 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
index 7b9fcac..4e489dd 100644
--- a/docs/man/xl.conf.pod.5
+++ b/docs/man/xl.conf.pod.5
@@ -55,6 +55,15 @@ reduce the amount of memory given to domain 0 by default.
 
 Default: C<1>
 
+=item B<mem_set_enforce_limit=BOOLEAN>
+
+If enabled then C<xl mem-set> will set memory allocation target
+and enforce maximum memory allocation for given domain.
+If disabled then C<xl mem-set> will set memory allocation target
+only for given domain.
+
+Default: C<1>
+
 =item B<run_hotplug_scripts=BOOLEAN>
 
 If disabled hotplug scripts will be called from udev, as it used to
diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
index b0caa32..7913bd8 100644
--- a/tools/examples/xl.conf
+++ b/tools/examples/xl.conf
@@ -4,6 +4,10 @@
 # memory to create a domain
 #autoballoon=1
 
+# xl mem-set will set memory allocation target and enforce
+# maximum memory allocation for given domain
+#mem_set_enforce_limit=1
+
 # 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 4c598db..5c445ca 100644
--- a/tools/libxl/xl.c
+++ b/tools/libxl/xl.c
@@ -38,6 +38,7 @@ xentoollog_logger_stdiostream *logger;
 int dryrun_only;
 int force_execution;
 int autoballoon = 1;
+int mem_set_enforce_limit = 1;
 char *blkdev_start;
 int run_hotplug_scripts = 1;
 char *lockfile;
@@ -72,6 +73,9 @@ static void parse_global_config(const char *configfile,
     if (!xlu_cfg_get_long (config, "autoballoon", &l, 0))
         autoballoon = l;
 
+    if (!xlu_cfg_get_long (config, "mem_set_enforce_limit", &l,
0))
+        mem_set_enforce_limit = l;
+
     if (!xlu_cfg_get_long (config, "run_hotplug_scripts", &l, 0))
         run_hotplug_scripts = l;
 
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index b881f92..321a0d0 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -143,6 +143,7 @@ int xl_child_pid(xlchildnum); /* returns 0 if child struct
is not in use */
 
 /* global options */
 extern int autoballoon;
+extern int mem_set_enforce_limit;
 extern int run_hotplug_scripts;
 extern int dryrun_only;
 extern char *lockfile;
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 2d40f8f..9c83afd 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2505,7 +2505,7 @@ static void set_memory_target(uint32_t domid, const char
*mem)
         exit(3);
     }
 
-    libxl_set_memory_target(ctx, domid, memorykb, 0, /* enforce */ 1);
+    libxl_set_memory_target(ctx, domid, memorykb, 0, mem_set_enforce_limit);
 }
 
 int main_memset(int argc, char **argv)
-- 
1.7.10.4

On Thu, Mar 21, 2013 at 02:52:59PM +0100, Daniel Kiper
wrote:
> Add mem_set_enforce_limit option to xl.conf file.
> It gives a chance to align xl mem-set behavior
> to xm mem-set behavior. Default xl mem-set
> behavior is not changed.
> 
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> ---
>  docs/man/xl.conf.pod.5   |    9 +++++++++
>  tools/examples/xl.conf   |    4 ++++
>  tools/libxl/xl.c         |    4 ++++
>  tools/libxl/xl.h         |    1 +
>  tools/libxl/xl_cmdimpl.c |    2 +-
>  5 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/man/xl.conf.pod.5 b/docs/man/xl.conf.pod.5
> index 7b9fcac..4e489dd 100644
> --- a/docs/man/xl.conf.pod.5
> +++ b/docs/man/xl.conf.pod.5
> @@ -55,6 +55,15 @@ reduce the amount of memory given to domain 0 by
default.
>  
>  Default: C<1>
>  
> +=item B<mem_set_enforce_limit=BOOLEAN>
> +
> +If enabled then C<xl mem-set> will set memory allocation target
> +and enforce maximum memory allocation for given domain.
I know what that means but I don''t know if a normal user has any idea
what ''maximum memory allocation'' or ''memory
allocation target'' means.

Do you think it could be possible to explain it a bit more so that
a less technical person can understand it?

Otherwise, Reviewed-by: Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com>
> +If disabled then C<xl mem-set> will set memory allocation target
> +only for given domain.
> +
> +Default: C<1>
> +
>  =item B<run_hotplug_scripts=BOOLEAN>
>  
>  If disabled hotplug scripts will be called from udev, as it used to
> diff --git a/tools/examples/xl.conf b/tools/examples/xl.conf
> index b0caa32..7913bd8 100644
> --- a/tools/examples/xl.conf
> +++ b/tools/examples/xl.conf
> @@ -4,6 +4,10 @@
>  # memory to create a domain
>  #autoballoon=1
>  
> +# xl mem-set will set memory allocation target and enforce
> +# maximum memory allocation for given domain
> +#mem_set_enforce_limit=1
> +
>  # 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 4c598db..5c445ca 100644
> --- a/tools/libxl/xl.c
> +++ b/tools/libxl/xl.c
> @@ -38,6 +38,7 @@ xentoollog_logger_stdiostream *logger;
>  int dryrun_only;
>  int force_execution;
>  int autoballoon = 1;
> +int mem_set_enforce_limit = 1;
>  char *blkdev_start;
>  int run_hotplug_scripts = 1;
>  char *lockfile;
> @@ -72,6 +73,9 @@ static void parse_global_config(const char *configfile,
>      if (!xlu_cfg_get_long (config, "autoballoon", &l, 0))
>          autoballoon = l;
>  
> +    if (!xlu_cfg_get_long (config, "mem_set_enforce_limit",
&l, 0))
> +        mem_set_enforce_limit = l;
> +
>      if (!xlu_cfg_get_long (config, "run_hotplug_scripts",
&l, 0))
>          run_hotplug_scripts = l;
>  
> diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> index b881f92..321a0d0 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -143,6 +143,7 @@ int xl_child_pid(xlchildnum); /* returns 0 if child
struct is not in use */
>  
>  /* global options */
>  extern int autoballoon;
> +extern int mem_set_enforce_limit;
>  extern int run_hotplug_scripts;
>  extern int dryrun_only;
>  extern char *lockfile;
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 2d40f8f..9c83afd 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2505,7 +2505,7 @@ static void set_memory_target(uint32_t domid, const
char *mem)
>          exit(3);
>      }
>  
> -    libxl_set_memory_target(ctx, domid, memorykb, 0, /* enforce */ 1);
> +    libxl_set_memory_target(ctx, domid, memorykb, 0,
mem_set_enforce_limit);
>  }
>  
>  int main_memset(int argc, char **argv)
> -- 
> 1.7.10.4
>

Daniel Kiper writes ("[PATCH v2 2/2] xl: Add mem_set_enforce_limit option
to xl.conf file"):
> Add mem_set_enforce_limit option to xl.conf file.
> It gives a chance to align xl mem-set behavior
> to xm mem-set behavior. Default xl mem-set
> behavior is not changed.
Should this be a per-domain config option ?  I confess I don''t
understand the usage model very well but it would seem that enforcing
against some guests but not others would be a sensible otpion.

Ian.

Daniel Kiper writes ("[PATCH v2 1/2] libxl: xl mem-max et consortes must
update static-max in xenstore too"):
> xl mem-max et consortes must update static-max in xenstore too.
> Without this patch there is no chance to increase memory reservation
> for given domain above memory limit defined in config file. It means
> that memory hotplug is practicaly unusable without this patch.
I haven''t checked the details here but iirc there is a doc comment in
libxl about guest memory management.  Either that comment is wrong, in
which case you need to patch it too (and your explanation needs to
discuss it), or the comment is right but the code doesn''t agree, in
which case mentioning it in the commit message would help me a lot.

Thanks,
Ian.

On Mon, Mar 25, 2013 at 04:15:26PM +0000, Ian Jackson
wrote:
> Daniel Kiper writes ("[PATCH v2 2/2] xl: Add mem_set_enforce_limit
option to xl.conf file"):
> > Add mem_set_enforce_limit option to xl.conf file.
> > It gives a chance to align xl mem-set behavior
> > to xm mem-set behavior. Default xl mem-set
> > behavior is not changed.
> 
> Should this be a per-domain config option ?  I confess I don''t
> understand the usage model very well but it would seem that enforcing
> against some guests but not others would be a sensible otpion.
This option is to provide a mechanism to fallback to xend behavior.
> 
> Ian.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

On Thu, Mar 21, 2013 at 02:52:57PM +0100, Daniel Kiper
wrote:
> Hi,
>
> Here are two small but important libxl/xl memory management patches:
>   - libxl: xl mem-max et consortes must update static-max in xenstore too,
>   - xl: Add mem_set_enforce_limit option to xl.conf file.
Thanks guys for comments. I will send new version of patches
after my holiday in the middle of next week.

Daniel

On Mon, 2013-03-25 at 16:17 +0000, Ian Jackson wrote:
> Daniel Kiper writes ("[PATCH v2 1/2] libxl: xl mem-max et consortes
must update static-max in xenstore too"):
> > xl mem-max et consortes must update static-max in xenstore too.
> > Without this patch there is no chance to increase memory reservation
> > for given domain above memory limit defined in config file. It means
> > that memory hotplug is practicaly unusable without this patch.
> 
> I haven''t checked the details here but iirc there is a doc comment
in
> libxl about guest memory management.
I think you are thinking of tools/libxl/libxl_memory.txt

As a more general comment do we want to separate the operations for
memory hotplug from those which just balloon a guest within its existing
static-max?

Ian.