On 9/23/20 5:01 PM, Jiri Denemark wrote:
> 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@xxxxxxxxxxxxx>
>>>>>
>>>>> 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@xxxxxxxxxx>
>>>>> ---
>>>>> 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@xxxxxxxxxx>
>>>>
>>>> I'll wait some time for Collin to add Signed-of-by tag before pushing
>>>> this.
>>>>
>>>
>>> Signed-off-by: Collin Walling <walling@xxxxxxxxxxxxx>
>>>
>>> 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
>
On s390x, QMP will internally convert the model "host" to a proper CPU
model.
How about when baselining and if there is only a single CPU definition
and the model is "host" (either provided verbatim by the file, or
converted when the mode is host-passthrough), then perform a CPU model
expansion on the single CPU definition? No baselining would technically
be performed in this case.
I think this would support the behavior reported by the virsh man page:
"""
When FILE contains only a single CPU definition, the command will print
the same CPU with restrictions imposed by the capabilities of
the hypervisor. Specifically, running th virsh
hypervisor-cpu-baseline command with no additional options on the
result of virsh domcapabili‐ ties will transform the host CPU
model from domain capabilities XML to a form directly usable in
domain XML.
"""
Essentially 2 patches:
- set model name to "host" when mode is "host-passthrough" for baseline
- use CPU model expansion when baseline is executed with only a single CPU
I can propose the patches tonight and we can look them over whenever.
--
Regards,
Collin
Stay safe and stay healthy
