[PATCH 0/2] libxl: memory management patches

[PATCH 0/2] libxl: memory management patches

[Daniel Kiper]

Hi,

Here are two small but important libxl memory management patches:
  - libxl: xl mem-max et consortes must update static-max in xenstore too,
  - libxl: xl mem-set should not enforce memory limits.

Daniel

[PATCH 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>
---
 tools/libxl/libxl.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index a09c0fa..5edeaa6 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -3480,6 +3480,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

[PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[Daniel Kiper]

xl mem-set should not enforce memory limits. If it does
then after decreasing reservation there is no chance to
increase reservation directly from guest domain by
writing to /sys/devices/system/xen_memory/xen_memory0/target*

Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
---
 tools/libxl/xl_cmdimpl.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index a98705e..9add281 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2492,7 +2492,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, 0);
 }
 
 int main_memset(int argc, char **argv)
-- 
1.7.10.4

Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[Ian Campbell]

On Fri, 2013-03-15 at 16:25 +0000, Daniel Kiper wrote:
> xl mem-set should not enforce memory limits. If it does

NB the title says libxl but this is actually an xl change.

> then after decreasing reservation there is no chance to
> increase reservation directly from guest domain by
> writing to /sys/devices/system/xen_memory/xen_memory0/target*

This is a feature not a bug I think. It is intentional that if the host
admin sets a memory target the a guest admin cannot exceed that limit.

Ian.

> 
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> ---
>  tools/libxl/xl_cmdimpl.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index a98705e..9add281 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2492,7 +2492,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, 0);
>  }
>  
>  int main_memset(int argc, char **argv)

Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[Konrad Rzeszutek Wilk]

On Fri, Mar 15, 2013 at 04:32:53PM +0000, Ian Campbell wrote:
> On Fri, 2013-03-15 at 16:25 +0000, Daniel Kiper wrote:
> > xl mem-set should not enforce memory limits. If it does
> 
> NB the title says libxl but this is actually an xl change.
> 
> > then after decreasing reservation there is no chance to
> > increase reservation directly from guest domain by
> > writing to /sys/devices/system/xen_memory/xen_memory0/target*
> 
> This is a feature not a bug I think. It is intentional that if the host
> admin sets a memory target the a guest admin cannot exceed that limit.

Even thought 'xm' did not do it?
> 
> Ian.
> 
> > 
> > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> > ---
> >  tools/libxl/xl_cmdimpl.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index a98705e..9add281 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -2492,7 +2492,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, 0);
> >  }
> >  
> >  int main_memset(int argc, char **argv)
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[Ian Campbell]

On Fri, 2013-03-15 at 16:57 +0000, Konrad Rzeszutek Wilk wrote:
> On Fri, Mar 15, 2013 at 04:32:53PM +0000, Ian Campbell wrote:
> > On Fri, 2013-03-15 at 16:25 +0000, Daniel Kiper wrote:
> > > xl mem-set should not enforce memory limits. If it does
> > 
> > NB the title says libxl but this is actually an xl change.
> > 
> > > then after decreasing reservation there is no chance to
> > > increase reservation directly from guest domain by
> > > writing to /sys/devices/system/xen_memory/xen_memory0/target*
> > 
> > This is a feature not a bug I think. It is intentional that if the host
> > admin sets a memory target the a guest admin cannot exceed that limit.
> 
> Even thought 'xm' did not do it?

Yes. Please check the archives, this was discussed at the time (at
length IIRC)

Ian.

Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[Daniel Kiper]

On Fri, Mar 15, 2013 at 04:32:53PM +0000, Ian Campbell wrote:
> On Fri, 2013-03-15 at 16:25 +0000, Daniel Kiper wrote:
> > xl mem-set should not enforce memory limits. If it does
>
> NB the title says libxl but this is actually an xl change.

OK.

> > then after decreasing reservation there is no chance to
> > increase reservation directly from guest domain by
> > writing to /sys/devices/system/xen_memory/xen_memory0/target*
>
> This is a feature not a bug I think. It is intentional that if the host
> admin sets a memory target the a guest admin cannot exceed that limit.

I think that xl mem-max should be used to enforce limits. If admin
would like to enforce "hard" limit it should call xl mem-set and
xl mem-max in sequence. If we would like to leave old xl mem-set
behavior we should change comment for this command because now
it does not mention anythig about limit enforcement. Or we should
add an option which explicitly disables memory limit enforment
(this behavior is in line with xm mem-set behavior).

Daniel

Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[Ian Jackson]

