Re: [Xen-devel] [PATCH v7 21/21] xl: vNUMA support

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Mar 12, 2015 at 10:00:24AM +0000, Ian Campbell wrote:
> On Wed, 2015-03-11 at 20:14 +0000, Wei Liu wrote:
> > On Wed, Mar 11, 2015 at 03:12:06PM +0000, Ian Campbell wrote:
> > > On Mon, 2015-03-09 at 12:51 +0000, Wei Liu wrote:
> > > 
> > > > +Each B<VNODE_CONFIG_OPTION> is a quoted key=value pair. Supported
> > > > +B<VNODE_CONFIG_OPTION>s are:
> > > 
> > > Which of these are optional and which are mandatory? All mandatory?
> > > 
> > 
> > Yes, all mandatory for now.
> 
> Please can you say so.

No problem.

> > 
> > > > +    if (!xlu_cfg_get_long (config, "maxmem", &l, 0))
> > > > +        max_memkb1 = l * 1024;
> > > 
> > > I think you should arrange that if the user has specified maxmem then it
> > > has already been parsed and is available in b_info->max_memkb for use in
> > > this function. Reparsing it just leaves scope to get out of sync.
> > > 
> > > In particular this doesn't handle the defaulting of maxmem to memory.
> > > 
> > 
> > This is exactly the reason I do this here.  At this point
> > b_info->max_memkb is always set.
> > 
> > If user has specified maxmem we should check if the configuration he /
> > she specifies matches the vnuma value to avoid misconfiguration;
> > otherwise we just use vnuma memory as maxmem.
> > 
> > But see below regarding maxvcpus=.
> [...]
> > > > +    if (total_cpus != b_info->max_vcpus) {
> > > > +        fprintf(stderr, "xl: vnuma vcpus and maxvcpus= mismatch\n");
> > > > +        exit(1);
> > > > +    }
> > > 
> > > Can't we do as we do with memory here and set max_vcpus if it isn't
> > > already configured?
> > > 
> > 
> > This can be done.  It will require moving parse_vnuma_config to
> > beginning of parse_config_data and stubbing out parsing of maxvcpus
> > and maxmem if vnuma configuration presents.
> 
> I'm afraid I don't follow.
> 
> I was thinking the flow would be:
> 
>       Call libxl_foo_init to set everything to the default.
> 
>         f ( max_mem in cfg )
>                 cfg->maxmem = v
>         if ( memory in cfg )
>                 cfg->memory = v
>         if ( vcpus in cfg )
>                 cfg->vcpus = v
>                 
>         do numa parsing, using fact that cfg->{maxmem,memory,vcpus} are
>         either default value (in which case they are set here) or
>         explicitly set to some value, in which case they are sanity
>         checked.
> 
>         handle maxmem/memory/vcpus/etc still being default values by
>         setting the appropriate defaults.
> 

Now I get your idea. I think we are more or less talking about the same
thing. I was proposing:

        parse vcpus
        parse memory
        parse maxmem

        parse vnuma

        if maxmem is not set by either config option or vnuma, set to memory
        if maxvcpus is not set by either config option or vnuma, set to vcpus

Wei.

> Ian.
> 

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