[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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




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.
Is this setting supposed to be global (at either the host or specific
toolstack level) or is it supposed to be per-domain?
Global

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.

Yes. That will work too. This was how the earlier versions had it.

I will post a new version when I back in office tomorrow.

Thanks!

Ian.



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.