Re: [Xen-devel] [PATCH 0/2] xl: Add subcommand mem-max and fix mem-set

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 12 May 2010, Jonathan Knowles wrote:
> 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.


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@xxxxxxxxxxxxx>
Acked-by: Jonathan Knowles <Jonathan.Knowles@xxxxxxxxxxxxx> 

---


diff -r 9d53864d7be6 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Thu May 13 12:21:16 2010 +0100
+++ b/tools/libxl/libxl.c       Thu May 13 13:17:51 2010 +0100
@@ -2468,7 +2468,6 @@
     char *mem, *endptr;
     uint32_t memorykb;
     char *dompath = libxl_xs_get_dompath(ctx, domid);
-    int rc;
 
     mem = libxl_xs_read(ctx, XBT_NULL, libxl_sprintf(ctx, "%s/memory/target", 
dompath));
     if (!mem) {
@@ -2486,20 +2485,16 @@
         return 1;
     }
 
-    rc = xc_domain_setmaxmem(ctx->xch, domid, max_memkb);
-    if (rc != 0)
-        return rc;
-
     if (domid != 0)
         libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, 
"%s/memory/static-max", dompath), "%lu", max_memkb);
 
-    return rc;
+    return 0;
 }
 
-int libxl_set_memory_target(struct libxl_ctx *ctx, uint32_t domid, uint32_t 
target_memkb)
+int libxl_set_memory_target(struct libxl_ctx *ctx, uint32_t domid, uint32_t 
target_memkb, int enforce)
 {
     int rc = 0;
-    uint32_t memorykb, videoram;
+    uint32_t memorykb = 0, videoram = 0;
     char *memmax, *endptr, *videoram_s = NULL;
     char *dompath = libxl_xs_get_dompath(ctx, domid);
     xc_domaininfo_t info;
@@ -2539,6 +2533,11 @@
     uuid = libxl_uuid2string(ctx, ptr.uuid);
     libxl_xs_write(ctx, XBT_NULL, libxl_sprintf(ctx, "/vm/%s/memory", uuid), 
"%lu", target_memkb / 1024);
 
+    if (enforce || !domid)
+        memorykb = target_memkb;
+    rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb + 
LIBXL_MAXMEM_CONSTANT);
+    if (rc != 0)
+        return rc;
     rc = xc_domain_memory_set_pod_target(ctx->xch, domid, (target_memkb - 
videoram) / 4, NULL, NULL, NULL);
     return rc;
 }
diff -r 9d53864d7be6 tools/libxl/libxl.h
--- a/tools/libxl/libxl.h       Thu May 13 12:21:16 2010 +0100
+++ b/tools/libxl/libxl.h       Thu May 13 13:17:51 2010 +0100
@@ -340,7 +340,7 @@
 int libxl_domain_unpause(struct libxl_ctx *ctx, uint32_t domid);
 
 int libxl_domain_setmaxmem(struct libxl_ctx *ctx, uint32_t domid, uint32_t 
target_memkb);
-int libxl_set_memory_target(struct libxl_ctx *ctx, uint32_t domid, uint32_t 
target_memkb);
+int libxl_set_memory_target(struct libxl_ctx *ctx, uint32_t domid, uint32_t 
target_memkb, int enforce);
 
 int libxl_console_attach(struct libxl_ctx *ctx, uint32_t domid, int cons_num);
 
diff -r 9d53864d7be6 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Thu May 13 12:21:16 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c  Thu May 13 13:17:51 2010 +0100
@@ -1262,7 +1262,7 @@
         exit(3);
     }
     printf("setting domid %d memory to : %d\n", domid, memorykb);
-    libxl_set_memory_target(&ctx, domid, memorykb);
+    libxl_set_memory_target(&ctx, domid, memorykb, /* enforce */ 1);
 }
 
 int main_memset(int argc, char **argv)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel