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

[Don Slutz <dslutz@xxxxxxxxxxx>]

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;
> > > +        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);

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