On 7/18/24 9:39 AM, Markus Armbruster wrote: > Collin Walling <wall...@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 <jdene...@redhat.com> >> Signed-off-by: Collin Walling <wall...@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." This patch modifies what is populated in the list by filtering it via an intersection of the model's /default/ feature set. The reason for this change was that the deprecated-props list reported for very old s390 models was showing features that were not *actually* a subset of the model's feature set. One of the changes proposed by this patch corrects this for static model expansions. Explained by: "Now, this list is populated with the model's enabled property set when delta changes are applied." The other change introduced by this patch is that the entire list of deprecated-properties is now reported via a full model expansion, explained by: "All deprecated properties are reported otherwise." My thoughts were to acknowledge this change in case a user sees different results between QEMU versions. However, as this @deprecated-props thing is relatively new (a few months), perhaps it does not make sense to include this note? What would you suggest? > > docs/devel/qapi-code-gen.rst section "Documentation markup": > > For legibility, wrap text paragraphs so every line is at most 70 > characters long. > > Separate sentences with two spaces. > Forgot about the 70 rule for these docs. Missed the double space. Thanks for reminding me. Will update. >> # >> # Since: 2.8 >> ## > > [...] > > -- Regards, Collin
