Re: [Xen-devel] [PATCH 2/6] xl: Implement XENMEM_claim_pages support via 'claim_mode' global config

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-04-16 at 00:20 +0100, konrad wilk wrote:
> On 4/15/2013 5:34 AM, Ian Campbell wrote:
> > On Fri, 2013-04-12 at 19:03 +0100, Ian Jackson wrote:
> >> Ian Jackson writes ("Re: [PATCH 2/6] xl: Implement XENMEM_claim_pages 
> >> support via 'claim_mode' global config"):
> >>> Sorry, I just spotted this.  I think the libxl_defbool_setdefault
> >>> shouldn't be there.  The defbool should be initialised to "default",
> >>> which can be done by setting it to 0, as per:
> >>>
> >>>    * To allow users of the library to naively select all defaults this
> >>>    * state is represented as 0. False is < 0 and True is > 0.
> >>>
> >>> in libxl.h.  And since it's a variable of static duration the C
> >>> implementation will initialise it to 0.  So just deleting the
> >>> setdefault is right.
> >>>
> >>> The result is that the default is set in libxl, only.
> >> Konrad points out that without this, xl can't easily find out whether
> >> the claim mode is enabled or not.
> > Does it need to know? Is the presence of any non-zero value for a claim
> > enough indication for each function which might care to make a local
> > decision? At least nothing in this particular patch appears to care what
> > libxl's default is.
> 
> The issue was that if you try to do libxl_get_defbool and the bool is a 
> default - it will
> blow up with an assert.

My real question is who (outside of libxl) is doing that
libxl_get_defbool and why?

> > Is this setting supposed to be global (at either the host or specific
> > toolstack level) or is it supposed to be per-domain?
> Global

Hrm, this suggests that the approach here (which is inherently
per-domain) is wrong then? i.e. this part of my original mail applies:
> > If its supposed to be host wide then that seems to argue for a
> > requirement for a libxl specific configuration file, so that all
> > toolstacks (at least those which use libxl) can be configured. The xapi
> > guys were asking me about the possibility of such settings last week in
> > the context of host wide driver domain policy...

> > Anyway, back to the original point of this mail, assuming my questions
> > above haven't made that moot:

I think it has :-(

[...]
> >     xl.c:
> >     int claim_mode; /* = 0 */
> >
> >     xl.c:parse_global_config():
> >     if (!xlu_cfg_get_long (config, "claim_mode", &l, 0))
> >              claim_mode = l;
> >
> >     xl_cmdimpl.c:parse_config_data():
> >          libxl_defbool_set_default(&b_info->claim_mode, claim_mode)
> >
> > i.e. xl's glboal claim mode setting is just a bool, not a defbool.
> 
> Yes. That will work too. This was how the earlier versions had it.

Unfortunately this approach is only really valid if claim is per-domain,
if it is per host (i.e. global) as you suggest then the approach needs
to be entirely different AFAICT.

Ian.


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