[PATCH v2 0/2] libxl: memory management patches

[PATCH v2 0/2] libxl: memory management patches

[Daniel Kiper]

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

[PATCH v2 1/2] libxl: xl mem-max et consortes must update static-max in xenstore too

[Daniel Kiper]

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

Re: [PATCH v2 1/2] libxl: xl mem-max et consortes must update static-max in xenstore too

[Ian Jackson]

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.

Re: [PATCH v2 1/2] libxl: xl mem-max et consortes must update static-max in xenstore too

[Ian Campbell]

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.

[PATCH v2 2/2] xl: Add mem_set_enforce_limit option to xl.conf file

[Daniel Kiper]

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

Re: [PATCH v2 2/2] xl: Add mem_set_enforce_limit option to xl.conf file

[Konrad Rzeszutek Wilk]

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
> 

Re: [PATCH v2 2/2] xl: Add mem_set_enforce_limit option to xl.conf file

[Ian Jackson]

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.

Re: [PATCH v2 2/2] xl: Add mem_set_enforce_limit option to xl.conf file

[Konrad Rzeszutek Wilk]

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
> 

Re: [PATCH v2 0/2] libxl: memory management patches

[Daniel Kiper]

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