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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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?

If you want to delay it to Xen 4.4 that is OK with me - I can carry the
patch in my tree and post it then.

> 
> Ian.
>