xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Knowles <jonathan.knowles@eu.citrix.com>
To: Yu Zhiguo <yuzg@cn.fujitsu.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	Keir Fraser <Keir.Fraser@eu.citrix.com>,
	Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>
Subject: Re: [PATCH 0/2] xl: Add subcommand mem-max and fix mem-set
Date: Wed, 12 May 2010 16:33:31 +0100	[thread overview]
Message-ID: <4BEACA4B.7050701@eu.citrix.com> (raw)
In-Reply-To: <4BEA4A13.3000801@cn.fujitsu.com>

Hi all

As one of the maintainers of the Xen API tool-stack [1], I have
a couple of comments about these patches.

Currently, the Xen API tool-stack does not use libxenlight, but
we're interested in moving to it in the near future.

Yu Zhiguo wrote:
> I'm trying to add subcommand 'mem-max', I think
> xc_domain_setmaxmem should be used in it but not here.
>
> I'll move this code to 'mem-max', and in 'mem-set', a check
> should be added because setting memory larger than max memory
> is invalid.
>
> 1. Add 'mem-max' Add libxl_domain_setmaxmem, it calls
> xc_domain_setmaxmem. /local/domain/$domid/memory/static-max
> should be updated when set max memory, it is missing now.

 From the point of view of the Xen API tool-stack, these values
have different purposes.

1. The XenStore value: /local/domain/$domid/memory/static-max

    This value reflects the maximum amount of physical host memory
    that's addressable by the guest OS.

    The Xen API tool-stack writes this value into XenStore at the
    time of domain construction, and leaves the value alone until
    the domain is destroyed.

    I think it would make sense for libxenlight to do the same.

    Many balloon drivers read this value, along with the "target"
    field, in order to determine how large they should make their
    balloons:

    balloon size = (static-max - target)

    In the absence of hotplug support, balloon drivers expect the
    value of "static-max" to remain constant for the lifetime of
    the domain.

2. The Xen value: maxmem

   Xen uses this value as a sort of "ratchet", to prevent a domain
   that's currently ballooned down from growing too large.

   Xen will reject any call to domain_memory_increase_reservation
   that would increase the reservation beyond the value of maxmex.

   It's possible to change the value of "maxmex" while a domain is
   running. Indeed, whenever the Xen API tool-stack writes a new
   balloon target into XenStore, it also calls xc_domain_setmaxmem
   with the same value.

   This mechanism allows the tool-stack to prevent a balloon driver
   from claiming back more memory if the guest is already using more
   than its target memory allocation.

So, I think it's fair to say that locking these two values
together does not make sense from the point of view of the Xen API
tool-stack.

>>>>> 2. fix 'mem-set' Delete xc_domain_setmaxmem. Get max
>>>>>  memory from /local/domain/$domid/memory/static-max,
>>>>>  and then do value check. It seems that we cannot get
>>>>>  max memory use libxc routines.

If the purpose of "mem-set" is to set a balloon target for a
guest, then it might also make sense to call xc_domain_setmaxmem
with the same value, whenever the target changes, for the reasons
stated above (preventing a domain that's currently ballooned down
from growing too large).

>>>> I fixed it, please check.
>>>>
>>>> [PATCH 1/2] xl: Add command 'xl mem-max' [PATCH 2/2]
>>>> xl: Add check for command 'xl mem-set'

Cheers,
Jonathan

[1] http://www.xen.org/products/cloud_source.html

  parent reply	other threads:[~2010-05-12 15:33 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-07  9:39 [PATCH] xl: Update memory info in xenstore when use 'xl mem-set' Yu Zhiguo
2010-05-07 17:42 ` Ian Jackson
2010-05-09  8:45   ` Yu Zhiguo
2010-05-12  6:26     ` [PATCH 0/2] xl: Add subcommand mem-max and fix mem-set Yu Zhiguo
2010-05-12  6:41       ` Yu Zhiguo
2010-05-12 11:33         ` Stefano Stabellini
2010-05-12 11:39           ` Stefano Stabellini
2010-05-14  2:15         ` [PATCH] xl: Fix missing memory target in xenstore Yu Zhiguo
2010-05-12 15:33       ` Jonathan Knowles [this message]
2010-05-13 12:21         ` [PATCH 0/2] xl: Add subcommand mem-max and fix mem-set Stefano Stabellini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BEACA4B.7050701@eu.citrix.com \
    --to=jonathan.knowles@eu.citrix.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=Keir.Fraser@eu.citrix.com \
    --cc=Stefano.Stabellini@eu.citrix.com \
    --cc=xen-devel@lists.xensource.com \
    --cc=yuzg@cn.fujitsu.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).