[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 for-4.6] libxl_set_memory_target: retain the same maxmem offset on top of the current target



On 12/03/14 12:31, Stefano Stabellini wrote:
On Tue, 2 Dec 2014, Don Slutz wrote:
On 12/02/14 09:59, Don Slutz wrote:
On 12/02/14 09:26, Stefano Stabellini wrote:
On Tue, 2 Dec 2014, Don Slutz wrote:
On 12/02/14 06:53, Stefano Stabellini wrote:
In libxl_set_memory_target when setting the new maxmem, retain the
same
offset on top of the current target. The offset includes memory
allocated by QEMU for rom files.

Signed-off-by: Stefano Stabellini<stefano.stabellini@xxxxxxxxxxxxx>

---

Changes in v2:
- call libxl_domain_info instead of libxl_dominfo_init;
- call libxl_domain_info before retry_transaction.

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index de23fec..569a32a 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4694,6 +4694,9 @@ int libxl_set_memory_target(libxl_ctx *ctx,
uint32_t
domid,
        char *uuid;
        xs_transaction_t t;
    +    if (libxl_domain_info(ctx, &ptr, domid) < 0)
+        goto out_no_transaction;
+
    retry_transaction:
        t = xs_transaction_start(ctx->xsh);
    @@ -4767,10 +4770,9 @@ retry_transaction:
                    "%s/memory/videoram", dompath));
        videoram = videoram_s ? atoi(videoram_s) : 0;
    -    if (enforce) {
-        memorykb = new_target_memkb;
-        rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb +
-                LIBXL_MAXMEM_CONSTANT);
+    if (enforce && new_target_memkb > 0) {
+        memorykb = ptr.max_memkb - current_target_memkb +
new_target_memkb;
My testing shows that this should be:

         memorykb = ptr.max_memkb - (current_target_memkb + videoram) +
             new_target_memkb;

As far as I can tell the reason for this is that memory/target (aka
current_target_memkb) was set based on:

     new_target_memkb -= videoram;
Thank you very much for testing and the suggestion!

I think that the right fix for this is to remove videoram from
new_target_memkb earlier and only when the new target is absolute,
otherwise we risk removing videoram twice (in case the new target is
relative). I wonder why we didn't notice this before.

Sounds like a good idea. No clue, I have been looking real close at this stuff.
and that my be why I tripped over it.


diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index d5d5204..4803cc4 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4744,13 +4744,17 @@ retry_transaction:
          goto out;
      }
+ videoram_s = libxl__xs_read(gc, t, libxl__sprintf(gc,
+                "%s/memory/videoram", dompath));
+    videoram = videoram_s ? atoi(videoram_s) : 0;
+
      if (relative) {
          if (target_memkb < 0 && abs(target_memkb) > current_target_memkb)
              new_target_memkb = 0;
          else
              new_target_memkb = current_target_memkb + target_memkb;
      } else
-        new_target_memkb = target_memkb;
+        new_target_memkb = target_memkb - videoram;
      if (new_target_memkb > memorykb) {
          LIBXL__LOG(ctx, LIBXL__LOG_ERROR,
                  "memory_dynamic_max must be less than or equal to"
@@ -4766,9 +4770,6 @@ retry_transaction:
          abort_transaction = 1;
          goto out;
      }
-    videoram_s = libxl__xs_read(gc, t, libxl__sprintf(gc,
-                "%s/memory/videoram", dompath));
-    videoram = videoram_s ? atoi(videoram_s) : 0;
if (enforce && new_target_memkb > 0) {
          memorykb = ptr.max_memkb - current_target_memkb + new_target_memkb;
@@ -4782,7 +4783,6 @@ retry_transaction:
          }
      }
- new_target_memkb -= videoram;
      rc = xc_domain_set_pod_target(ctx->xch, domid,
              new_target_memkb / 4, NULL, NULL, NULL);
      if (rc != 0) {

This does look like a bugfix for just the videoram issue.  Not sure why
you made this v2 since I do not see the original change.

Anyway if you want to post this change for 4.5 (?) I would be happy to review it.
    -Don Slutz

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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.