[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 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
>>>
>>
> 
> 


-- 
Regards,
Collin

Stay safe and stay healthy


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]