On 8/20/19 10:06 AM, Jiri Denemark wrote:
First, let me apologize for such a late review. I'll try my best to review your series earlier next time.
Your review is greatly appreciated! I haven't replied to your other posts on this series as my comments were mostly acknowledgements rather than discussion pieces. I'm working through them.
On Wed, Jul 17, 2019 at 10:03:21 -0400, Collin Walling wrote:
When baselining CPU models and the user appends the --features argument to the command, s390x will only report back features that supersede the base model definition.
*NOTE* if the --features flag is intended to expand /ALL/ features available to a CPU model (such as the huge list of features reported by a full CPU model expansion), please let me know and I can resolve this.
I'm not sure how well this fits s390 world, but the semantics of VIR_CONNECT_BASELINE_CPU_EXPAND_FEATURES flag is to report all CPU features which are hidden behind the CPU model. That is, all feature which you'd get when starting QEMU with just the CPU model name and no additional features. The extra features should not be touched at all. Specifically, removing them when the flag is not used is wrong.
To me this looks like the flag should really result in running query-cpu-model-expansion (but likely the "static" one rather then "full" expansion) on the baseline CPU model and reporting the enabled features along with those already included in the baseline feature set.
Actually, query-cpu-model-baseline will return a CPU model along with a feature set. The features returned are the same as those produced from a static expansion on the model. Correct me if I am wrong here: virsh should report features *only* if the --features flag is present. Otherwise, we only report the model name (which can be accomplished by stripping the result of all reported features).
Jirka
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Thank you for your review! -- Respectfully, - Collin Walling