From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ian Campbell <ian.campbell@citrix.com>
Subject: Re: [PATCH for-4.6 2/3] xl: fix vNUMA vcpus parsing
Date: Thu, 13 Aug 2015 11:07:47 +0100
Message-ID: <1439460467.23981.53.camel@citrix.com>
References: <1439408161-15015-1-git-send-email-wei.liu2@citrix.com>
	<1439408161-15015-3-git-send-email-wei.liu2@citrix.com>
	<1439458946.23981.43.camel@citrix.com>
	<20150813095405.GQ7460@zion.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta14.messagelabs.com ([193.109.254.103])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <prvs=66097562a=Ian.Campbell@citrix.com>)
	id 1ZPpQO-0000iB-NT
	for xen-devel@lists.xenproject.org; Thu, 13 Aug 2015 10:07:52 +0000
In-Reply-To: <20150813095405.GQ7460@zion.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Wei Liu <wei.liu2@citrix.com>
Cc: Xen-devel <xen-devel@lists.xenproject.org>, Dario Faggioli <dario.faggioli@citrix.com>, Ian Jackson <Ian.Jackson@eu.citrix.com>, Boris Ostrovsky <boris.ostrovsky@oracle.com>
List-Id: xen-devel@lists.xenproject.org

On Thu, 2015-08-13 at 10:54 +0100, Wei Liu wrote:
> 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.

Right. Which is trivial to detect as we go through the parsing and raise an
appropriate error.

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

If the cost of supporting that is this patch then I disagree. If you can
find a way to do it simply and cleanly then fine, maybe.

In fact I even disagree in general, we can and should provide warnings or
errors for things which we know are bad and which are most likely
unintentional, but provide overrides.

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

Not always true, e.g. from vcpuset:

       if (max_vcpus > dominfo.vcpu_online && max_vcpus > host_cpu) {
            fprintf(stderr, "You are overcommmitting! You have %d physical" \
                    " CPUs and want %d vCPUs! Aborting, use --ignore-host to" \
                    " continue\n", host_cpu, max_vcpus);
            rc = 1;
        }

Ian.