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.stabell...@eu.citrix.com> --- 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@lists.xen.org
http://lists.xen.org/xen-devel
