On Fri, 2015-02-13 at 15:12 +0000, Wei Liu wrote:
> On Fri, Feb 13, 2015 at 02:15:47PM +0000, Ian Jackson wrote:
> > Wei Liu writes ("[PATCH v5 08/24] libxl: introduce libxl__vnuma_config_check"):
> > > This function is used to check whether vNUMA configuration (be it
> > > auto-generated or supplied by user) is valid.
> > 
> > This looks plausible, but I think you should explain what the impact
> > of this patch is.  Presumably the intent is to replace various later
> > failures with ERROR_FAIL with something more useful and more
> > specific ?
> > 
> 
> Yes, providing more useful error message is on aspect. Another aspect is
> just to do sanity check -- passing an invalid layout to guest doesn't
> make much sense.
> 
I agree with Wei. There are a lot of possible variants and combinations
of all these parameters, and the earlier we assess the entire set makes
sense the better.

> > Are there any cases which this new check forbids but which are
> > currently accepted by libxl ?  If so then we have to think about
> > compatibility.
> > 
> 
> First thing is there is no previous supported vNUMA interface in
> toolstack so there won't be a situation where previous good config
> doesn't pass this check.
> 
> Second thing is if user supplies a config without vNUMA configuration
> this function will not get called, so it won't have any effect.
> 
Indeed.

> > Also I would like to see an ack from the authors of the vnuma support,
> > as I'm not familiar enough with vnuma to fully understand the
> > semantics of the new checks.
> > 
> 
> Elena and Dario, what do you think?
> 
I made some comments on the code, but, those aside, the checks Wei
performs are the correct ones for me.

Regards,
Dario