Re: [PATCH for-4.6 2/3] xl: fix vNUMA vcpus parsing

[Ian Campbell <ian.campbell@citrix.com>]

On Thu, 2015-08-13 at 10:54 +0100, Wei Liu wrote:
> On Thu, Aug 13, 2015 at 10:42:26AM +0100, Ian Campbell wrote:
> > On Wed, 2015-08-12 at 20:36 +0100, Wei Liu wrote:
> > > Originally, if user didn't specify maxvcpus= in xl config file, the
> > > maximum size of vcpu bitmap was always equal to maximum number of 
> > > pcpus.
> > > This might not be what user wants.
> > 
> > What are you suggesting they wanted instead? We are only talking about 
> > the
> > bitmap right, and the typical/sensible config will have #vcpus <= 
> > #pcpus,
> > so they will fit even if they "waste" some bits during parsing.
> 
> #vcpus > #pcpus, bitmap is too small.

Right. Which is trivial to detect as we go through the parsing and raise an
appropriate error.

> > I'm almost inclined to suggest that if a user wants #vcpus > #pcpus 
> > they
> > should have to specify maxvcpus and not rely on the vnuma parsing code
> > inferring this fact.
> > 
> 
> I don't think we should prevent people from shooting themselves in the
> foot.

If the cost of supporting that is this patch then I disagree. If you can
find a way to do it simply and cleanly then fine, maybe.

In fact I even disagree in general, we can and should provide warnings or
errors for things which we know are bad and which are most likely
unintentional, but provide overrides.

> > IOW maybe this code could just error out (or print a warning) if this
> > happens? + a doc update.
> > 
> 
> Xl doesn't complain when you set vcpus > pcpus. I don't think vNUMA
> should behave differently.

Not always true, e.g. from vcpuset:

       if (max_vcpus > dominfo.vcpu_online && max_vcpus > host_cpu) {
            fprintf(stderr, "You are overcommmitting! You have %d physical" \
                    " CPUs and want %d vCPUs! Aborting, use --ignore-host to" \
                    " continue\n", host_cpu, max_vcpus);
            rc = 1;
        }

Ian.