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