On Wed, Apr 24, 2024 at 03:12:42PM -0400, Collin Walling wrote:
> On 4/24/24 13:51, Collin Walling wrote:
> > On 4/24/24 04:20, Daniel P. Berrangé wrote:
> >> On Tue, Apr 23, 2024 at 05:06:53PM -0400, Collin Walling wrote:
> >>> This optional parameter for query-cpu-model-expansion enables CPU
> >>> model features flagged as deprecated to appear in the resulting
> >>> list of properties.
> >>>
> >>> This commit does not add support beyond adding a new argument
> >>> to the query. All queries with this option present will result
> >>> in an error claiming this option is not supported.
> >>>
> >>> Signed-off-by: Collin Walling <wall...@linux.ibm.com>
> >>> ---
> >>> qapi/machine-target.json | 7 ++++++-
> >>> target/arm/arm-qmp-cmds.c | 7 +++++++
> >>> target/i386/cpu-sysemu.c | 7 +++++++
> >>> target/s390x/cpu_models_sysemu.c | 7 +++++++
> >>> 4 files changed, 27 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> >>> index 29e695aa06..b9da284d2d 100644
> >>> --- a/qapi/machine-target.json
> >>> +++ b/qapi/machine-target.json
> >>> @@ -285,6 +285,10 @@
> >>> #
> >>> # @type: expansion type, specifying how to expand the CPU model
> >>> #
> >>> +# @disable-deprecated-feats: include CPU model features that are
> >>> +# flagged as deprecated. If supported, these features will appear
> >>> +# in the properties list paired with false.
> >>> +#
> >>> # Returns: a CpuModelExpansionInfo describing the expanded CPU model
> >>> #
> >>> # Errors:
> >>> @@ -298,7 +302,8 @@
> >>> ##
> >>> { 'command': 'query-cpu-model-expansion',
> >>> 'data': { 'type': 'CpuModelExpansionType',
> >>> - 'model': 'CpuModelInfo' },
> >>> + 'model': 'CpuModelInfo',
> >>> + '*disable-deprecated-feats': 'bool' },
> >>> 'returns': 'CpuModelExpansionInfo',
> >>> 'if': { 'any': [ 'TARGET_S390X',
> >>> 'TARGET_I386',
> >>
> >> I think this is an odd design approach. Lets consider the
> >> current output:
> >>
> >> (QEMU) query-cpu-model-expansion type=static model={"name":"z14"}
> >> {
> >> "return": {
> >> "model": {
> >> "name": "z14-base",
> >> "props": {
> >> "aefsi": true,
> >> "aen": true,
> >> ...snip...
> >> "vxpd": true,
> >> "zpci": true
> >> }
> >> }
> >> }
> >> }
> >>
> >>
> >> If we want to inform a mgmt app of some features being deprecated,
> >> why not just unconditionally include that info in the reply thus:
> >>
> >>
> >> (QEMU) query-cpu-model-expansion type=static model={"name":"z14"}
> >> {
> >> "return": {
> >> "model": {
> >> "name": "z14-base",
> >> "props": {
> >> "aefsi": true,
> >> "aen": true,
> >> ...snip...
> >> "vxpd": true,
> >> "zpci": true
> >> }
> >> "deprecated-props": ["ppa15", "ri"]
> >> }
> >> }
> >> }
> >>
> >>
> >>
> >> With regards,
> >> Daniel
> >
> > That's a good idea. In this way, we're not mucking up any of the CPU
> > model information and this makes it much more clear as to which features
> > are actually deprecated... I like this more.
> >
> > I'll work on this.
> >
>
> Follow-up question as I look more closely to the QMP response data
> structures: should the "deprecated-props" list be added to the
> CpuModelInfo struct, or to the CpuModelExpansionInfo struct?
>
> The former makes more sense to me, as the deprecated features are tied
> to the actual CPU model... but unsure if other QMP commands would even
> care about this info? I will begin with this approach, and if feedback
> in the interim strongly sways in the other direction, then it should be
> an easy change :)
I hink CpuModelInfo makes more sense than CpuModelExpansionInfo.
The CpuModelExpansionInfo struct feels pretty pointless to me
in fact, since the only thing it contains is CpuModelInfo !
I think it should also be added to 'CpuDefinitionInfo', which
is the return type of 'query-cpu-defintions'. This command already
has a 'unavailable-features' array listing features which the host
does not support. Conceptually having a 'deprecated-features' array
alongside that is a nice fit.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
