From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [PATCH v2 for-4.6] libxl_set_memory_target: retain the same maxmem offset on top of the current target Date: Tue, 02 Dec 2014 09:59:49 -0500 Message-ID: <547DD3E5.9090303@terremark.com> References: <547DBC05.9070904@terremark.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini , Don Slutz Cc: Ian Jackson , xen-devel@lists.xensource.com, "Wei Liu (Intern)" , Ian Campbell List-Id: xen-devel@lists.xenproject.org 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 >>> >>> --- >>> >>> 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; >>> + rc = xc_domain_setmaxmem(ctx->xch, domid, memorykb); >>> if (rc != 0) { >>> LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, >>> "xc_domain_setmaxmem domid=%d memkb=%d failed " >> You need to remove LIBXL_MAXMEM_CONSTANT here also. > I don't think so: LIBXL_MAXMEM_CONSTANT is supposed to be a safety > buffer and we should keep it as is across libxl_set_memory_target calls. > Arguably LIBXL_MAXMEM_CONSTANT could be removed entirely with the > proposed QEMU changes but that is a separate matter. > I was talking about the line: "rc=%d\n", domid, memorykb + LIBXL_MAXMEM_CONSTANT, rc); (which is missing from the diff but is part of the LIBXL__LOG_ERRNO call). The error message no longer matches what xc_domain_setmaxmem() was called with. -Don Slutz > >>> @@ -4800,8 +4802,6 @@ retry_transaction: >>> goto out; >>> } >>> - libxl_dominfo_init(&ptr); >>> - xcinfo2xlinfo(ctx, &info, &ptr); >>> uuid = libxl__uuid2string(gc, ptr.uuid); >>> libxl__xs_write(gc, t, libxl__sprintf(gc, "/vm/%s/memory", uuid), >>> "%"PRIu32, new_target_memkb / 1024);