[PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target

[PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target

[Stefano Stabellini]

libxl: introduce libxl_set_relative_memory_target

Introduce libxl_set_relative_memory_target to modify the memory target
of a domain by a relative amount of memory in a single xenstore
transaction.
Modify libxl_set_memory_target to use xenstore transactions.
The first time we are reading/writing dom0 memory target, fill the
informations in xenstore if they are missing.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff -r 990ff10b7b00 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Wed Aug 25 20:59:35 2010 +0100
+++ b/tools/libxl/libxl.c	Wed Aug 25 21:01:36 2010 +0100
@@ -2751,56 +2751,243 @@ out:
     return rc;
 }
 
+static int fill_dom0_memory_info(libxl_gc *gc, uint32_t *target_memkb)
+{
+    int rc;
+    libxl_dominfo info;
+    char *target = NULL, *endptr = NULL;
+    char *target_path = "/local/domain/0/memory/target";
+    char *max_path = "/local/domain/0/memory/static-max";
+    xs_transaction_t t;
+    libxl_ctx *ctx = libxl_gc_owner(gc);
+
+retry_transaction:
+    t = xs_transaction_start(ctx->xsh);
+
+    target = libxl_xs_read(gc, t, target_path);
+    if (target) {
+        *target_memkb = strtoul(target, &endptr, 10);
+        if (*endptr != '\0') {
+            XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
+                    "invalid memory target %s from %s\n", target, target_path);
+            rc = ERROR_FAIL;
+            goto out;
+        }
+        rc = 0;
+        goto out;
+    }
+
+    rc = libxl_domain_info(ctx, &info, 0);
+    if (rc < 0)
+        return rc;
+
+    libxl_xs_write(gc, t, target_path, "%"PRIu32, (uint32_t) info.target_memkb);
+    libxl_xs_write(gc, t, max_path, "%"PRIu32, (uint32_t) info.max_memkb);
+
+    *target_memkb = (uint32_t) info.target_memkb;
+    rc = 0;
+
+out:
+    if (!xs_transaction_end(ctx->xsh, t, 0))
+        if (errno == EAGAIN)
+            goto retry_transaction;
+
+
+    return rc;
+}
+
 int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb, int enforce)
 {
     libxl_gc gc = LIBXL_INIT_GC(ctx);
-    int rc = 1;
-    uint32_t memorykb = 0, videoram = 0;
-    char *memmax, *endptr, *videoram_s = NULL;
+    int rc = 1, abort = 0;
+    uint32_t videoram = 0;
+    char *videoram_s = NULL;
     char *dompath = libxl_xs_get_dompath(&gc, domid);
     xc_domaininfo_t info;
     libxl_dominfo ptr;
     char *uuid;
-
-    if (domid) {
-        memmax = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/memory/static-max", dompath));
-        if (!memmax) {
+    xs_transaction_t t;
+
+retry_transaction:
+    t = xs_transaction_start(ctx->xsh);
+
+    videoram_s = libxl_xs_read(&gc, t, libxl_sprintf(&gc, "%s/memory/videoram", dompath));
+    videoram = videoram_s ? atoi(videoram_s) : 0;
+
+    if (enforce) {
+        rc = xc_domain_setmaxmem(ctx->xch, domid, target_memkb + LIBXL_MAXMEM_CONSTANT);
+        if (rc != 0) {
             XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
-                "cannot get memory info from %s/memory/static-max\n", dompath);
+                    "xc_domain_setmaxmem domid=%d memkb=%d failed "
+                    "rc=%d\n", domid, target_memkb + LIBXL_MAXMEM_CONSTANT, rc);
+            abort = 1;
             goto out;
         }
-        memorykb = strtoul(memmax, &endptr, 10);
+        libxl_xs_write(&gc, t, libxl_sprintf(&gc, "%s/memory/static-max", dompath), "%"PRIu32, target_memkb);
+    }
+
+    rc = xc_domain_memory_set_pod_target(ctx->xch, domid, (target_memkb - videoram) / 4, NULL, NULL, NULL);
+    if (rc != 0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
+                "xc_domain_memory_set_pod_target domid=%d, memkb=%d "
+                "failed rc=%d\n", domid, (target_memkb - videoram) / 4,
+                rc);
+        abort = 1;
+        goto out;
+    }
+
+    libxl_xs_write(&gc, t, libxl_sprintf(&gc, "%s/memory/target", dompath), "%"PRIu32, target_memkb);
+    rc = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
+    if (rc != 1 || info.domain != domid) {
+        abort = 1;
+        goto out;
+    }
+    xcinfo2xlinfo(&info, &ptr);
+    uuid = libxl_uuid2string(&gc, ptr.uuid);
+    libxl_xs_write(&gc, t, libxl_sprintf(&gc, "/vm/%s/memory", uuid), "%"PRIu32, target_memkb / 1024);
+
+out:
+    if (!xs_transaction_end(ctx->xsh, t, abort) && !abort)
+        if (errno == EAGAIN)
+            goto retry_transaction;
+
+    libxl_free_all(&gc);
+    return rc;
+}
+
+int libxl_set_relative_memory_target(libxl_ctx *ctx, uint32_t domid, int32_t relative_target_memkb, int enforce)
+{
+    libxl_gc gc = LIBXL_INIT_GC(ctx);
+    int rc = 1, abort = 0;
+    uint32_t memorykb = 0, videoram = 0, target_memkb = 0, new_target_memkb = 0;
+    char *memmax, *endptr, *videoram_s = NULL, *target = NULL;
+    char *dompath = libxl_xs_get_dompath(&gc, domid);
+    xc_domaininfo_t info;
+    libxl_dominfo ptr;
+    char *uuid;
+    xs_transaction_t t;
+
+retry_transaction:
+    t = xs_transaction_start(ctx->xsh);
+
+    target = libxl_xs_read(&gc, t, libxl_sprintf(&gc, "%s/memory/target", dompath));
+    if (!target && !domid) {
+        xs_transaction_end(ctx->xsh, t, 1);
+        rc = fill_dom0_memory_info(&gc, &target_memkb);
+        if (rc < 0) {
+            abort = 1;
+            goto out;
+        }
+        goto retry_transaction;
+    } else if (!target) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
+                "cannot get target memory info from %s/memory/target\n", dompath);
+        abort = 1;
+        goto out;
+    } else {
+        target_memkb = strtoul(target, &endptr, 10);
         if (*endptr != '\0') {
             XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
-                "invalid max memory %s from %s/memory/static-max\n", memmax, dompath);
-            goto out;
-        }
-
-        if (target_memkb > memorykb) {
-            XL_LOG(ctx, XL_LOG_ERROR,
-                "memory_dynamic_max must be less than or equal to memory_static_max\n");
+                    "invalid memory target %s from %s/memory/target\n", target, dompath);
+            abort = 1;
             goto out;
         }
     }
