qemu-s390x
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Collin Walling
Subject: Re: [PATCH v3] target/s390x: filter deprecated properties based on model expansion type
Date: Fri, 26 Jul 2024 15:11:12 -0400
User-agent: Mozilla Thunderbird

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]