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

[Ian Campbell <Ian.Campbell@citrix.com>]

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..."?

> > 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?

> > 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?

> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> > > 
> > > ---
> > > 
> > > 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.
> > > +    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.

Ian.