Re: [PATCH v3] target/s390x: filter deprecated properties based on model expansion type

[Collin Walling <walling@linux.ibm.com>]

On 7/26/24 3:15 AM, Markus Armbruster wrote:
> Collin Walling <walling@linux.ibm.com> writes:
> 
>> On 7/25/24 3:39 AM, David Hildenbrand wrote:
>>> On 25.07.24 09:35, Markus Armbruster wrote:
>>>> Markus Armbruster <armbru@redhat.com> writes:
> 
> [...]
> 
>>>>> Arguments that are silently ignored is bad interface design.
>>>>>
>>>>> Observe: when CpuModelInfo is an argument, @deprecated-props is always
>>>>> ignored.  When it's a return value, absent means {}, and it can be
>>>>> present only for certain targets (currently S390).
>>>>>
>>>>> The reason we end up with an argument we ignore is laziness: we use the
>>>>> same type for both roles.  We can fix that easily:
>>>>>
>>>>>      { 'struct': 'CpuModel',
>>>>>        'data': { 'name': 'str',
>>>>>                  '*props': 'any' } }
>>>>>
>>>>>      { 'struct': 'CpuModelInfo',
>>>>>        'base': 'CpuModel',
>>>>>        'data': { '*deprecated-props': ['str'] } }
>>>>>
>>>>> Use CpuModel for arguments, CpuModelInfo for return values.
>>>>>
>>>>> Since @deprecated-props is used only by some targets, I'd make it
>>>>> conditional, i.e. 'if': 'TARGET_S390X'.
>>>>
>>>> If we want just query-cpu-model-expansion return deprecated properties,
>>>> we can instead move @deprecated-props from CpuModelInfo to
>>>> CpuModelExpansionInfo.
>>>
>>> That might a bit more sense, because deprecated-props does not make any 
>>> sense as input parameter, for example.
>>
>> Will do.  Thanks for the feedback.  v4 in the works.
> 
> We better get this into 9.1.  Plan B: mark @deprecated-props unstable to
> avoid backward compatibility pain.
> 
> 

Agreed, it would go a long way to squeeze this in before things get too
messy.

v4 is posted.  I think Thomas is unavailable, so if @David would not
mind handling this?  I'm confident that including this data within the
expansion response is the right way to go.  If there are any lingering
discrepancies WRT documentation, that can be fixed later.

-- 
Regards,
  Collin