Re: [PATCH v15 10/11] qapi/s390x/cpu topology: CPU_POLARITY_CHANGE qapi event

[Nina Schoetterl-Glausch]

On Thu, 2023-02-09 at 10:04 +0000, Daniel P. Berrangé wrote:
> On Wed, Feb 08, 2023 at 06:35:39PM +0100, Nina Schoetterl-Glausch wrote:
> > On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote:
> > > When the guest asks to change the polarity this change
> > > is forwarded to the admin using QAPI.
> > > The admin is supposed to take according decisions concerning
> > > CPU provisioning.
> > > 
> > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> > > ---
> > >  qapi/machine-target.json | 30 ++++++++++++++++++++++++++++++
> > >  hw/s390x/cpu-topology.c  |  2 ++
> > >  2 files changed, 32 insertions(+)
> > > 
> > > diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> > > index 58df0f5061..5883c3b020 100644
> > > --- a/qapi/machine-target.json
> > > +++ b/qapi/machine-target.json
> > > @@ -371,3 +371,33 @@
> > >    },
> > >    'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] }
> > >  }
> > > +
> > > +##
> > > +# @CPU_POLARITY_CHANGE:
> > > +#
> > > +# Emitted when the guest asks to change the polarity.
> > > +#
> > > +# @polarity: polarity specified by the guest
> > > +#
> > > +# The guest can tell the host (via the PTF instruction) whether the
> > > +# CPUs should be provisioned using horizontal or vertical polarity.
> > > +#
> > > +# On horizontal polarity the host is expected to provision all vCPUs
> > > +# equally.
> > > +# On vertical polarity the host can provision each vCPU differently.
> > > +# The guest will get information on the details of the provisioning
> > > +# the next time it uses the STSI(15) instruction.
> > > +#
> > > +# Since: 8.0
> > > +#
> > > +# Example:
> > > +#
> > > +# <- { "event": "CPU_POLARITY_CHANGE",
> > > +#      "data": { "polarity": 0 },
> > > +#      "timestamp": { "seconds": 1401385907, "microseconds": 422329 } }
> > > +#
> > > +##
> > > +{ 'event': 'CPU_POLARITY_CHANGE',
> > > +  'data': { 'polarity': 'int' },
> > > +   'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM'] }
> > 
> > I wonder if you should depend on CONFIG_KVM or not. If tcg gets topology
> > support it will use the same event and right now it would just never be 
> > emitted.
> > On the other hand it's more conservative this way.
> > 
> > I also wonder if you should add 'feature' : [ 'unstable' ].
> > On the upside, it would mark the event as unstable, but I don't know what 
> > the
> > consequences are exactly.
> 
> The intention of this flag is to allow mgmt apps to make a usage policy
> decision.
> 
> Libvirt's policy is that we'll never use features marked unstable.

Does it enforce that, e.g via compat policies?
If so, I assume there is some way to allow use of unstable features in libvirt 
for development?
If for example you're prototyping a new mgmt feature that uses unstable 
commands.

> 
> IOW, the consequence of marking it unstable is that it'll likely
> go unused until the unstable marker gets removed.
> 
> Using 'unstable' is useful if you want to get complex code merged
> before you're quite happy with the design, and then iterate on the
> impl in-tree. This is OK if there's no urgent need for apps to
> consume the feature. If you want the feature to be used for real
> though, the unstable flag is not desirable and you need to finalize
> the design.
> 
> > Also I guess one can remove qemu events without breaking backwards 
> > compatibility,
> > since they just won't be emitted? Unless I guess you specify that a event 
> > must
> > occur under certain situations and the client waits on it?
> 
> As Markus says, that's not a safe assumption. If a mgmt app is expecting
> to receive an event, ceasing to emit it would likely be considered a
> regression.
> 
> 
> With regards,
> Daniel