Re: [Xen-devel] [libvirt] [PATCH RESENT 04/12] libxl: populate xenstore memory entries at startup\

[Marek Marczykowski <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

On 22.05.2013 18:58, Stefano Stabellini wrote:
> On Wed, 22 May 2013, Jim Fehlig wrote:
>> Marek Marczykowski wrote:
>>> On 19.04.2013 13:10, Stefano Stabellini wrote:
>>>   
>>>> On Thu, 11 Apr 2013, Marek Marczykowski wrote:
>>>>     
>>>>> On 11.04.2013 09:52, Ian Campbell wrote:
>>>>>       
>>>>>> On Thu, 2013-04-11 at 05:09 +0100, Jim Fehlig wrote:
>>>>>>         
>>>>>>>> +    /* This will fill xenstore info about free and dom0 memory - if 
>>>>>>>> missing,
>>>>>>>> +     * should be called before starting first domain */
>>>>>>>> +    if (libxl_get_free_memory(libxl_driver->ctx, &free_mem)) {
>>>>>>>> +        VIR_ERROR(_("cannot get free memory info"));
>>>>>>>> +        goto error;
>>>>>>>> +    }
>>>>>>>>   
>>>>>>>>             
>>>>>>> Should failure of libxl_get_free_memory() really be fatal and prevent
>>>>>>> the driver from loading?
>>>>>>>           
>>>>>> I'm not sure it is intended to be called like this...
>>>>>>
>>>>>> I think it is intended to be called as part of starting every domain, to
>>>>>> check if there is enough free memory for that domain, rather than
>>>>>> calling it once at start of day.
>>>>>>
>>>>>> In that context if it fails or returns less than the required amount of
>>>>>> memory then that would be fatal for starting that domain.
>>>>>>
>>>>>> In xl we use this as part of the auto balloon of dom0, see
>>>>>> xl_cmdimplg.c:freemem. Does libvirt do autoballooning or does it require
>>>>>> dom0_mem? Perhaps this is handled at a higher level?
>>>>>>         
>>>>> The problem is how libxl set initial value for freemem-slack. If, for any
>>>>> reason, dom0 hasn't (almost) all memory assigned before creating first 
>>>>> domain,
>>>>> 15% of host memory will no longer be used at all. This "any reason" can be
>>>>> dom0_mem, which is covered by "auto" value for autoballoon in xen-unstable
>>>>> (actually only for xl, not libxl in general).
>>>>>       
>>>> This is the (in)famous incompatibility between autoballoon and dom0_mem.
>>>>
>>>>
>>>>     
>>>>> But this can also happen if
>>>>> somebody calls xl set-mem 0 <some value>. The later case doesn't mean the 
>>>>> user
>>>>> want to disable autoballoon completely.
>>>>>       
>>>> Calling "xl set-mem 0" manually should be compatible with
>>>> autoballoon. 
>>>>     
>>>
>>> Unless it is called before first domain start (which case is covered by my 
>>> patch).
>>>
>>>   
>>>> However it wouldn't work with dom0_mem.
>>>>
>>>>
>>>>     
>>>>> And to answer you question - libvirt rely on libxl autoballoon.
>>>>>       
>>>> Could we introduce something similar to autoballoon=auto to libvirt?
>>>>
>>>> Maybe we should push down the autoballoon option to libxl: we should
>>>> probably rename autoballoon to libxl_memory_management, enable it
>>>> automatically during initialization (and call
>>>> libxl__fill_dom0_memory_info) unless dom0_mem has been passed as an
>>>> option, in which case we would disable it. If we do it at the libxl
>>>> level we would cover xl as well as libvirt and also fix the "xl set-mem
>>>> 0" case.
>>>>     
>>>
>>> Looks good for me, but IIUC it's too late for such change in 4.3 and it
>>> doesn't qualify for later backport, right? If so, some approach still is
>>> needed in libvirt for xen 4.2 and 4.3 (like my simple patch). This still
>>> doesn't make libvirt compatible with dom0_mem, but at least cover one
>>> (independent) case. Perhaps additionally autoballoon=auto code should be
>>> copied from xl to libvirt to cover dom0_mem case with xen 4.2 and 4.3?
>>>   
>>
>> After reading this thread again, it sounds like the best option is to
>> support the autoballoon option in libvirt, e.g. in
>> /etc/libvirt/libxl.conf.  (We currently don't have a config file for the
>> libxl driver, but I think we'll need one anyhow for other knobs, similar
>> to qemu.conf.)   As you note, even if autoballoon is pushed down to
>> libxl, libvirt would need to handle it for older libxl.  And libvirt
>> needs to handle the dom0_mem case...
>  
> 
> Given all the troubles that we had with the autoballoon option in
> xl/libxl and how it clashes with dom0_mem, I would strongly advise to
> consider implementing something like autoballoon=auto from the start.

Ok, will send such patch (almost ready). For now it will contain copy&paste
auto_autobaloon() from xl.c, so the same logic. Can be easily extended to
config option in the future, but probably I will not have time for this.

-- 
Best Regards,
Marek Marczykowski
Invisible Things Lab

Attachment: signature.asc
Description: OpenPGP digital signature

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