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@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list > Thank you for your review! -- Respectfully, - Collin Walling -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
