[libvirt] [PATCH v4 0/8] CPU Model Baseline and Comparison for s390x

[Collin Walling]

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 at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
> 

Thank you for your review!

-- 
Respectfully,
- Collin Walling