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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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.

> 
> 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.

> 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.

Wei.

> > Calculate the maximum number of vcpus before allocating vcpu bitmap.
> > This requires parsing the same config options twice. Extra a macro to do
> > that.
> 
> I'm not sure a macro was really the right answer here. e.g. a function
> which takes a pointer to a bitmap, if the bitmap is null return the size
> which would be needed, otherwise fill in the bitmap, or something. That
> could also be split more obviously into a refactoring step and then the
> addition of the new checks.
> 
> > Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> > ---
> > Valgrind is still clean after this patch applied.
> > 
> > This patch looks massive because it involves indentation changes, but in
> > fact the meat of it is small.
> 
> Even with -b it's impossible to read.
> 
> Ian.

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