On Tue, Apr 17, 2018 at 11:32:17 -0400, Collin Walling wrote:
> On 04/17/2018 04:18 AM, Jiri Denemark wrote:
> > On Mon, Apr 16, 2018 at 19:16:05 -0400, Collin Walling wrote:
> >> [Overview]
> >>
> >> This patch series implements an interface to "query-cpu-model-comparison"
> >> (available QEMU ~2.8.0) via virsh cpu-compare.
> >>
> >> [Using This Feature]
> >>
> >> Run virsh cpu-compare (ensure you are running the virsh in your build dir) 
> >> and
> >> pass it an xml file describing a cpu definition. You can copy the cpu xml 
> >> from
> >> virsh capabilities (if you want to compare the host cpu to itself), or a 
> >> cpu 
> >> defined in any guest xml. Additionally, you can create a cpu xml as such 
> >> (e.g.
> >> for s390x):
> >>
> >> <cpu>
> >>     <arch>s390x</arch>
> >>     <model fallback='forbid'>model_name</model>
> >>     <feature policy='require|disable' name='feature_name'/>
> >> </cpu>
> >>
> >> NOTE: the presence of <arch> is optional and it will treat the cpu defined 
> >> in 
> >>       the xml as a host cpu. This will disregard all feature policies 
> >> (i.e. 
> >>       all features listed will behave with policy='require', even if 
> >> disable 
> >>       is specified).
> >>
> >> NOTE: as s390x only supports feature policies 'require' and 'disable', I am
> >>       uncertain how to handle the other policies when parsing CPUDef to 
> >> JSON.
> >>
> >> [Example Output]
> >>
> >> On an s390x system running a z13.2, this is the expected output (where 
> >> each file
> >> describes a CPU model corresponding to the name of the file):
> >>
> >>     $ virsh cpu-compare zEC12.xml
> >>     Host CPU is a superset of CPU described in zEC12.xml
> >>
> >>     $ virsh cpu-compare z13.2.xml
> >>     CPU described in z13.2.xml is identical to host CPU
> >>
> >>     $ virsh cpu-compare z14.xml
> >>     CPU described in z14.xml is incompatible with host CPU
> >>
> >>     $ virsh cpu-compare z14.xml --error
> >>     error: Failed to compare host CPU with z14.xml
> >>     error: the CPU is incompatible with host CPU
> >>
> >>     $ virsh cpu-compare z12345.xml 
> >>     error: Failed to compare host CPU with z12345cpu.xml
> >>     error: internal error: unable to execute QEMU command 
> >> 'query-cpu-model-comparison': The CPU definition 'z12345-base' is unknown.
> >>
> >> [Patch Rundown]
> >>
> >> The first patch copies the host CPU definition from qemuCaps to virCaps so
> >> we can easily access the host CPU model and features when we handle the CPU
> >> comparison in qemu_driver. Note that we take care to not clobber anything
> >> already stored in the host CPU definition until we have successfully 
> >> constructed a new host CPU definition.
> > 
> > I think this is a wrong approach. You'd be basically giving random
> > answers depending on which QEMU binary is probed first. The reason for
> > storing the CPU model in qemuCaps is that it is tight to a particular
> > QEMU binary rather than the host itself. The model reported by one
> > binary may not be usable with other binaries and this applies to any
> > comparisons or other operations done with this CPU model.
> > 
> > In other words, we need to introduce a new set of CPU related APIs which
> > will take more arguments so that the caller may specify what binary,
> > virt type, and machine type they want to use. In other words, the APIs
> > should support parameters similar to virConnectGetDomainCapabilities().
> > 
> > I'm currently starting to work on these new APIs.
> > 
> > Jirka
> 
> I see your concern.
> 
> I understand your points behind having multiple arguments to finely control
> which qemu we probe, but what do you think of the current code within
> "virQEMUCapsInitGuest"? If I understand it correctly, then it has a way of 
> querying the "native qemu binary" capabilities (e.g. qemu-kvm).
> 
> We could refactor this code to get these "kvmbinCaps" when we need it, and
> from that we can retrieve the host CPU model. We would not need to specify
> a binary for this, as we already have a list of "native binaries" that we can
> test. As for virt type, we can still specify this via 
> "virQEMUCapsGetHostModel".

Why would we need to refactor anything? We definitely don't want to
advertise any CPU model we get from QEMU in host capabilities and the
API implementations in qemu_driver have access to the internal cache of
QEMU capabilities. Any code which needs to know the host CPU model
for a specific QEMU binary should get it from the cache via
virQEMUCapsGetHostModel.

> I think that would suffice, at least enough for what this patch series needs.
> I could spin up a patch for this if you'd like and we can see if it makes 
> sense?

Since libvirt doesn't know how to detect and compare s390 CPU by itself,
we can't really implement the existing CPU comparison API. See also
Daniel's email for further explanation.

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to