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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 28 Jan 2015, Ian Campbell wrote:
> On Wed, 2015-01-28 at 14:35 +0000, Stefano Stabellini wrote:
> > On Wed, 28 Jan 2015, Ian Campbell wrote:
> > > On Mon, 2015-01-26 at 17:03 +0000, 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.
> > > 
> > > Did we apply that patch for 4.5? (should this be backported?)
> > 
> > No, we didn't. We decided to wait for the new dev cycle.
> 
> OK, so "The offset includes..." should really be "In the future the
> offset will include..."?

Yes, I'll change it.


> > > How is this change expected to interact with relative vs. absolute mode?
> > 
> > This change works well with both.
> 
> What I meant was what are the semantics of relative mode, it seems like
> that should require now change to the function?

Relative mode changes the memory target relative to the current target.
This patch leaves this behavior unchanged.

If enforce=1 is passed to the function, libxl_set_memory_target is
supposed to enforce the new limit setting maxmem accordingly. This
behavior is also unchanged, but the maxmem calculation is more
accurate.


> > > Does docs/misc/libxl_memory.txt not need an update to account for this
> > > change in behaviour?
> > 
> > No, because this patch doesn't change the memory layout: it only makes
> > sure that it stays the same when libxl_set_memory_target is called
> > after the guest has booted.
> 
> OK, perhaps the ROM file accounting patch needs that change then?

Yes, please don't apply the ROM file accounting patch for the moment.


> > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > > > 
> > > > ---
> > > > 
> > > > Changes in v2:
> > > > - remove LIBXL_MAXMEM_CONSTANT from LIBXL__LOG_ERRNO.
> > > 
> > > And from the setmaxmem call too from the looks of it, can the reason for
> > > that be explained in the commit log please.
> > 
> > I am not really removing LIBXL_MAXMEM_CONSTANT for maxmem: by setting
> > the new maxmem as a relative change to the current one,
> > LIBXL_MAXMEM_CONSTANT has already been included.
> 
> Ah right. please allude to that in the commit log.

OK


> > > > +    if (enforce && new_target_memkb > 0) {
> > > 
> > > How does this change in the condition relate to the change here?
> > 
> > This is just one more correctness fix. I should note into the commit
> > message.
> 
> Please.
> 
> Although, if someone asks to set RAM to 0 and enforce -- should we not
> do so? Or error, or something other than silently nothing.

I think you are right. I'll change it to log an error and return.

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