-
-    videoram_s = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/memory/videoram", dompath));
+    memmax = libxl_xs_read(&gc, t, libxl_sprintf(&gc, "%s/memory/static-max", dompath));
+    if (!memmax) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
+                "cannot get memory info from %s/memory/static-max\n", dompath);
+        abort = 1;
+        goto out;
+    }
+    memorykb = strtoul(memmax, &endptr, 10);
+    if (*endptr != '\0') {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
+                "invalid max memory %s from %s/memory/static-max\n", memmax, dompath);
+        abort = 1;
+        goto out;
+    }
+
+    new_target_memkb = target_memkb + relative_target_memkb;
+    if (new_target_memkb > memorykb) {
+        XL_LOG(ctx, XL_LOG_ERROR,
+                "memory_dynamic_max must be less than or equal to memory_static_max\n");
+        abort = 1;
+        goto out;
+    }
+
+    videoram_s = libxl_xs_read(&gc, t, libxl_sprintf(&gc, "%s/memory/videoram", dompath));
     videoram = videoram_s ? atoi(videoram_s) : 0;
 
-    libxl_xs_write(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/memory/target", dompath), "%"PRIu32, target_memkb);
-
+    if (enforce) {
+        memorykb = new_target_memkb;
+        rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb + LIBXL_MAXMEM_CONSTANT);
+        if (rc != 0) {
+            XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
+                    "xc_domain_setmaxmem domid=%d memkb=%d failed "
+                    "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT, rc);
+            abort = 1;
+            goto out;
+        }
+        libxl_xs_write(&gc, t, libxl_sprintf(&gc, "%s/memory/static-max", dompath), "%"PRIu32, memorykb);
+    }
+
+    rc = xc_domain_memory_set_pod_target(ctx->xch, domid, (new_target_memkb - videoram) / 4, NULL, NULL, NULL);
+    if (rc != 0) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
+                "xc_domain_memory_set_pod_target domid=%d, memkb=%d "
+                "failed rc=%d\n", domid, (new_target_memkb - videoram) / 4,
+                rc);
+        abort = 1;
+        goto out;
+    }
+
+    libxl_xs_write(&gc, t, libxl_sprintf(&gc, "%s/memory/target", dompath), "%"PRIu32, new_target_memkb);
     rc = xc_domain_getinfolist(ctx->xch, domid, 1, &info);