Daniel Kiper writes ("Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits"):
> I think that xl mem-max should be used to enforce limits. If admin
> would like to enforce "hard" limit it should call xl mem-set and
> xl mem-max in sequence. If we would like to leave old xl mem-set
> behavior we should change comment for this command because now
> it does not mention anythig about limit enforcement. Or we should
> add an option which explicitly disables memory limit enforment
> (this behavior is in line with xm mem-set behavior).

I think this conversation is related to the fact that at Oracle you
have a different model of the Xen memory allocation model to everyone
else.

Outside Oracle, guests are supposed to aim for the balloon target and
are not permitted to exceed it (when ballooning up) or to regress
(when ballooning down).

Ian.

Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[Daniel Kiper]

On Fri, Mar 15, 2013 at 04:59:34PM +0000, Ian Campbell wrote:
> On Fri, 2013-03-15 at 16:57 +0000, Konrad Rzeszutek Wilk wrote:
> > On Fri, Mar 15, 2013 at 04:32:53PM +0000, Ian Campbell wrote:
> > > On Fri, 2013-03-15 at 16:25 +0000, Daniel Kiper wrote:
> > > > xl mem-set should not enforce memory limits. If it does
> > >
> > > NB the title says libxl but this is actually an xl change.
> > >
> > > > then after decreasing reservation there is no chance to
> > > > increase reservation directly from guest domain by
> > > > writing to /sys/devices/system/xen_memory/xen_memory0/target*
> > >
> > > This is a feature not a bug I think. It is intentional that if the host
> > > admin sets a memory target the a guest admin cannot exceed that limit.
> >
> > Even thought 'xm' did not do it?
>
> Yes. Please check the archives, this was discussed at the time (at
> length IIRC)

Could you remind thread subject (more or less)?
It was about ballooning or xm/xl itself?

Daniel

Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[George Dunlap]

On Fri, Mar 15, 2013 at 5:09 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> Daniel Kiper writes ("Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits"):
>> I think that xl mem-max should be used to enforce limits. If admin
>> would like to enforce "hard" limit it should call xl mem-set and
>> xl mem-max in sequence. If we would like to leave old xl mem-set
>> behavior we should change comment for this command because now
>> it does not mention anythig about limit enforcement. Or we should
>> add an option which explicitly disables memory limit enforment
>> (this behavior is in line with xm mem-set behavior).
>
> I think this conversation is related to the fact that at Oracle you
> have a different model of the Xen memory allocation model to everyone
> else.
>
> Outside Oracle, guests are supposed to aim for the balloon target and
> are not permitted to exceed it (when ballooning up) or to regress
> (when ballooning down).

Would it make sense to make the current behavior the default, but have
it override-able by a configuration variable in xl.conf?

 -George

Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[Daniel Kiper]

On Fri, Mar 15, 2013 at 05:09:51PM +0000, Ian Jackson wrote:
> Daniel Kiper writes ("Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits"):
> > I think that xl mem-max should be used to enforce limits. If admin
> > would like to enforce "hard" limit it should call xl mem-set and
> > xl mem-max in sequence. If we would like to leave old xl mem-set
> > behavior we should change comment for this command because now
> > it does not mention anythig about limit enforcement. Or we should
> > add an option which explicitly disables memory limit enforment
> > (this behavior is in line with xm mem-set behavior).
>
> I think this conversation is related to the fact that at Oracle you
> have a different model of the Xen memory allocation model to everyone
> else.
>
> Outside Oracle, guests are supposed to aim for the balloon target and
> are not permitted to exceed it (when ballooning up) or to regress
> (when ballooning down).

I think that it is possible to make "everyone else" and Oracle guys happy:
  - leave old xl mem-set behavior as is,
  - improve comment for this commnd,
  - add option which disables memory limit enforcement.

Is it OK for you?

Daniel

Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[Konrad Rzeszutek Wilk]

On Fri, Mar 15, 2013 at 05:09:51PM +0000, Ian Jackson wrote:
> Daniel Kiper writes ("Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits"):
> > I think that xl mem-max should be used to enforce limits. If admin
> > would like to enforce "hard" limit it should call xl mem-set and
> > xl mem-max in sequence. If we would like to leave old xl mem-set
> > behavior we should change comment for this command because now
> > it does not mention anythig about limit enforcement. Or we should
> > add an option which explicitly disables memory limit enforment
> > (this behavior is in line with xm mem-set behavior).
> 
> I think this conversation is related to the fact that at Oracle you
> have a different model of the Xen memory allocation model to everyone
> else.

