Re: [RFC PATCH 0/1] support deprecated-props from query-cpu-model-expansion

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.

> 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.

> 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.

> > 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/

> > 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(a)lists.libvirt.org > To unsubscribe send an email to devel-leave(a)lists.libvirt.org With regards, Daniel