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

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.