Daniel is trying to fix an bug that Linux kernel is tripping over
b/c of this. Look at the converstation and patch that Daniel posted
a week ago for the Linux kernel.
> 
> Outside Oracle, guests are supposed to aim for the balloon target and
> are not permitted to exceed it (when ballooning up) or to regress
> (when ballooning down).

s/Oracle/Xend/. As Xend had this distinction. 'xm mem-set' would only set
the target. 'xm mem-max' on the other hand would enforce the limit.

Daniel is just bringing this behavior to 'xl'.

Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[Ian Campbell]

On Fri, 2013-03-15 at 17:26 +0000, Konrad Rzeszutek Wilk wrote:
> On Fri, Mar 15, 2013 at 05:09:51PM +0000, Ian Jackson wrote:
> > Daniel Kiper writes ("Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits"):
> > > I think that xl mem-max should be used to enforce limits. If admin
> > > would like to enforce "hard" limit it should call xl mem-set and
> > > xl mem-max in sequence. If we would like to leave old xl mem-set
> > > behavior we should change comment for this command because now
> > > it does not mention anythig about limit enforcement. Or we should
> > > add an option which explicitly disables memory limit enforment
> > > (this behavior is in line with xm mem-set behavior).
> > 
> > I think this conversation is related to the fact that at Oracle you
> > have a different model of the Xen memory allocation model to everyone
> > else.
> 
> Daniel is trying to fix an bug that Linux kernel is tripping over
> b/c of this. Look at the converstation and patch that Daniel posted
> a week ago for the Linux kernel.

It would be useful to mention (or at least) the rationale for a change
such as this in the commit message.

However that's rather moot in this case because the rationale is surely
wrong. Linux (and indeed balloon drivers generally) are expected to
behave correctly whether the toolstack chooses to be enforcing or
non-enforcing regarding the balloon target. So you can't "fix" the
kernel by simply mandating that all toolstacks are non-enforcing, sorry.

> > Outside Oracle, guests are supposed to aim for the balloon target and
> > are not permitted to exceed it (when ballooning up) or to regress
> > (when ballooning down).
> 
> s/Oracle/Xend/. As Xend had this distinction. 'xm mem-set' would only set
> the target. 'xm mem-max' on the other hand would enforce the limit.
> 
> Daniel is just bringing this behavior to 'xl'.

xl deliberately deviated from xend on this point.

Ian.

Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[Konrad Rzeszutek Wilk]

On Fri, Mar 15, 2013 at 08:01:00PM +0000, Ian Campbell wrote:
> On Fri, 2013-03-15 at 17:26 +0000, Konrad Rzeszutek Wilk wrote:
> > On Fri, Mar 15, 2013 at 05:09:51PM +0000, Ian Jackson wrote:
> > > Daniel Kiper writes ("Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits"):
> > > > I think that xl mem-max should be used to enforce limits. If admin
> > > > would like to enforce "hard" limit it should call xl mem-set and
> > > > xl mem-max in sequence. If we would like to leave old xl mem-set
> > > > behavior we should change comment for this command because now
> > > > it does not mention anythig about limit enforcement. Or we should
> > > > add an option which explicitly disables memory limit enforment
> > > > (this behavior is in line with xm mem-set behavior).
> > > 
> > > I think this conversation is related to the fact that at Oracle you
> > > have a different model of the Xen memory allocation model to everyone
> > > else.
> > 
> > Daniel is trying to fix an bug that Linux kernel is tripping over
> > b/c of this. Look at the converstation and patch that Daniel posted
> > a week ago for the Linux kernel.
> 
> It would be useful to mention (or at least) the rationale for a change
> such as this in the commit message.
> 
> However that's rather moot in this case because the rationale is surely
> wrong. Linux (and indeed balloon drivers generally) are expected to
> behave correctly whether the toolstack chooses to be enforcing or
> non-enforcing regarding the balloon target. So you can't "fix" the
> kernel by simply mandating that all toolstacks are non-enforcing, sorry.
> 
> > > Outside Oracle, guests are supposed to aim for the balloon target and
> > > are not permitted to exceed it (when ballooning up) or to regress
> > > (when ballooning down).
> > 
> > s/Oracle/Xend/. As Xend had this distinction. 'xm mem-set' would only set
> > the target. 'xm mem-max' on the other hand would enforce the limit.
> > 
> > Daniel is just bringing this behavior to 'xl'.
> 
> xl deliberately deviated from xend on this point.