-    if (rc != 1 || info.domain != domid)
+    if (rc != 1 || info.domain != domid) {
+        abort = 1;
         goto out;
+    }
     xcinfo2xlinfo(&info, &ptr);
     uuid = libxl_uuid2string(&gc, ptr.uuid);
-    libxl_xs_write(&gc, XBT_NULL, libxl_sprintf(&gc, "/vm/%s/memory", uuid), "%"PRIu32, target_memkb / 1024);
-
-    if (enforce || !domid)
-        memorykb = target_memkb;
-    rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb + LIBXL_MAXMEM_CONSTANT);
-    if (rc != 0)
+    libxl_xs_write(&gc, t, libxl_sprintf(&gc, "/vm/%s/memory", uuid), "%"PRIu32, new_target_memkb / 1024);
+
+out:
+    if (!xs_transaction_end(ctx->xsh, t, abort) && !abort)
+        if (errno == EAGAIN)
+            goto retry_transaction;
+
+    libxl_free_all(&gc);
+    return rc;
+}
+
+int libxl_get_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t *out_target)
+{
+    libxl_gc gc = LIBXL_INIT_GC(ctx);
+    int rc = 1;
+    char *target = NULL, *endptr = NULL;
+    char *dompath = libxl_xs_get_dompath(&gc, domid);
+    uint32_t target_memkb;
+
+    target = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/memory/target", dompath));
+    if (!target && !domid) {
+        rc = fill_dom0_memory_info(&gc, &target_memkb);
+        if (rc < 0)
+            goto out;
+    } else if (!target) {
+        XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
+                "cannot get target memory info from %s/memory/target\n", dompath);
         goto out;
-    rc = xc_domain_memory_set_pod_target(ctx->xch, domid, (target_memkb - videoram) / 4, NULL, NULL, NULL);
+    } else {
+        target_memkb = strtoul(target, &endptr, 10);
+        if (*endptr != '\0') {
+            XL_LOG_ERRNO(ctx, XL_LOG_ERROR,
+                    "invalid memory target %s from %s/memory/target\n", target, dompath);
+            goto out;
+        }
+    }
+    *out_target = target_memkb;
+    rc = 0;
 
 out:
     libxl_free_all(&gc);
diff -r 990ff10b7b00 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Wed Aug 25 20:59:35 2010 +0100
+++ b/tools/libxl/libxl.h	Wed Aug 25 21:01:36 2010 +0100
@@ -322,6 +322,8 @@ int libxl_domain_core_dump(libxl_ctx *ct
 
 int libxl_domain_setmaxmem(libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb);
 int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t target_memkb, int enforce);
+int libxl_set_relative_memory_target(libxl_ctx *ctx, uint32_t domid, int32_t relative_target_memkb, int enforce);
+int libxl_get_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t *out_target);
 
 int libxl_vncviewer_exec(libxl_ctx *ctx, uint32_t domid, int autopass);
 int libxl_console_exec(libxl_ctx *ctx, uint32_t domid, int cons_num, libxl_console_constype type);

Re: [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target

[Ian Jackson]

Stefano Stabellini writes ("[Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target"):
> libxl: introduce libxl_set_relative_memory_target
> 
> Introduce libxl_set_relative_memory_target to modify the memory target
> of a domain by a relative amount of memory in a single xenstore
> transaction.
> Modify libxl_set_memory_target to use xenstore transactions.
> The first time we are reading/writing dom0 memory target, fill the
> informations in xenstore if they are missing.

>  int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid,
>                              uint32_t target_memkb, int enforce)

See my earlier comments about memory targets.  I don't think it makes
much sense to give a domain a memory target and then let it exceed it.
So I think "enforce" should be abolished (as if it were always set).

Also please can you try to keep your code to <75ish columns ? :-)
(75 because there should be room for > and + quoting without wrap
damage.)

