From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH for-4.6 v2 2/3] xl: error out if vNUMA specifies more vcpus than pcpus Date: Sun, 16 Aug 2015 09:51:53 +0100 Message-ID: <1439715113.3480.13.camel@citrix.com> References: <1439480480-20939-1-git-send-email-wei.liu2@citrix.com> <1439480480-20939-3-git-send-email-wei.liu2@citrix.com> <1439508345.24583.91.camel@citrix.com> <20150813233802.GA4698@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZQtfa-0003Sz-Oz for xen-devel@lists.xenproject.org; Sun, 16 Aug 2015 08:51:58 +0000 In-Reply-To: <20150813233802.GA4698@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 , Dario Faggioli Cc: Xen-devel , Ian Jackson List-Id: xen-devel@lists.xenproject.org On Fri, 2015-08-14 at 00:38 +0100, Wei Liu wrote: > > In fact, if b_info->max_vcpus is 0, the elements of vcpu_parsed are > > sized against the host pcpus, and we risk to call libxl_bitmap_set() for > > vcpus beyond that limit, while parsing the "vcpus" subsection of the > > vnode specification (which happens _before_ this check). > > > > Or am I missing something? > > > > That's fine because that function has no effect when you try to set a > bit beyond its size. This sort of subtlety is worthy of a comment in either the code of the commit message. > > Assuming I'm not, it seems to me that a solution could be to check for > > this situation _inside_ the 'else if (!strcmp("vcpus", option))'. In > > fact, if "maxvcpus" has not been specified, as soon as the end of one of > > the ranges --as returned by parse_range()-- is beyond host_cpus, we know > > we'd be going past the limit of the corresponding element of > > vcpu_parsed, and we can error out. FWIW that's what I was expecting to happen... > > It'll most likely be a bit uglier than this patch, but probably still > > less complex than v1. :-) > That doesn't make any difference in terms of functionality. I would > rather leave the parsing bit as it is and deal with fallout > separately. > That would make code cleaner IMHO. ... It's a bit wasteful to keep parsing after things have already failed, but I suppose that's not too bad and we can live with it. Ian