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 :)

-- 
Regards,
  Collin


Reply via email to