>  int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid,
>                 uint32_t target_memkb, int enforce)
...
> +int libxl_set_relative_memory_target(libxl_ctx *ctx, uint32_t
> +            domid, int32_t relative_target_memkb, int enforce)

These functions are really rather too similar for my taste.  They
seem to differ only in whether they read the existing target and add
it on.  Surely they should be combined.

Also, I don't really think this patch to introuce the relative setting
function should involves adding a lot of code to the absolute setting
function.  It's a shame that we have to set so many different copies
of the same value, but if we do then that should be done in a separate
patch first perhaps ?

Ian.

Re: [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target

[Stefano Stabellini]

On Tue, 31 Aug 2010, Ian Jackson wrote:
> Stefano Stabellini writes ("[Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target"):
> > libxl: introduce libxl_set_relative_memory_target
> > 
> > Introduce libxl_set_relative_memory_target to modify the memory target
> > of a domain by a relative amount of memory in a single xenstore
> > transaction.
> > Modify libxl_set_memory_target to use xenstore transactions.
> > The first time we are reading/writing dom0 memory target, fill the
> > informations in xenstore if they are missing.
> 
> >  int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid,
> >                              uint32_t target_memkb, int enforce)
> 
> See my earlier comments about memory targets.  I don't think it makes
> much sense to give a domain a memory target and then let it exceed it.
> So I think "enforce" should be abolished (as if it were always set).
> 

I can do that.


> Also please can you try to keep your code to <75ish columns ? :-)
> (75 because there should be room for > and + quoting without wrap
> damage.)
> 

Yes. We need a CODING_STILE, I'll post a patch with it later on.


> >  int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid,
> >                 uint32_t target_memkb, int enforce)
> ...
> > +int libxl_set_relative_memory_target(libxl_ctx *ctx, uint32_t
> > +            domid, int32_t relative_target_memkb, int enforce)
> 
> These functions are really rather too similar for my taste.  They
> seem to differ only in whether they read the existing target and add
> it on.  Surely they should be combined.
> 
> Also, I don't really think this patch to introuce the relative setting
> function should involves adding a lot of code to the absolute setting
> function.  It's a shame that we have to set so many different copies
> of the same value, but if we do then that should be done in a separate
> patch first perhaps ?
> 

The separate patch is a good idea, but merging the two functions
together will result in code harder to read in the implementation of a
very important function.

Re: [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target

[Jeremy Fitzhardinge]

 On 08/31/2010 10:25 AM, Ian Jackson wrote:
> Stefano Stabellini writes ("[Xen-devel] [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target"):
>> libxl: introduce libxl_set_relative_memory_target
>>
>> Introduce libxl_set_relative_memory_target to modify the memory target
>> of a domain by a relative amount of memory in a single xenstore
>> transaction.
>> Modify libxl_set_memory_target to use xenstore transactions.
>> The first time we are reading/writing dom0 memory target, fill the
>> informations in xenstore if they are missing.
>>  int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid,
>>                              uint32_t target_memkb, int enforce)
> See my earlier comments about memory targets.  I don't think it makes
> much sense to give a domain a memory target and then let it exceed it.
> So I think "enforce" should be abolished (as if it were always set).

There needs to be a way to set the maxmem for a domain without also
setting the memory/target xenstore key, so that you can allow a domain
to control its own ballooning up to a certain limit without also
triggering its xenstore watch (which would trigger an external balloon
target request).

Ideally there should also be a xenstore key that allows a guest to
determine what its max is, rather than just feeling for it until it gets
hypercall failures.

In general, guests can treat "memory/target" as a host recommendation of
what size it should ideally be if it is being as cooperative as
possible.  A simple implementation - like the current balloon driver -
just treats it literally.  But there's no reason why that's the best
thing to do.  (If there is some kind of cost model where there's an
active incentive for a domain to reduce its own memory usage it would
change the landscape a lot.)

By extension, there's no reason why the guest must be limited by
"target" - its possible that its ideal size, based on its own internal
memory pressure - is larger than target.  It can stay larger than target
if it is shrinking by stopping before it hits target, but I don't see an
inherent reason why the toolstack might not want to suggest a particular
target but allow a domain to grow beyond that.

There's also the question of how tmem makes use of maxmem - I believe it
allows a domain to use tmem pages up to its maxmem, without actually
changing the current memory usage in terms of pages mapped into the domain.

