Re: [libvirt] [PATCH 1/6] s390: Cpu driver support for update and compare
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(a)linux.vnet.ibm.com)