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

[Thomas Huth <thuth@redhat.com>]

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