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

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

[Jim Fehlig]

Marek Marczykowski wrote:
> libxl uses some xenstore entries for hints in memory management
> (especially when starting new domain). This includes dom0 memory limit
> and Xen free memory margin, based on current system state. Entries are
> created at first usage, so force such usage at daemon startup, which most
> likely will be before any domain startup.
>   

Hmm, I'd like to get the xen developer's opinion on this change. 
Perhaps Ian C. or Ian J. have some comment...

> ---
>  src/libxl/libxl_driver.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 011edf8..89546a5 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -1115,6 +1115,7 @@ libxlStartup(bool privileged,
>      char *log_file = NULL;
>      virCommandPtr cmd;
>      int status, ret = 0;
> +    unsigned int free_mem;
>   

uint32_t to match libxl_get_free_memory() definition?

>      char ebuf[1024];
>  
>      /* Disable libxl driver if non-root */
> @@ -1240,6 +1241,13 @@ libxlStartup(bool privileged,
>          goto error;
>      }
>  
> +    /* 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?

Regards,
Jim

> +
>      if (!(libxl_driver->xmlconf = virDomainXMLConfNew(&libxlDomainXMLPrivateDataCallbacks,
>                                                        NULL)))
>          goto error;
>   

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

[Ian Campbell]

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?

Ian.



> 
> Regards,
> Jim
> 
> > +
> >      if (!(libxl_driver->xmlconf = virDomainXMLConfNew(&libxlDomainXMLPrivateDataCallbacks,
> >                                                        NULL)))
> >          goto error;
> >   
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

[Marek Marczykowski]

[-- Attachment #1.1: Type: text/plain, Size: 1759 bytes --]

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). 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.

And to answer you question - libvirt rely on libxl autoballoon.

-- 
Best Regards / Pozdrawiam,
Marek Marczykowski
Invisible Things Lab


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 553 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Ian Campbell]

On Thu, 2013-04-11 at 11:53 +0100, 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 is probably one for Stefano who understands this aspect best, I
think. (He's back next week, so this may need to wait). IanJ also knows
this stuff better than I, I've added both of them to the CC.

Note that whatever problem you are envisaging is also true if /creating
first domain/starting libvirtd/ (or whatever the relevant daemon is
called).

>  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). 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.
> 
> And to answer you question - libvirt rely on libxl autoballoon.

AIUI autoballoon and dom0_mem (and I think mem-set at start of day too)
are mutually exclusive, which means that as it stands the libvirt
backend is incompatible with dom0_mem -- obviously this is something
which should be addressed. I don't think this patch is the right way to
do that but I'm not sure what is, hopefully Stefano or IanJ have some
ideas...

Ian.

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

[Stefano Stabellini]

On Thu, 11 Apr 2013, 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.

Actually libxl_get_free_memory is just meant to return to total amount
of free memory in the system. Can be called at any time.
As a side effect of libxl_get_free_memory, freemem-slack will be
written to xenstore if it wasn't before.

I think that it is OK to call it as Marek did in this patch, even though
might be nicer if we had a proper "libxl_initialize_memory_accounting"
function.

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

[Stefano Stabellini]

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

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

[Marek Marczykowski]

[-- Attachment #1.1: Type: text/plain, Size: 3180 bytes --]

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?

-- 
Best Regards / Pozdrawiam,
Marek Marczykowski
Invisible Things Lab


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 553 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Stefano Stabellini]

On Fri, 19 Apr 2013, Marek Marczykowski wrote:
> >> 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?

Might be a decent solution for the moment.

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

[Jim Fehlig]

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

Regards,
Jim

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

[Stefano Stabellini]

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.

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

[Marek Marczykowski]

[-- Attachment #1.1: Type: text/plain, Size: 4583 bytes --]

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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 555 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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