Re: [Xen-devel] [PATCH] libxl: fix "xl mem-set" regression from 0c029c4da2

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.04.15 at 19:55, <Ian.Campbell@xxxxxxxxxx> wrote:
> On Wed, 2015-04-22 at 17:33 +0100, Jan Beulich wrote:
>> >>> On 22.04.15 at 17:36, <ian.campbell@xxxxxxxxxx> wrote:
>> > On Wed, 2015-04-22 at 15:41 +0100, Jan Beulich wrote:
>> >> >>> On 22.04.15 at 16:01, <ian.campbell@xxxxxxxxxx> wrote:
>> >> > On Wed, 2015-04-22 at 13:02 +0100, Jan Beulich wrote:
>> >> >> Said commit ("libxl_set_memory_target: retain the same maxmem offset on
>> >> >> top of the current target") caused a regression for "xl mem-set"
>> >> >> against Dom0: While prior to creation of the first domain this works,
>> >> >> the first domain creation involving ballooning breaks. Due to "enforce"
>> >> >> not being set in the domain creation case, and due to Dom0's initial
>> >> >> ->max_pages (in the hypervisor) being UINT_MAX, the calculation of
>> >> >> "memorykb" in the first "xl mem-set" adusting the target upwards
>> >> >> subsequent to domain creation and termination may cause an overflow,
>> >> >> resulting in Dom0's maximum getting to a very small value. This small
>> >> >> maximum will the make the subsequent setting of the PoD target fail.
>> >> >> 
>> >> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> >> >> ---
>> >> >> Note that this only fixes the immediate problem - there appear to be
>> >> >> further issues lurking here:
>> >> >> - libxl_set_memory_target()'s *_memkb variables all being 32-bit,
>> >> >> - libxl_domain_setmaxmem()'s max_memkb parameter being 32-bit,
>> >> > 
>> >> > I think that increasing the width of these variables wouldn't break the
>> >> > API guarantee which we make, at least not in a practical way, since any
>> >> > existing 32-bit arguments passed will just get promoted.
>> >> 
>> >> No, not even on 64-bit. On 32-bit, two arguments slots are needed
>> >> for what so far requires only one. On 64-bit (at least x86-64), the
>> >> calling code isn't required to zero-extend a value calculated in a
>> >> register (e.g. a result of earlier calculations which had more than
>> >> 32 significant bits could be passed unchanged to the called function);
>> >> it just so happens that 32-bit arithmetic on registers would always
>> >> implicitly zero the upper halves (and iirc that's the same on ARM64).
>> > 
>> > You seem to be talking about ABI? As I tried to note in my response for
>> > libxl we only make guarantees about the API (P not B in the middle).
>> 
>> Oh, okay. That would mean the libxl side is fine. The change to the
>> libxc interface might however still be a problem for qemu (Stefano
>> was telling me something to the effect of a compatibility layer in
>> upstream qemu to deal with such version differences, but of course
>> the implication - without versioned libxc interfaces - would still be
>> that existing qemu binaries on top of updated libxc would no
>> longer work).
> 
> Correct, but we would surely bump the SONAME for a change such as this,
> so it wouldn't matter in practice.

So what's the situation with this patch? Can it go in? Is someone
working on a better fix for the described problem?

Thanks, Jan


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