Re: [PATCH v2 1/3] cpu-models: add "disable-deprecated-feats" option to cpu model expansion

[Collin Walling]

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 <walling@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.

-- 
Regards,
  Collin