If libxl is supposed to be the general-purpose base of a toolstack, I
don't think it should be trying to make these kinds of policy decisions
("target == max") internally - or implicitly as part of the API design.

In this case, I think it would be most sensible to have:

    int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t
    target_memkb)

(no enforce parameter) which simply sets the target xenstore key
(nothing else), and

    int libxl_set_memory_limit(libxl_ctx *ctx, uint32_t domid, uint32_t
    target_memkb)

which sets the domain's Xen-enforced max size (and maybe a xenstore key
so that the domain can tell that its max has been changed and what to).

And that's it.  The target and limit are completely orthogonal concepts,
and there definitely should not be any implementation of rules like
"target must be smaller than limit".

If a toolstack wants to implement more mechanism/policy on top of this,
it is free to do so.  But libxl should just provide plain basics.

    J

RE: [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target

[Dan Magenheimer]

Thanks for cc'ing me Jeremy... I saw the posting but missed
the implications (thinking it just a vanilla reimplementation of
"xm memset").

> There needs to be a way to set the maxmem for a domain without also
> setting the memory/target xenstore key, so that you can allow a domain
> to control its own ballooning up to a certain limit without also
> triggering its xenstore watch (which would trigger an external balloon
> target request).

Indeed, that's what selfballooning does.  The xenstore watch
is irrelevant for selfballooning (though the watch also can be used
asynchronously for backwards compatibility).

IMHO, attempts to do memory load balancing externally (e.g. setting
a memory target from tools in dom0) are doomed to failure.  There 
was a discussion of memory "rightsizing" at the recent Linux MM summit;
this is an almost impossible problem even within a single kernel,
though there were heuristics discussed as to how to approach it...
and a better understanding about why in-kernel tmem-ish functionalities
like cleancache and frontswap are useful for mitigating the problems
that occur when rightsizing is approximated.

> There's also the question of how tmem makes use of maxmem - I believe
> it
> allows a domain to use tmem pages up to its maxmem, without actually
> changing the current memory usage in terms of pages mapped into the
> domain.

There are two classes of tmem memory: ephemeral and persistent.
Ephemeral is clean pages that can be discarded at any time and
tmem doesn't count those against maxmem.  Persistent pages
must be retained until the guest that "owns" them flushes
them or dies; these pages DO count against the guest's maxmem
but are not mapped into the domain.

So, frankly, I think the "xm memset" functionality is largely
useless, but agree that it should be maintained in xl for backwards
compatibility.  But trying to comingle the concepts of maxmem
and target is a bad idea.

Dan

Re: [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target

[Jeremy Fitzhardinge]

 On 09/01/2010 01:03 PM, Dan Magenheimer wrote:
> Indeed, that's what selfballooning does.  The xenstore watch
> is irrelevant for selfballooning (though the watch also can be used
> asynchronously for backwards compatibility).

There's no mechanism to make the balloon driver ignore the target watch,
so any updates to xenstore will update the driver's target.

> IMHO, attempts to do memory load balancing externally (e.g. setting
> a memory target from tools in dom0) are doomed to failure.  There 
> was a discussion of memory "rightsizing" at the recent Linux MM summit;
> this is an almost impossible problem even within a single kernel,
> though there were heuristics discussed as to how to approach it...
> and a better understanding about why in-kernel tmem-ish functionalities
> like cleancache and frontswap are useful for mitigating the problems
> that occur when rightsizing is approximated.
> [...]
> So, frankly, I think the "xm memset" functionality is largely
> useless, but agree that it should be maintained in xl for backwards
> compatibility.  But trying to comingle the concepts of maxmem
> and target is a bad idea.

In the general case I think you're probably right (I can't see it being
useful in a VPS hosting service, for example), but there are definitely
special cases where it is useful.  Squashing down existing domains to
make room for a new one, for example, or more app-specific uses.

Giving domains some real incentive to be economical with memory would
probably change the landscape a lot.  But I don't think there's a real
solution without knowing the specifics of that incentive.

    J

RE: [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target

[Dan Magenheimer]

> From: Jeremy Fitzhardinge [mailto:jeremy@goop.org]
> 
>  On 09/01/2010 01:03 PM, Dan Magenheimer wrote:
> > Indeed, that's what selfballooning does.  The xenstore watch
> > is irrelevant for selfballooning (though the watch also can be used
> > asynchronously for backwards compatibility).
> 
> There's no mechanism to make the balloon driver ignore the target
> watch,
> so any updates to xenstore will update the driver's target.

The selfballooning patch currently applies to the balloon driver
so it could easily disable the target watch, though it does not.
 
> > So, frankly, I think the "xm memset" functionality is largely
> > useless, but agree that it should be maintained in xl for backwards
> > compatibility.  But trying to comingle the concepts of maxmem
> > and target is a bad idea.
> 
> In the general case I think you're probably right (I can't see it being
> useful in a VPS hosting service, for example), but there are definitely
> special cases where it is useful.  Squashing down existing domains to
> make room for a new one, for example, or more app-specific uses.

Agreed in general, though I suspect sysadmins would be rather
peeved if/when a simple xm command in dom0 causes kernel OOMs
and application kills... or, worse, guest kernel crashes (which are
circumvented by minimum-target code in the linux-xen balloon
driver but NOT in the pvops balloon driver).

So "useless" is an overstatement, but "must be used extremely
carefully with knowledge of current activity in the guest
and/or willingness for the targeted guest or its applications
to die for a greater cause" is not an overstatement.
 
> Giving domains some real incentive to be economical with memory would
> probably change the landscape a lot.  But I don't think there's a real
> solution without knowing the specifics of that incentive.

Agreed.  Lacking a clear incentive though, reducing the disincentive
is a reasonable approach... which is what tmem+selfballooning does.

My long term view of the incentive is something like a VPS hoster
that offers service for $10/month, but offers it for $5/month
if using a tmem(+selfballooning)-enabled kernel.  This is essentially
like the electric utility companies that give customers a discount
if the customer allows them a remote-kill switch to turn off your air
conditioning if system-wide conditions warrant.

Dan

Re: [PATCH 2 of 8] libxl: introduce libxl_set_relative_memory_target

[Stefano Stabellini]

On Wed, 1 Sep 2010, Jeremy Fitzhardinge wrote:
> There needs to be a way to set the maxmem for a domain without also
> setting the memory/target xenstore key, so that you can allow a domain
> to control its own ballooning up to a certain limit without also
> triggering its xenstore watch (which would trigger an external balloon
> target request).
> 
> Ideally there should also be a xenstore key that allows a guest to
> determine what its max is, rather than just feeling for it until it gets
> hypercall failures.
> 
> In general, guests can treat "memory/target" as a host recommendation of
> what size it should ideally be if it is being as cooperative as
> possible.  A simple implementation - like the current balloon driver -
> just treats it literally.  But there's no reason why that's the best
> thing to do.  (If there is some kind of cost model where there's an
> active incentive for a domain to reduce its own memory usage it would
> change the landscape a lot.)
> 
> By extension, there's no reason why the guest must be limited by
> "target" - its possible that its ideal size, based on its own internal
> memory pressure - is larger than target.  It can stay larger than target
> if it is shrinking by stopping before it hits target, but I don't see an
> inherent reason why the toolstack might not want to suggest a particular
> target but allow a domain to grow beyond that.
> 
> There's also the question of how tmem makes use of maxmem - I believe it
> allows a domain to use tmem pages up to its maxmem, without actually
> changing the current memory usage in terms of pages mapped into the domain.
> 
> If libxl is supposed to be the general-purpose base of a toolstack, I
> don't think it should be trying to make these kinds of policy decisions
> ("target == max") internally - or implicitly as part of the API design.
 

I actually agree with you on this point, but yesterday morning I
couldn't come up with any practical use case to support this argument.


> 
> In this case, I think it would be most sensible to have:
> 
>     int libxl_set_memory_target(libxl_ctx *ctx, uint32_t domid, uint32_t
>     target_memkb)
> 
> (no enforce parameter) which simply sets the target xenstore key
> (nothing else), and
> 
>     int libxl_set_memory_limit(libxl_ctx *ctx, uint32_t domid, uint32_t
>     target_memkb)
> 
> which sets the domain's Xen-enforced max size (and maybe a xenstore key
> so that the domain can tell that its max has been changed and what to).
> 

Considering that there is already a libxl function to set memmax
(libxl_domain_setmaxmem), I'll just keep libxl_set_memory_target as it
is because it might be very useful to set a new target and a new memmax
in a single transaction.