Re: [libvirt] [PATCHv2 3/3] qemu_monitor: query-cpu-model-baseline QMP command

From: "John Ferlan" <jferlan(a)redhat.com&gt; To: "Chris Venteicher" <cventeic(a)redhat.com&gt;, libvir-list(a)redhat.com Sent: Friday, April 27, 2018 1:41:03 PM Subject: Re: [libvirt] [PATCHv2 3/3] qemu_monitor: query-cpu-model-baseline QMP command $SUBJ: qemu: Introduce qemuMonitorGetCPUModelBaseline On 04/19/2018 12:06 AM, Chris Venteicher wrote: > Function qemuMonitorGetCPUModelBaseline exposed to carry out a QMP > query-cpu-model-baseline transaction with QEMU. > > QEMU determines a baseline CPU Model from two input CPU Models to > complete the query-cpu-model-baseline transaction. > --- > src/qemu/qemu_monitor.c | 13 ++++++++++ > src/qemu/qemu_monitor.h | 5 ++++ > src/qemu/qemu_monitor_json.c | 57 > ++++++++++++++++++++++++++++++++++++++++++++ > src/qemu/qemu_monitor_json.h | 7 ++++++ > 4 files changed, 82 insertions(+) > > diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c > index 7b647525b..c1098ff72 100644 > --- a/src/qemu/qemu_monitor.c > +++ b/src/qemu/qemu_monitor.c > @@ -3874,6 +3874,19 @@ qemuMonitorCPUModelInfoCopy(const > qemuMonitorCPUModelInfo *orig) > return NULL; > } > 2 blank lines. > +int > +qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon, > + qemuMonitorCPUModelInfoPtr model_a, > + qemuMonitorCPUModelInfoPtr model_b, > + qemuMonitorCPUModelInfoPtr *model_baseline) > +{ > + VIR_DEBUG("model_a->name=%s", model_a->name); > + VIR_DEBUG("model_b->name=%s", model_b->name); > + > + QEMU_CHECK_MONITOR_JSON(mon); > + > + return qemuMonitorJSONGetCPUModelBaseline(mon, model_a, model_b, > model_baseline); > +} No consumer for this yet - are there plans? Just remember that for the consumer we'll probably need some sort of qemu capability to manage whether or not the command "query-cpu-model-baseline" exists for the particular version or not.

> > int > qemuMonitorGetCommands(qemuMonitorPtr mon, > diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h > index d04148e56..c7a80ca63 100644 > --- a/src/qemu/qemu_monitor.h > +++ b/src/qemu/qemu_monitor.h > @@ -1079,6 +1079,11 @@ void > qemuMonitorCPUModelInfoFree(qemuMonitorCPUModelInfoPtr model_info); > qemuMonitorCPUModelInfoPtr > qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig); > > +int qemuMonitorGetCPUModelBaseline(qemuMonitorPtr mon, > + qemuMonitorCPUModelInfoPtr model_a, > + qemuMonitorCPUModelInfoPtr model_b, > + qemuMonitorCPUModelInfoPtr > *model_baseline); > + > int qemuMonitorGetCommands(qemuMonitorPtr mon, > char ***commands); > int qemuMonitorGetEvents(qemuMonitorPtr mon, > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c > index 44c1b2f15..8cb10268f 100644 > --- a/src/qemu/qemu_monitor_json.c > +++ b/src/qemu/qemu_monitor_json.c > @@ -5539,6 +5539,63 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr > mon, > return ret; > } > > +int > +qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, > + qemuMonitorCPUModelInfoPtr model_a, > + qemuMonitorCPUModelInfoPtr model_b, > + qemuMonitorCPUModelInfoPtr > *model_baseline) > +{ > + int ret = -1; > + virJSONValuePtr cmd = NULL; > + virJSONValuePtr reply = NULL; > + virJSONValuePtr data = NULL; > + virJSONValuePtr modela = NULL; > + virJSONValuePtr modelb = NULL; > + > + *model_baseline = NULL; > + > + if (!(modela = qemuMonitorJSONBuildCPUModelInfoToJSON(model_a))) > + goto cleanup; > + > + if (!(modelb = qemuMonitorJSONBuildCPUModelInfoToJSON(model_b))) > + goto cleanup; > + > + if (!(cmd = qemuMonitorJSONMakeCommand("query-cpu-model-baseline", > + "a:modela", &modela, > + "a:modelb", &modelb, > + NULL))) > + goto cleanup; > + > + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) > + goto cleanup; > + > + /* Urgh, some QEMU architectures have query-cpu-model-baseline > + * command but return 'GenericError' with string "Not supported", > + * instead of simply omitting the command entirely > + */ > + if (qemuMonitorJSONHasError(reply, "GenericError")) { > + ret = 0; So this won't be an error then? Or is this because you're calling without checking whether or not the capability exists in the target emulator?

> + goto cleanup; > + } > + > + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) > + goto cleanup; > + > + data = virJSONValueObjectGetObject(reply, "return"); Here's where I'd add the : if (!(cpu_model = virJSONValueObjectGetObject(data, "model"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-cpu-model-baseline reply data was missing 'model'")); goto cleanup; } > + > + if (!(*model_baseline = > qemuMonitorJSONBuildCPUModelInfoFromJSON(data))) > + goto cleanup; and pass the cpu_model here John > + > + ret = 0; > + > + cleanup: > + virJSONValueFree(cmd); > + virJSONValueFree(reply); > + virJSONValueFree(modela); > + virJSONValueFree(modelb);> + > + return ret; > +} > > int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, > char ***commands) > diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h > index 045df4919..aa6f3582e 100644 > --- a/src/qemu/qemu_monitor_json.h > +++ b/src/qemu/qemu_monitor_json.h > @@ -361,6 +361,13 @@ int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr > mon, > qemuMonitorCPUModelInfoPtr > *model_info) > ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(5); > > +int qemuMonitorJSONGetCPUModelBaseline(qemuMonitorPtr mon, > + qemuMonitorCPUModelInfoPtr model_a, > + qemuMonitorCPUModelInfoPtr model_b, > + qemuMonitorCPUModelInfoPtr > *model_baseline) > + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4); > + > + > int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, > char ***commands) > ATTRIBUTE_NONNULL(2); > -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list