From: Collin Walling <walling(a)linux.ibm.com&gt; Before: $ uname -m s390x $ cat passthrough-cpu.xml <cpu check="none" mode="host-passthrough" /> $ virsh hypervisor-cpu-compare passthrough-cpu.xml error: Failed to compare hypervisor CPU with passthrough-cpu.xml error: internal error: unable to execute QEMU command 'query-cpu-model-comp arison': Invalid parameter type for 'modelb.name', expected: string After: $ virsh hypervisor-cpu-compare passthrough-cpu.xml CPU described in passthrough-cpu.xml is identical to the CPU provided by hy pervisor on the host Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ae715c01d7..1cecef01f7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12336,6 +12336,15 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn, if (virCPUDefParseXMLString(xmlCPU, VIR_CPU_TYPE_AUTO, &cpu) < 0) goto cleanup; + if (!cpu->model) { + if (cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) { + cpu->model = g_strdup("host"); + } else { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("cpu parameter is missing a model name")); + goto cleanup; + } + } ret = qemuConnectCPUModelComparison(qemuCaps, cfg->libDir, cfg->user, cfg->group, hvCPU, cpu, failIncompatible);

On 9/23/20 10:18 AM, Jiri Denemark wrote: > On Wed, Sep 23, 2020 at 09:26:58 +0200, Tim Wiederhake wrote: >> From: Collin Walling <walling(a)linux.ibm.com&gt; >> >> Before: >> $ uname -m >> s390x >> $ cat passthrough-cpu.xml >> <cpu check="none" mode="host-passthrough" /> >> $ virsh hypervisor-cpu-compare passthrough-cpu.xml >> error: Failed to compare hypervisor CPU with passthrough-cpu.xml >> error: internal error: unable to execute QEMU command 'query-cpu-model-comp >> arison': Invalid parameter type for 'modelb.name', expected: string >> >> After: >> $ virsh hypervisor-cpu-compare passthrough-cpu.xml >> CPU described in passthrough-cpu.xml is identical to the CPU provided by hy >> pervisor on the host >> >> Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; >> --- >> src/qemu/qemu_driver.c | 9 +++++++++ >> 1 file changed, 9 insertions(+) >> >> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c >> index ae715c01d7..1cecef01f7 100644 >> --- a/src/qemu/qemu_driver.c >> +++ b/src/qemu/qemu_driver.c >> @@ -12336,6 +12336,15 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn, >> if (virCPUDefParseXMLString(xmlCPU, VIR_CPU_TYPE_AUTO, &cpu) < 0) >> goto cleanup; >> >> + if (!cpu->model) { >> + if (cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) { >> + cpu->model = g_strdup("host"); >> + } else { >> + virReportError(VIR_ERR_INVALID_ARG, "%s", >> + _("cpu parameter is missing a model name")); >> + goto cleanup; >> + } >> + } >> ret = qemuConnectCPUModelComparison(qemuCaps, cfg->libDir, >> cfg->user, cfg->group, >> hvCPU, cpu, failIncompatible); > > Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com&gt; > > I'll wait some time for Collin to add Signed-of-by tag before pushing > this. > Signed-off-by: Collin Walling <walling(a)linux.ibm.com&gt; Thanks!

On Wed, Sep 23, 2020 at 16:17:04 -0400, Collin Walling wrote: > On 9/23/20 2:52 PM, Collin Walling wrote: >> On 9/23/20 10:18 AM, Jiri Denemark wrote: >>> On Wed, Sep 23, 2020 at 09:26:58 +0200, Tim Wiederhake wrote: >>>> From: Collin Walling <walling(a)linux.ibm.com&gt; >>>> >>>> Before: >>>> $ uname -m >>>> s390x >>>> $ cat passthrough-cpu.xml >>>> <cpu check="none" mode="host-passthrough" /> >>>> $ virsh hypervisor-cpu-compare passthrough-cpu.xml >>>> error: Failed to compare hypervisor CPU with passthrough-cpu.xml >>>> error: internal error: unable to execute QEMU command 'query-cpu-model-comp >>>> arison': Invalid parameter type for 'modelb.name', expected: string >>>> >>>> After: >>>> $ virsh hypervisor-cpu-compare passthrough-cpu.xml >>>> CPU described in passthrough-cpu.xml is identical to the CPU provided by hy >>>> pervisor on the host >>>> >>>> Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; >>>> --- >>>> src/qemu/qemu_driver.c | 9 +++++++++ >>>> 1 file changed, 9 insertions(+) >>>> >>>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c >>>> index ae715c01d7..1cecef01f7 100644 >>>> --- a/src/qemu/qemu_driver.c >>>> +++ b/src/qemu/qemu_driver.c >>>> @@ -12336,6 +12336,15 @@ qemuConnectCompareHypervisorCPU(virConnectPtr conn, >>>> if (virCPUDefParseXMLString(xmlCPU, VIR_CPU_TYPE_AUTO, &cpu) < 0) >>>> goto cleanup; >>>> >>>> + if (!cpu->model) { >>>> + if (cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) { >>>> + cpu->model = g_strdup("host"); >>>> + } else { >>>> + virReportError(VIR_ERR_INVALID_ARG, "%s", >>>> + _("cpu parameter is missing a model name")); >>>> + goto cleanup; >>>> + } >>>> + } >>>> ret = qemuConnectCPUModelComparison(qemuCaps, cfg->libDir, >>>> cfg->user, cfg->group, >>>> hvCPU, cpu, failIncompatible); >>> >>> Reviewed-by: Jiri Denemark <jdenemar(a)redhat.com&gt; >>> >>> I'll wait some time for Collin to add Signed-of-by tag before pushing >>> this. >>> >> >> Signed-off-by: Collin Walling <walling(a)linux.ibm.com&gt; >> >> Thanks! >> > > Actually, it might help to extend this functionality for baseline as > well. If anything to at least catch the case when a CPU definition in > the XML file is missing a <model> tag. Right now, virsh will either > report "an unknown error occurred" when the XML file contains a _single_ > <cpu> element without a <model> tag, or it will report the same "Invalid > parameter type for 'modela.name', expected: string" mentioned above when > there are multiple definitions in the file, and at least one of them is > missing a <model> tag. Hmm, I would expect libvirt to complain about missing CPU model. If that's not the case, we need to fix it. But we should not fix it the same way. This change in the Compare API is hidden inside libvirt, we just tell QEMU we're comparing "host" when the cpu->mode is host-passthrough and get the result, which is basically yes/no. But with baseline we get the result and make a guest CPU definition out of it. By setting cpu->model to "host" we would could end up with <model>host</model> in the result or even random result depending on the host performing the baseline API. This API should just reject any CPU definition without <model> as invalid argument. Jirka