Re: [libvirt] [PATCH v1 0/4] CPU Model Comparsion via QEMU
On 04/18/2018 02:46 AM, Jiri Denemark wrote: > 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): s390x model_name NOTE: the presence of 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 > Indeed. I wanted to reject my response after reading Daniel's explanation yesterday :) I'll keep my eye out for these patches that introduce this new
Re: [libvirt] [PATCH v1 0/4] CPU Model Comparsion via QEMU
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): > >> > >> > >> s390x > >> model_name > >> > >> > >> > >> NOTE: the presence of 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
Re: [libvirt] [PATCH v1 0/4] CPU Model Comparsion via QEMU
On Tue, Apr 17, 2018 at 10:18:58AM +0200, 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): > > > > > > s390x > > model_name > > > > > > > > NOTE: the presence of 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(). FYI, in addition to all this, the current 'virsh cpu-compare' command is in fact 100% accurate in what it is reporting today and we can not change it. The 'virsh cpu-compare' command is answering the question "Is this CPU model compatible with the host CPU model" The problem is that when doing live migration, we don't care about this question. We were asking the wrong question. We instead need to ask "Is this CPU model compatible with the hypervisor CPU model" We can't simply change the semantics of the existing API to answer a different question, because the original question is still relevant and useful for some usage scenarios. So yes, we must have a new API that lets us ask the right question for live migration compat. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 0/4] CPU Model Comparsion via QEMU
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): >> >> >> s390x >> model_name >> >> >> >> NOTE: the presence of 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". 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? Just some of my thoughts, and thanks for your response. > > -- > libvir-list mailing list > libvir-list@redhat.com > https://www.redhat.com/mailman/listinfo/libvir-list > -- Respectfully, - Collin Walling -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 0/4] CPU Model Comparsion via QEMU
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): > > > s390x > model_name > > > > NOTE: the presence of 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 0/4] CPU Model Comparsion via QEMU
[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): s390x model_name NOTE: the presence of 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. The second patch refactors the xml -> CPUDef code. This is used in virCPUCompareXML, qemuCompareCPU (and potentially for baseline). The third patch creates the interface from libvirt to QEMU and handles parsing the CPUDef -> JSON and JSON -> CPUModelInfo. In order to respect the data returned from this command, we capture the "responsible_properties" field from the comparison command even though it is not entirely necessary for what we are trying to accomplish here (which is to simply report if the comparison result is "incompatible", "superset", or "identical"). The fourth patch creates the interface from virsh cpu-compare to the qemu driver. The qemu driver will handle parsing the xml passed to the virsh command and parsing the result of the QMP command. Note that the "incompatible" and "subset" results are grouped together (and report "incompatible"). TODO: - implement cpu-baseline (I've noted Chris Venteicher's current progress on this): https://www.redhat.com/archives/libvir-list/2018-April/msg01274.html - update qemucapabilitiestest files for qemu >= 2.8.0 (need to add comparison command) - consider adding a new test file for comparing cpu models via QEMU if necessary Collin Walling (4): qemu: place qemuCaps host cpu model in virCaps cpu_conf: xml to cpu definition parse helper qemu: implement query-cpu-model-comparison via qemu qemu: hook up cpu-comparison to qemu driver src/conf/cpu_conf.c | 30 ++ src/conf/cpu_conf.h | 6 ++ src/cpu/cpu.c| 14 + src/libvirt_private.syms | 1 + src/qemu/qemu_capabilities.c | 108 +++- src/qemu/qemu_capabilities.h | 9 +++ src/qemu/qemu_driver.c | 96 src/qemu/qemu_monitor.c | 14 + src/qemu/qemu_monitor.h | 6 ++ src/qemu/qemu_monitor_json.c | 128 +++ src/qemu/qemu_monitor_json.h | 6 ++ 11 files changed, 404 insertions(+), 14 deletions(-) -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
