On 6/3/24 1:53 PM, Daniel P. Berrangé wrote: > On Tue, May 07, 2024 at 06:24:20PM -0400, Collin Walling wrote: >> Notes >> ===== >> >> - In my example below, I am running on a z14.2 machine. >> >> - The features that are flagged as deprecated for this model are: bpb, csske, cte, te. >> >> - The discussion regarding the QEMU changes can be found here: https://mail.gnu.org/archive/html/qemu-devel/2024-04/msg04605.html >> >> >> Example of Desired Changes >> ========================== >> >> Here is what I'd like the resulting guest's transient XML to look like for the <cpu> section (I have trimmed the features list for brevity): >> >> ... >> <cpu mode='custom' match='exact' check='partial'> >> <model fallback='forbid'>z14.2-base</model> >> <feature policy='require' name='aen'/> >> <feature policy='require' name='cmmnt'/> >> <feature policy='require' name='aefsi'/> >> ... >> <feature policy='disable' name='cte'/> *** >> <feature policy='require' name='ais'/> >> <feature policy='disable' name='bpb'/> *** >> <feature policy='require' name='ctop'/> >> <feature policy='require' name='gs'/> >> <feature policy='require' name='ppa15'/> >> <feature policy='require' name='zpci'/> >> <feature policy='require' name='sea_esop2'/> >> <feature policy='disable' name='te'/> *** >> <feature policy='require' name='cmm'/> >> <feature policy='disable' name='csske'/> *** >> </cpu> >> ... > >> New Host CPU Model >> ------------------ >> >> Create a new CPU model that is a mirror of the host CPU model with >> deprecated features turned off. Let's call this model "host-recommended". >> A user may define this model in the guest XML as they would any other >> CPU model: >> >> <cpu mode='host-recommended' check='partial'/> >> >> Just as how host-model works, anything defined nested in the <cpu> tag will be ignored. >> >> This model could potentially be listed in the domcapabilities output after the host-model: >> >> <cpu> >> <mode name='host-passthrough' supported='yes'> >> ... >> </mode> >> ... >> <mode name='host-model' supported='yes'> >> ... >> </mode> >> <mode name='host-recommended' supported='yes'> >> ... >> <feature policy='disable' name='cte'/> >> <feature policy='require' name='ais'/> >> <feature policy='disable' name='bpb'/> >> <feature policy='require' name='ctop'/> >> <feature policy='require' name='gs'/> >> <feature policy='require' name='ppa15'/> >> <feature policy='require' name='zpci'/> >> <feature policy='require' name='sea_esop2'/> >> <feature policy='disable' name='te'/> >> <feature policy='require' name='cmm'/> >> <feature policy='disable' name='csske'/> >> </cpu> > > Downside of a new "host-recommended" model is that we now have the > task of wiring it up into various mgmt applications. > Fair. I believe Jiri as also against this new model as well. Let's nix adding a new model and discuss other approaches. >> New Nested Element Under <cpu> >> ------------------------------ >> >> Create a new optional XML element nested under the <cpu> tag that may be used to disable deprecated features. This approach is more explicit compared to creating a new CPU model, and allows the user to disable these features when defining a specific model other than host-model. Here is an example of what the guest's defined XML for the CPU could look like: >> >> <cpu mode='host-model' check='partial'> >> <deprecated_features>off</deprecated_features> >> </cpu> >> >> However, a conflict arises with this approach: parameter priority. It would need to be discussed what the expected behavior should be if a user defines a guest with both a mode to disable deprecated features and any deprecated features listed with the 'require' policy, e.g.: >> >> <cpu mode='custom' match='exact' check='partial'> >> <model fallback='allow'>z13.2-base</model> >> <!-- which one takes priority? --> >> <deprecated_features>off</deprecated_features> >> <feature policy='require' name='csske'/> >> </cpu> > > IMHO 'deprecate_features' controls the initial expansion > of the 'host-model', and then '<feature>' fine tunes it. > > IOW, your example here disables all the deprecated features > by default, and the user has then turned back on the csske > deprecated feature. > > I wouldn't call this a conflict - its just a documentation > task to clarify the prioritization. > Fair enough. As long as it the behavior is consistent and documented, then all should be well with the world. >> Another conflict is setting this option to "on" would have >> no effect on the CPU model (I can't think of a reason why >> someone would want to explicitly enable these features). >> This may not communicate well to the user. > > Well it depends on what we declare the "default" behaviour > to be. > > We only guarantee the host-model -> custom expansion within > the scope of a running guest. > > IOW, if you boot a guest, shut it down, boot it again, we > do NOT guarantee that you'll get the same expansion both > times. > > With this in mind, we could take the view that disabling > deprecated features should be the default behaviour, and > thus an option to turn "on" deprecated features becomes > relevant. > > In theory this could even be a host level tunable in > qemu.conf of whether the host admin wants deprecated > features skipped by default or not. > > Either way, the existance of the <deprecation_fetures> > field on the *live* XML, will give a clear statement of > what behaviour libvirt applied. > Okay, so what I'm taking away from this is that there should be *some* sort of indication on the live guest XML that makes it clear that the feature list includes deprecated features turned on|off. >> >> To report these features, a <deprecatedProperties> tag could be added to the domcapabilities output using the same format I use in my proposed patch for the qemu capabilities file: >> >> <cpu> >> <mode name='host-passthrough' supported='yes'> >> ... >> </mode> >> ... >> <mode name='host-model' supported='yes'> >> ... >> </mode> >> <deprecatedProperties> >> <property name='bpb'/> >> <property name='te'/> >> <property name='cte'/> >> <property name='csske'/> >> </deprecatedProperties> >> </cpu> > > Yes, I think domcapabilities should be reporting on deprecated > features, but s/Properties/Features/ and s/property/feature/ > > In summary, and I'll attempt to incorporate Jiri's feedback here as well: - s/properties/features - ensure a clear indication that the live guest XML includes deprecated features on|off (ideas: via a nested element; an additional cpu attribute -- Jiri seems in favor of the latter) - domcapabilities should report deprecated features (but how and where they should be placed needs to be discussed to make it clear which model those features were reported on) - how to handle baseline needs more discussion (mentioned in Jiri's feedback) >> >> Please let me know your thoughts. Once an approach is agreed upon, I will begin development. >> >> Collin Walling (1): >> qemu: monitor: parse deprecated-props from query-cpu-model-expansion >> response >> >> src/qemu/qemu_capabilities.c | 30 ++++++++++++++++++++++++++++++ >> src/qemu/qemu_monitor.h | 2 ++ >> src/qemu/qemu_monitor_json.c | 29 ++++++++++++++++++++++++----- >> 3 files changed, 56 insertions(+), 5 deletions(-) >> >> -- >> 2.43.0 >> _______________________________________________ >> Devel mailing list -- devel@xxxxxxxxxxxxxxxxx >> To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx > > With regards, > Daniel Thanks for your response! -- Regards, Collin
