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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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.

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

If it is at the toolstack level then shouldn't it be set in the
libxl_ctx instead of libxl_domain_build_info? If it is per-domain then
shouldn't there be the possibility of an override in the domain config?

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:

> Options are:
> 
> 1. Leave it as it is, default set in libxl but overridden by
>    separate setting in xl (perhaps we should add a comment).
>    Tolerable.
> 
> 2. Move the default setting out of libxl entirely, so callers
>    must pass 0 or 1.  (I don't approve of this; we might want
>    to change the behaviour of naive toolstacks in the future.)

Agree that this is best avoided.

> 3. Provide a new interface to libxl which allows the claim
>    default to be retrieved.  Palaver.

Could incorporate it into whichever of the libxl_*info interfaces seems
most appropriate. libxl_physinfo contains a lot of the associated free
memory values, so although claim mode isn't really "phys" perhaps that's
the best place for it?

> 4. Have xl operations which need to know the default claim mode
>    set up a dummy domain creation config, ask libxl to set the
>    defaults in it, and then read out the value.  Very ugly.

Very.

> Of these I prefer 1.  Opinions ?  Whatever we do needs to be in 4.3.

1 is probably the best of the bunch but I note that in that case the
implementation in xl should be just:

   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.

Ian.


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