Re: [Qemu-devel] [RFC 0/2] cpu-add compatibility for query-hotpluggable-cpus implementations

[Peter Krempa]

On Mon, Jul 18, 2016 at 18:20:35 +0200, Igor Mammedov wrote:
> On Mon, 18 Jul 2016 17:06:18 +0200
> Peter Krempa <address@hidden> wrote:
> > On Mon, Jul 18, 2016 at 19:19:18 +1000, David Gibson wrote:

[...]

> > First of the problem is the missing link between the NUMA topology
> > (currently confirured via 'cpu id' which is not linked in any way to the
> > query-hotpluggable-cpus entries). This basically means that I'll have to
> > re-implement the qemu numbering scheme and hope that it doesn't change
> > until a better approach is added.
> with current 'in order' plug/unplug limitation behavior is the same as
> for cpu-add (wrt x86) so device_add could be used as direct replacement
> of cpu-add in NUMA case.
> 
> Numa node to CPU in query-hotpluggable-cpus a missing part
> but once numa mapping for hotplugged CPUs (which is broken now) is fixed
> (fix https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg00595.html)
> I'll be ready to extend x86.query-hotpluggable-cpus with numa mapping
> that -numa cpus=1,2,3... happened to configure.

So this is one instance where we need to know the relation between the
cpu 'number' and the entry in query-hotpluggable-cpus. (see below ...)

I'm aware though that for the NUMA mapping there is a plan to do it
differently in an upcomming release so that may eventually be solved

> (note: that device_add cpu,node=X that doesn't match whatever has been
> configured with -numa cpus=... will rise error, as numa configuration
> is static and fixed at VM creation time, meaning that "node" option
> in query-hotpluggable-cpus is optional and only to inform users to
> which node cpu belongs)

That is okay in this regard, I'm not planing on modifying the
configuration once we start it. We although need to allow keeping an
arbitrary configuration as it is possible now.

> > Secondly from my understanding of the current state it's impossible to
> > select an arbitrary cpu to hotplug but they need to happen 'in order' of
> > the cpu id pointed out above (which is not accessible). The grand plan
> > is to allow adding the cpus in any order. This makes the feature look
> > like a proof of concept rather than something useful.
> having out-of-order plug/unplug would be nice but that wasn't

Not only nice but it's really necessary for NUMA enabled guests so that
we can plug cpus into a given node. Otherwise NUMA guests can't take any
advantage of this since you can't control where to add vcpus.

> the grand plan. Main reason is to replace cpu-add with 'device_add cpu' and
> on top of that provide support for 'device_del cpu' instead of adding cpu-del
> command.

Unfortunately combination of the following:

- necessity to plug the vcpus in a certain order

- query-hotpluggable-cpus not reporting any ordering information

- order of entries in query-hotpluggable-cpus is arbitrary
  The documentation doesn't codify any ordering of the entries. This
  series also contains a patch that changes the order, thus the order
  information is unreliable.

make the interface unusable as-is.

With the interface there isn't a certain way how we could select the
correct entry to plug. We can just guess (basically reimplement qemu's
algorithm for numbering the cpus).

By codifying the order of entries (in any order, but it shall not be
changed afterthat) or numbering the entries we can at least eliminate the
guessing it would be possible to actually use this. (This basically
means that either the order or a index will in the end encode the
information that I've requested earlier [1])

As for the NUMA node numbering we can still guess (reimplement qemu's
algorithm for the numbering) with the data scraped from the above
information (thus basically infer the 'index' of the cpus [1]). This can
be later changed to the new interface once it will be done.

The gist of the above is that by disallowing arbitrary order of hotplug
you basically need to tell libvirt the 'index' of the cpus either
directly or indirectly by inference from the order of entries in
query-hotpluggable-cpus and the 'vcpus-count' field.

> And as result of migration to device_add to avoid changing -smp to match
> present cpus count on target and reuse the same interface as other devices.
> 
> We can still pick 'out of order' device_add cpu using migration_id patch
> and revert in-order limit patch. It would work for x86,
> but I think there were issues with SPAPR, that's why I'm in favor of
> in-order limit approach.

[...]

> > > To make this work, I need to broaden the semantics of cpu-add: it will
> > > a single entry from query-hotpluggable-cpus, which means it may add
> > > multiple vcpus, which the x86 implementation did not previously do.  
> > 
> > See my response to 2/2. If this requires to add -device for the
> > hotplugged entries when migrating it basically doesn't help at all.
> > 
> > > I'm not sure if the intended semantics of cpu-add were ever defined
> > > well enough to say if this is "wrong" or not.  
> > 
> > For x86 I'll also need to experiment with the combined use of cpu-add
> > and device_add interfaces.
> It should work, though I'd not recommend to use them together as cpu-add
> will be obsoleted eventually.

That makes sense now that I know how it's supposed to work.

> >I plan to add a implementation which
> > basically uses the old API in libvirt but calls the new APIs in qemu if
> > they were used previously. 
> (skip)
> 
> >(We still need to fall back to the old API for migration compatibility)
> Why?

Now that I've read the other responses I see that we don't need to do it
any more.

Peter

[1]
https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg01710.html