Ah, I did not dig deep enough in the git annotate to see if there was a
story behind it. Perhaps then it would make sense to do two things:

 1). Add a comment in the code explicitly mentioning it.

 2). If we really want to provide an Xend-type behavior add an global
     configuration value that is called "xend-backwarts=1' ? That way
     we can fit all the other stuff that is inconsistent underneath
     that (such as timer_mode changing from number to string, etc).

     And as part of this global value also explain in the docs what
     those inconsistencies are?

> 
> Ian.
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

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

[Konrad Rzeszutek Wilk]

On Fri, Mar 15, 2013 at 05:25:01PM +0100, Daniel Kiper wrote:
> 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.

Reviewed-by: Konrad Rzeszutek Wilk <konrad@kernel.org>
> 
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
> ---
>  tools/libxl/libxl.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index a09c0fa..5edeaa6 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -3480,6 +3480,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
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
> 

Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[Ian Campbell]

On Sat, 2013-03-16 at 13:37 +0000, Konrad Rzeszutek Wilk wrote:
>  1). Add a comment in the code explicitly mentioning it.

That seems reasonable. It could/should even be in the manpage I guess?

>  2). If we really want to provide an Xend-type behavior add an global
>      configuration value that is called "xend-backwarts=1' ? That way
>      we can fit all the other stuff that is inconsistent underneath
>      that (such as timer_mode changing from number to string, etc).
> 
>      And as part of this global value also explain in the docs what
>      those inconsistencies are?

I'd rather see specific options for specific cases where there is a
genuine choice to be made rather than a catch-all global option.

I think we take care of the old for of timer_mode already, don't we?

Ian.

Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits

[Konrad Rzeszutek Wilk]

On Fri, Mar 15, 2013 at 08:01:00PM +0000, Ian Campbell wrote:
> On Fri, 2013-03-15 at 17:26 +0000, Konrad Rzeszutek Wilk wrote:
> > On Fri, Mar 15, 2013 at 05:09:51PM +0000, Ian Jackson wrote:
> > > Daniel Kiper writes ("Re: [PATCH 2/2] libxl: xl mem-set should not enforce memory limits"):
> > > > I think that xl mem-max should be used to enforce limits. If admin
> > > > would like to enforce "hard" limit it should call xl mem-set and
> > > > xl mem-max in sequence. If we would like to leave old xl mem-set
> > > > behavior we should change comment for this command because now
> > > > it does not mention anythig about limit enforcement. Or we should
> > > > add an option which explicitly disables memory limit enforment
> > > > (this behavior is in line with xm mem-set behavior).
> > > 
> > > I think this conversation is related to the fact that at Oracle you
> > > have a different model of the Xen memory allocation model to everyone
> > > else.
> > 
> > Daniel is trying to fix an bug that Linux kernel is tripping over
> > b/c of this. Look at the converstation and patch that Daniel posted
> > a week ago for the Linux kernel.
> 
> It would be useful to mention (or at least) the rationale for a change
> such as this in the commit message.
> 
> However that's rather moot in this case because the rationale is surely
> wrong. Linux (and indeed balloon drivers generally) are expected to
> behave correctly whether the toolstack chooses to be enforcing or
> non-enforcing regarding the balloon target. So you can't "fix" the
> kernel by simply mandating that all toolstacks are non-enforcing, sorry.
> 
> > > Outside Oracle, guests are supposed to aim for the balloon target and
> > > are not permitted to exceed it (when ballooning up) or to regress
> > > (when ballooning down).
> > 
> > s/Oracle/Xend/. As Xend had this distinction. 'xm mem-set' would only set
> > the target. 'xm mem-max' on the other hand would enforce the limit.
> > 
> > Daniel is just bringing this behavior to 'xl'.
> 
> xl deliberately deviated from xend on this point.

This was :

# HG changeset patch
# User Keir Fraser <keir.fraser@citrix.com>
# Date 1273819996 -3600
# Node ID b29e42cb4727d718025b4b7039b35a824c8a11d3
# Parent  9b9a277d8a51de049068d3251b5d84b73a24196b
libxl: Adjustments to memset/memmax handling

I think xl memset should change the memory currently used by the guest
and xl memmax should change the size of the guest's address space and
not the population.  For this reason libxl_set_memory_target should
provide a way to enforce the memory target, calling
xc_domain_setmaxmem.  On the other hand xl memmax shouldn't call
xc_domain_setmaxmem because that is the upper bound of the memory
reservation, it should just change static-max, that at the moment
wouldn't do much, but we can imagine that in the future could trigger
something useful in the guest.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Jonathan Knowles <Jonathan.Knowles@eu.citrix.com>=20

which just says: "I think". Was there some more background behind this?