On 11/28/2016 04:55 AM, Jiri Denemark wrote: ...
+static virCPUCompareResult +virCPUs390Compare(virCPUDefPtr host ATTRIBUTE_UNUSED, + virCPUDefPtr cpu ATTRIBUTE_UNUSED, + bool failMessages ATTRIBUTE_UNUSED)
The indentation is a bit off here.
+{ + return VIR_CPU_COMPARE_IDENTICAL; +}
I know there is no code to detect host CPU, but this essentially means that any guest CPU configuration can be started on any s390 host (I'm talking about KVM, of course). Is this correct or would it make sense to somehow compare the guest CPU with the host model returned by QEMU?
We rely on Qemu to perform all runability checking. Not duplicating the checks in Libvirt was a design choice. We are returning VIR_CPU_COMPARE_IDENTICAL to essentially bypass Libvirt checking. perhaps we should just comment that fact here?
Which reminds me I don't think I've ever seen any s390 CPU XML. Could you just add some test cases focused on s390 CPUs to qemuxml2argvtest?
Sure :)
And since this series is largely about QEMU capabilities, it would be nice if you could add a few .replies and corresponding .xml files to tests/qemucapabilitiesdata and also use them in domaincapstest?
Yep. ...
Are you going to support additional features for host-model CPUs? In other words, does the following XML make sense in s390?
<cpu mode='host-model'> <feature name='bla' policy='require'/> <feature name='ble' policy='disable'/> </cpu>
If so, the code is insufficient. Otherwise, it's unnecessarily complicated. There's no need to create "updated" CPU model when you don't need to look at the features specified in the original guest CPU definition.
Yes we are allowing policy='require' and policy='disable'. I don't understand why the current code is insufficient. It seems like virCPUDefPtr guest->features already has the guest's features with the correct policy statements. Our call to virCPUDefStealModel will copy the features pointer from guest to updated. What am I missing. -- -- Jason J. Herne (jjherne@linux.vnet.ibm.com)