Re: [Xen-devel] [PATCH 7/9] xl: enable using ranges of pCPUs when creating cpupools

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On Mon, 2015-03-09 at 10:58 +0000, Wei Liu wrote:
> On Fri, Mar 06, 2015 at 06:21:51PM +0100, Dario Faggioli wrote:

> > ---
> >  docs/man/xlcpupool.cfg.pod.5 |   22 +++++++++++++++++++---
> >  tools/libxl/xl_cmdimpl.c     |   17 ++++++++++++++---
> >  2 files changed, 33 insertions(+), 6 deletions(-)
> > 
> > diff --git a/docs/man/xlcpupool.cfg.pod.5 b/docs/man/xlcpupool.cfg.pod.5
> > index bb15cbe..2ff8ee8 100644
> > --- a/docs/man/xlcpupool.cfg.pod.5
> > +++ b/docs/man/xlcpupool.cfg.pod.5
> > @@ -93,10 +93,26 @@ Specifies the cpus of the NUMA-nodes given in C<NODES> 
> > (an integer or
> >  a list of integers) to be member of the cpupool. The free cpus in the
> >  specified nodes are allocated in the new cpupool.
> >  
> > -=item B<cpus="CPUS">
> > +=item B<cpus="CPU-LIST">
> >  
> > -The specified C<CPUS> are allocated in the new cpupool. All cpus must
> > -be free. Must not be specified together with B<nodes>.
> > +Specifies the cpus that will be member of the cpupool. All the specified
> > +cpus must be free, or creation will fail. C<CPU-LIST> may be specified
> > +as follows:
> > +
> > +=over 4
> > +
> > +=item ["2", "3", "5"]
> > +
> > +means that cpus 2,3,5 will be member of the cpupool.
> > +
> 
> I suppose this is the old syntax?
> 
It is.

> I'm asking because I want to be sure we still support the old syntax.  I
> think we still support the old syntax from the look of the changes below
> but I'd better get confirmation from you.
> 
I also think we should, and, in fact, we do. :-)

> > +=item "0-3,5,^1"
> > +
> > +means that cpus 0,2,3 and 5 will be member of the cpupool. A "node:" or
> > +"nodes:" modifier can be used. E.g., "0,node:1,nodes:2-3,^10-13" means
> > +that pcpus 0, plus all the cpus of NUMA nodes 1,2,3 with the exception
> > +of cpus 10,11,12,13 will be memeber of the cpupool.
> > +
> > +=back
> >  
> >  If neither B<nodes> nor B<cpus> are specified only the first free cpu
> >  found will be allocated in the new cpupool.
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index c748ba0..e6d1234 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -7163,18 +7163,29 @@ int main_cpupoolcreate(int argc, char **argv)
> >              fprintf(stderr, "no free cpu found\n");
> >              goto out_cfg;
> >          }
> > -    } else if (!xlu_cfg_get_list(config, "cpus", &cpus, 0, 0)) {
> > +    } else if (!xlu_cfg_get_list(config, "cpus", &cpus, 0, 1)) {
> >          n_cpus = 0;
> >          while ((buf = xlu_cfg_get_listitem(cpus, n_cpus)) != NULL) {
> >              i = atoi(buf);
> > -            if ((i < 0) || (i >= freemap.size * 8) ||
> > -                !libxl_bitmap_test(&freemap, i)) {
> > +            if ((i < 0) || !libxl_bitmap_test(&freemap, i)) {
> >                  fprintf(stderr, "cpu %d illegal or not free\n", i);
> >                  goto out_cfg;
> >              }
> >              libxl_bitmap_set(&cpumap, i);
> >              n_cpus++;
> >          }
> > +    } else if (!xlu_cfg_get_string(config, "cpus", &buf, 0)) {
> > +        if (cpurange_parse(buf, &cpumap))
> 
> Perhaps print something to say the cpu range specified is invalid?
> 
It happens already inside cpurange_parse() (well, to be precise, it's
inside update_cpumap_range() ), which is in a better position for
printing the most meaningful error messages.

If you check any other call site of cpurange_parse(), none prints
anything on failure, exactly for that reason.

Regards,
Dario

Attachment: signature.asc
Description: This is a digitally signed message part

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