On Tue, 2015-02-24 at 16:31 +0000, Wei Liu wrote:
> On Tue, Feb 24, 2015 at 04:19:02PM +0000, Dario Faggioli wrote:

> > > +                } else if (!strcmp("size", option)) {
> > > +                    val = strtoul(value, &endptr, 10);
> > > +                    ABORT_IF_FAILED(value);
> > > +                    p->memkb = val << 10;
> > > +                } else if (!strcmp("vcpus", option)) {
> > > +                    libxl_string_list cpu_spec_list;
> > > +                    int cpu;
> > > +                    unsigned long s, e;
> > > +
> > > +                    split_string_into_string_list(value, ",", &cpu_spec_list);
> > > +                    len = libxl_string_list_length(&cpu_spec_list);
> > > +
> > > +                    for (j = 0; j < len; j++) {
> > > +                        parse_range(cpu_spec_list[j], &s, &e);
> > > +                        for (cpu = s; cpu <=e; cpu++)
> > > +                            libxl_bitmap_set(&p->vcpus, cpu);
> > > +                    }
> > > +                    libxl_string_list_dispose(&cpu_spec_list);
> > >
> > I think that using vcpupin_parse() for "vcpus=" would allow for more
> > flexible syntax (i.e., things like "3-8,^5"), and save some code. The
> > only downside is that it also accepts things like "nodes:1", which we
> > clearly don't want in here... is that why you are not going for it?
> > 
> 
> Yes. I don't want "nodes" so I didn't reuse that function, and at that
> point I didn't think it's critical to support "^X".
> 
Ok, I just wanted to be sure you were aware of the possibility. I
actually agree that supporting "^x" is not that critical here.

> If you think this "^X" syntax is important, I can check for "nodes"
> before calling vcpupin_parse.
> 
I don't think it is... TBH, I'm more attracted by the code being
potentially simpler, and less duplicate parsing logic being around, but
I appreciate that having to check for "node[s]" not being present up
front would make things look clumsy... so I'm leaving this to you, I'm
happy either way.

> vcpus_parse? It's not restricted to vcpu pinning in any way, I think.
> 
If you go for it, yes, I like this as a name.

Regards,
Dario