[PATCH v1] qemu: monitor: substitute missing model name for host-passthrough

[Collin Walling]

One more ping in attempt to get this in the right direction. Otherwise
I'll post my next idea and we can go from there :)

Thinking about this issue, should a host-passthough CPU definition be
permitted for the baseline & comparison commands? The model represented
under this mode is not considered migration safe and it may make sense
to simply fail early since these commands aim to construct/determine a
migratable CPU model, respectively.

Perhaps if a host-passthrough CPU is detected, then an error reporting
something along the lines of "host-passthrough is not supported for
migration" would be sufficient for this?

Thanks again. Hope all is well.

On 8/19/20 11:05 AM, Collin Walling wrote:
> Polite ping to the mailing list regarding this bug. I recall the logic I
> proposed belongs elsewhere, but I'd like to kindly request a followup
> from the respective maintainer(s) for a direction.
> 
> Thanks!
> 
> On 6/26/20 2:31 PM, Collin Walling wrote:
>> On 6/26/20 3:22 AM, Peter Krempa wrote:
>>> On Thu, Jun 25, 2020 at 17:58:46 -0400, Collin Walling wrote:
>>>> When executing the hypervisor-cpu-compare/baseline commands and
>>>> the XML file contains a CPU definition using host-passthrough
>>>> and no model name, the commands will fail and return an error
>>>> message from the QMP response.
>>>
>>> This kind of logic does not seem to belong ...
>>>
>>>> Let's fix this by checking for host-passthrough and a missing
>>>> model name when converting a CPU definition to a JSON object.
>>>> If these conditions are matched, then the JSON object will be
>>>> constructed using "host" as the model name.
>>>>
>>>> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1850680
>>>>
>>>> Signed-off-by: Collin Walling <walling at linux.ibm.com>
>>>> ---
>>>>  src/qemu/qemu_monitor_json.c | 13 ++++++++++++-
>>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
>>>> index 3070c1e6b3..448a3a9356 100644
>>>> --- a/src/qemu/qemu_monitor_json.c
>>>> +++ b/src/qemu/qemu_monitor_json.c
>>>> @@ -5804,9 +5804,20 @@ qemuMonitorJSONMakeCPUModel(virCPUDefPtr cpu,
>>>>  {
>>>>      virJSONValuePtr model = virJSONValueNewObject();
>>>>      virJSONValuePtr props = NULL;
>>>> +    const char *model_name = cpu->model;
>>>>      size_t i;
>>>>  
>>>> -    if (virJSONValueObjectAppendString(model, "name", cpu->model) < 0)
>>>> +    if (!model_name) {
>>>> +        if (cpu->mode == VIR_CPU_MODE_HOST_PASSTHROUGH) {
>>>> +            model_name = "host";
>>>> +        } else {
>>>> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>>>> +                           _("cpu parameter is missing a model name"));
>>>> +            goto error;
>>>> +        }
>>>> +    }
>>>
>>> ... to the monitor code, where we try to just deal with the monitor
>>> itself rather than hiding any logic.
>>>
>>
>> That's fair. Perhaps it should belong to the qemu driver code after the
>> CPU XML to a CPU def conversion. I'm also unsure if "host" is an s390
>> specific thing, or if other archs use it as well.
>>
>>> But the final word needs to be from Jirka, but he is on holidays until
>>> the end of next week.
>>>
>>>
>>>> +
>>>> +    if (virJSONValueObjectAppendString(model, "name", model_name) < 0)
>>>>          goto error;
>>>>  
>>>>      if (cpu->nfeatures || !migratable) {
>>>> -- 
>>>> 2.26.2
>>>>
>>>
>>
>>
> 
>