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

[Collin Walling]

On 7/23/24 5:23 AM, Thomas Huth wrote:
> On 22/07/2024 16.50, Collin Walling wrote:
>> On 7/20/24 1:33 AM, Markus Armbruster wrote:
>>> Collin Walling <walling@linux.ibm.com> writes:
>>>
>>>> Currently, there is no way to execute the query-cpu-model-expansion
>>>> command to retrieve a comprehenisve list of deprecated properties, as
>>>> the result is dependent per-model. To enable this, the expansion output
>>>> is modified as such:
>>>>
>>>> When reporting a "full" CPU model, show the *entire* list of deprecated
>>>> properties regardless if they are supported on the model. A full
>>>> expansion outputs all known CPU model properties anyway, so it makes
>>>> sense to report all deprecated properties here too.
>>>>
>>>> This allows management apps to query a single model (e.g. host) to
>>>> acquire the full list of deprecated properties.
>>>>
>>>> Additionally, when reporting a "static" CPU model, the command will
>>>> only show deprecated properties that are a subset of the model's
>>>> *enabled* properties. This is more accurate than how the query was
>>>> handled before, which blindly reported deprecated properties that
>>>> were never otherwise introduced for certain models.
>>>>
>>>> Acked-by: David Hildenbrand <david@redhat.com>
>>>> Suggested-by: Jiri Denemark <jdenemar@redhat.com>
>>>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
>>>> ---
>>>>
>>>> Changelog:
>>>>
>>>>      v3
>>>>      - Removed the 'note' and cleaned up documentation
>>>>      - Revised commit message
>>>>
>>>>      v2
>>>>      - Changed commit message
>>>>      - Added documentation reflecting this change
>>>>      - Made code changes that more accurately filter the deprecated
>>>>          properties based on expansion type.  This change makes it
>>>>          so that the deprecated-properties reported for a static model
>>>>          expansion are a subset of the model's properties instead of
>>>>          the model's full-definition properties.
>>>>
>>>> ---
>>>>   qapi/machine-target.json         |  5 +++--
>>>>   target/s390x/cpu_models_sysemu.c | 16 +++++++++-------
>>>>   2 files changed, 12 insertions(+), 9 deletions(-)
>>>>
>>>> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
>>>> index a8d9ec87f5..67086f006f 100644
>>>> --- a/qapi/machine-target.json
>>>> +++ b/qapi/machine-target.json
>>>> @@ -21,8 +21,9 @@
>>>>   # @props: a dictionary of QOM properties to be applied
>>>>   #
>>>>   # @deprecated-props: a list of properties that are flagged as deprecated
>>>> -#     by the CPU vendor.  These props are a subset of the full model's
>>>> -#     definition list of properties. (since 9.1)
>>>> +#     by the CPU vendor.  These properties are either a subset of the
>>>> +#     properties enabled on the CPU model, or a set of properties
>>>> +#     deprecated across all models for the architecture.
>>>
>>>
>>> When is it "a subset of the properties enabled on the CPU model", and
>>> when is it "a set of properties deprecated across all models for the
>>> architecture"?
> ...
>>
>> Thanks for the feedback!  Pending your response to the above, I'll post
>> a v4.
> 
> Since we've got soft-freeze for 9.1 today, I went ahead and put v3 into my 
> last pull-request before the freeze period starts already. Please post the 
> update to the comments as diff on top of that instead - updates to comments 
> should still be fine for merging during the freeze period.
> 
>   Thanks,
>    Thomas
> 
> 

This is greatly appreciated, Thomas.  Thank you!

-- 
Regards,
  Collin