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