From mboxrd@z Thu Jan 1 00:00:00 1970 From: Elena Ufimtseva Subject: Re: [PATCH v5 08/24] libxl: introduce libxl__vnuma_config_check Date: Fri, 13 Feb 2015 11:11:39 -0500 Message-ID: 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> <20150213160635.GJ13644@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20150213160635.GJ13644@zion.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu Cc: 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 11:06 AM, Wei Liu wrote: > 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. Understood. > >> 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? Yes, I think the second condition is enough for strict checking. > > Wei. > >> > >> > Wei. >> > >> >> Thanks, >> >> Ian. >> >> >> >> -- >> Elena -- Elena