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