From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wei Liu Subject: Re: [PATCH v5 08/24] libxl: introduce libxl__vnuma_config_check Date: Fri, 13 Feb 2015 16:06:35 +0000 Message-ID: <20150213160635.GJ13644@zion.uk.xensource.com> References: <1423770294-9779-1-git-send-email-wei.liu2@citrix.com> <1423770294-9779-9-git-send-email-wei.liu2@citrix.com> <21726.1811.311219.353545@mariner.uk.xensource.com> <20150213151251.GE13644@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Elena Ufimtseva Cc: Wei Liu , Ian Campbell , Andrew Cooper , Dario Faggioli , Ian Jackson , "xen-devel@lists.xen.org" , Jan Beulich List-Id: xen-devel@lists.xenproject.org On Fri, Feb 13, 2015 at 10:39:25AM -0500, Elena Ufimtseva wrote: > On Fri, Feb 13, 2015 at 10:12 AM, 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. > > > >> 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. > > > >> 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? > > The checks themselves look reasonable. And unforgiving :) > I think we had discussion before and some previous patches were > bailing out to some default/basic vnuma > configuration (when possible) in case of 'recoverable' errors in config. > Since this is new I would start with strict then consider recoverable configs later. It's hard to code for something that's not yet well defined. > Any sanity checks for distances? > The same applies, what is a valid distance what is not? I guess zero is not valid? Or do we enforce that the distance to local node must be smaller than or equal to the distance to remote node? Wei. > > > > Wei. > > > >> Thanks, > >> Ian. > > > > -- > Elena