qemu-devel
[Top][All Lists]
Advanced

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

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


From: Thomas Huth
Subject: Re: [PATCH v2] target/s390x: filter deprecated properties based on model expansion type
Date: Fri, 19 Jul 2024 12:16:08 +0200
User-agent: Mozilla Thunderbird

On 18/07/2024 20.22, Collin Walling wrote:
On 7/18/24 9:39 AM, Markus Armbruster wrote:
Collin Walling <walling@linux.ibm.com> writes:

As s390 CPU models progress and deprecated properties are dropped
outright, it will be cumbersome for management apps to query the host
for a comprehensive list of deprecated properties that will need to be
disabled on older models. To remedy this, the query-cpu-model-expansion
output now behaves by filtering deprecated properties based on the
expansion type instead of filtering based off of the model's full set
of features:

When reporting a static CPU model, only show deprecated properties that
are a subset of the model's enabled features.

When reporting a full CPU model, show the entire list of deprecated
properties regardless if they are supported on the model.

Suggested-by: Jiri Denemark <jdenemar@redhat.com>
Signed-off-by: Collin Walling <walling@linux.ibm.com>
---

Changelog:

     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.

         For example:

         Previously, the z900 static model would report 'bpb' in the
         list of deprecated-properties.  However, this prop is *not*
         a part of the model's feature set, leading to some inaccuracy
         (albeit harmless).

         Now, this feature will not show during a static expansion.
         It will, however, show up in a full expansion (along with
         the rest of the list: 'csske', 'te', 'cte').

@David, I've elected to respectully forgo adding your ack-by on this
iteration since I have changed the code (and therefore the behavior)
between this version and the previous in case you do not agree with
these adjustments.

---
  qapi/machine-target.json         |  8 ++++++--
  target/s390x/cpu_models_sysemu.c | 16 +++++++++-------
  2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index a8d9ec87f5..d151504f25 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -21,8 +21,12 @@
  # @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.  (since 9.1).
+#
+# .. note:: Since 9.1, the list of deprecated props were always a subset
+#    of the model's full-definition list of properites. Now, this list is
+#    populated with the model's enabled property set when delta changes
+#    are applied. All deprecated properties are reported otherwise.

I'm confused.

"Since 9.1, the list of deprecated props were ..." and "Now, this list
is" sounds like you're explaining behavior before and after a change.
What change?  Since only released behavior matters, and
@deprecated-props is new, there is no old behavior to document, isn't
it?

I admittedly had some difficulty articulating the change introduced by
this patch.  The @deprecated-props array, as well as a way for s390x to
populate it, was introduced in release 9.1.  Prior to this patch, the
deprecated-props list was filtered by the CPU model's full feature set.
I attempted to explain this with:

"Since 9.1, the list of deprecated props were always a subset of the
model's full-definition list of properties."

Version 9.1 has not been released yet (see https://wiki.qemu.org/Planning/9.1), so I agree with Markus, this sounds confusing/wrong to me, too.

 Thomas





reply via email to

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