Re: [PATCH] libxl: Make 'xl vcpu-set' work properly on overcommited hosts.

[Ian Campbell <Ian.Campbell@citrix.com>]

On Wed, 2013-05-08 at 18:05 +0100, Konrad Rzeszutek Wilk wrote:
> On Wed, May 08, 2013 at 05:49:34PM +0100, Ian Campbell wrote:
> > On Wed, 2013-05-08 at 17:35 +0100, Konrad Rzeszutek Wilk wrote:
> > > On Wed, May 08, 2013 at 04:04:53PM +0100, Ian Campbell wrote:
> > > > On Wed, 2013-05-08 at 15:29 +0100, Konrad Rzeszutek Wilk wrote:
> > > > > On Wed, May 08, 2013 at 11:46:39AM +0100, Ian Campbell wrote:
> > > > > > On Wed, 2013-05-08 at 11:32 +0100, George Dunlap wrote:
> > > > > > > On Tue, May 7, 2013 at 9:40 PM, Konrad Rzeszutek Wilk
> > > > > > > <konrad.wilk@oracle.com> wrote:
> > > > > > > > The libxl_cpu_bitmap_alloc(..) function, if provided with a zero
> > > > > > > > value for max CPUs will call xc_get_max_cpus() which will retrieve
> > > > > > > > the number of physical CPUs the host has. This is usually
> > > > > > > > OK if the guest's maxvcpus <= host pcpus. But if the value
> > > > > > > > is different, then the bitmap for VCPUs is limited by the
> > > > > > > > number of CPUs the host has.
> > > > > > > >
> > > > > > > > This is incorrect as what we want is to hotplug in the guest
> > > > > > > > the amount of CPUs that the user specified on the command line
> > > > > > > > and not be limited by the amount of physical CPUs.
> > > > > > > >
> > > > > > > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > > > > > > 
> > > > > > > Given the cost/benefits of this, I'm inclined to say this should wait
> > > > > > > until the 4.4 window opens.  Having more guest vcpus than host pcpus
> > > > > > > is not an urgent need, and there is a chance (however small) of this
> > > > > > > exposing some other kind of bug.
> > > > > > 
> > > > > > Not disputing this.
> > > > > > 
> > > > > > I imagine the desire is to add vcpus to a guest after migrating to a
> > > > > > larger host. If so then this should be in the commit log because you are
> > > > > > right that as the commit message currently stands the immediate response
> > > > > > is "why on earth..."
> > > > > 
> > > > > > 
> > > > > > My only other concern would be that the existing code has a buffer
> > > > > > overrun under these circumstances. I've not checked this either.
> > > > > 
> > > > > No over-runs, but this is a regression compared to Xend.
> > > > 
> > > > >From your updated description it seems like if you migrated to a larger
> > > > host you could indeed plug more VCPUS, up to the limit on that host, is
> > > > that right?
> > > > 
> > > > If that's the case then it sounds to me as if the xl behaviour is
> > > > actually an improvement over xend's, except perhaps for some slightly
> > > > niche test scenarios.
> > > 
> > > Well, overcommit comes in mind. Say you migrate to a 4PCPU box and you
> > > have 12VCPUs, then you decide to go down to 4, then back to 16 before
> > > migrating it to some other box. Can't do.
> > 
> > You could do it *after* the migration back to a 16 way box n stead of
> > before though, which is most likely when you would actually want to do
> > it...
> 
> I am kind of lost. Are we arguing for this being a bug or whether there is
> justification for putting in Xen 4.3?

The former needs deciding before the latter.

I'm not convinced that the current xl behaviour of refusing to
overcommit VCPUs on a host isn't the right one for the majority of use
cases. Obviously the silently refusing bit is a bug which should be
fixed.

I don't buy that this is a "regression compared to Xend". It's certainly
a difference from how xend behaved but it seems on the whole to be a
positive one (i.e. xend was wrong).

Can you explain the use case for wanting to do this? I don't think the
migration one you give above is very convincing since a normal user
wouldn't want to overcommit on the source host, they would want to
migrate and then increase the number of vcpus, without ever
overcommitting, and therefore without the terrible performance of
overcommitting.

A compromise might be a non-default option to allow users to force
overcommit but to otherwise deny it.

Ian.