[libvirt] [PATCHv2 0/3] query-cpu-model-baseline QMP command

Implementation of libvirt functions to support the QEMU query-cpu-model-baseline QMP command. This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999 Signed-off-by: Chris Venteicher <cventeic@redhat.com> diff to v1: - Replaced c++ style comments with c style - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} use To/From form - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} return pointers not integers - VIR_STEAL_PTR form used - switch statement uses virReportEnumRangeError - qemuMonitorJSONHasError(reply, "GenericError") treated as no info, not fatal error - virJSONValueFree(cpu_props) during error case Chris Venteicher (3): qemu_monitor_json: Populate CPUModelInfo struct from json qemu_monitor_json: Build Json CPU Model Info qemu_monitor: query-cpu-model-baseline QMP command src/qemu/qemu_monitor.c | 13 ++++ src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 175 +++++++++++++++++++++++++++++++++++++------ src/qemu/qemu_monitor_json.h | 7 ++ 4 files changed, 178 insertions(+), 22 deletions(-) -- 2.14.1

New function qemuMonitorJSONBuildCPUModelInfoFromJSON created by extracting code from existing function qemuMonitorJSONGetCPUModelExpansion to create a reusable function for extracting cpu model info from json. Function qemuMonitorJSONGetCPUModelInfoFromJSON returns qemuMonitorCPUModelInfoPtr on success and NULL on failure. --- src/qemu/qemu_monitor_json.c | 78 +++++++++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 57c2c4de0..4368aaaa0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5337,6 +5337,57 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, return 0; } +/* model_json: {"model": {"name": "IvyBridge", "props": {}}} + */ +static qemuMonitorCPUModelInfoPtr +qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr model_json) +{ + virJSONValuePtr cpu_model; + virJSONValuePtr cpu_props; + qemuMonitorCPUModelInfoPtr machine_model = NULL; + qemuMonitorCPUModelInfoPtr model = NULL; + char const *cpu_name; + + if (!(cpu_model = virJSONValueObjectGetObject(model_json, "model"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'model'")); + goto cleanup; + } + + if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'name'")); + goto cleanup; + } + + if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'props'")); + goto cleanup; + } + + if (VIR_ALLOC(machine_model) < 0) + goto cleanup; + + if (VIR_STRDUP(machine_model->name, cpu_name) < 0) + goto cleanup; + + if (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props)) < 0) + goto cleanup; + + if (virJSONValueObjectForeachKeyValue(cpu_props, + qemuMonitorJSONParseCPUModelProperty, + machine_model) < 0) + goto cleanup; + + VIR_STEAL_PTR(model, machine_model); + + cleanup: + qemuMonitorCPUModelInfoFree(machine_model); + + return model; +} + int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, @@ -5351,9 +5402,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, virJSONValuePtr reply = NULL; virJSONValuePtr data; virJSONValuePtr cpu_model; - virJSONValuePtr cpu_props; qemuMonitorCPUModelInfoPtr machine_model = NULL; - char const *cpu_name; const char *typeStr = ""; *model_info = NULL; @@ -5426,30 +5475,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, goto retry; } - if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'name'")); - goto cleanup; - } - - if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'props'")); - goto cleanup; - } - - if (VIR_ALLOC(machine_model) < 0) - goto cleanup; - - if (VIR_STRDUP(machine_model->name, cpu_name) < 0) - goto cleanup; - - if (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props)) < 0) - goto cleanup; - - if (virJSONValueObjectForeachKeyValue(cpu_props, - qemuMonitorJSONParseCPUModelProperty, - machine_model) < 0) + if (!(machine_model = qemuMonitorJSONBuildCPUModelInfoFromJSON(data))) goto cleanup; ret = 0; -- 2.14.1

On 04/19/2018 12:06 AM, Chris Venteicher wrote:
New function qemuMonitorJSONBuildCPUModelInfoFromJSON created by extracting code from existing function qemuMonitorJSONGetCPUModelExpansion to create a reusable function for extracting cpu model info from json.
Function qemuMonitorJSONGetCPUModelInfoFromJSON returns qemuMonitorCPUModelInfoPtr on success and NULL on failure. --- src/qemu/qemu_monitor_json.c | 78 +++++++++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 57c2c4de0..4368aaaa0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5337,6 +5337,57 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, return 0; }
+/* model_json: {"model": {"name": "IvyBridge", "props": {}}} + */ +static qemuMonitorCPUModelInfoPtr +qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr model_json) +{ + virJSONValuePtr cpu_model; + virJSONValuePtr cpu_props; + qemuMonitorCPUModelInfoPtr machine_model = NULL; + qemuMonitorCPUModelInfoPtr model = NULL; + char const *cpu_name; + + if (!(cpu_model = virJSONValueObjectGetObject(model_json, "model"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'model'")); + goto cleanup; + }
Hmm... since you use this helper within functions other than expansion, you'll need a more generic error message here. or... this function could take an additional char *parameter that contains the name of the qmp command and you can append this to the error messages.
+ + if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'name'")); + goto cleanup; + } + + if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'props'")); + goto cleanup; + } + + if (VIR_ALLOC(machine_model) < 0) + goto cleanup; + + if (VIR_STRDUP(machine_model->name, cpu_name) < 0) + goto cleanup; + + if (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props)) < 0) + goto cleanup; + + if (virJSONValueObjectForeachKeyValue(cpu_props, + qemuMonitorJSONParseCPUModelProperty, + machine_model) < 0) + goto cleanup; + + VIR_STEAL_PTR(model, machine_model); + + cleanup: + qemuMonitorCPUModelInfoFree(machine_model); + + return model; +} + int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, @@ -5351,9 +5402,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, virJSONValuePtr reply = NULL; virJSONValuePtr data; virJSONValuePtr cpu_model; - virJSONValuePtr cpu_props; qemuMonitorCPUModelInfoPtr machine_model = NULL; - char const *cpu_name; const char *typeStr = "";
*model_info = NULL; @@ -5426,30 +5475,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, goto retry; }
- if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'name'")); - goto cleanup; - } - - if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'props'")); - goto cleanup; - } - - if (VIR_ALLOC(machine_model) < 0) - goto cleanup; - - if (VIR_STRDUP(machine_model->name, cpu_name) < 0) - goto cleanup; - - if (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props)) < 0) - goto cleanup; - - if (virJSONValueObjectForeachKeyValue(cpu_props, - qemuMonitorJSONParseCPUModelProperty, - machine_model) < 0) + if (!(machine_model = qemuMonitorJSONBuildCPUModelInfoFromJSON(data))) goto cleanup;
ret = 0;
Not super important at all, but: Since you're in the neighborhood, I think it would be helpful to clean up qemuMonitorJSONGetCPUModelExpansion a tiny bit and do a VIR_STEAL_PTR for model_info and machine_model here. It should also be safe to remove the free on machine_model here, since both the allocation and freeing is now handled within your Build function. -- Respectfully, - Collin Walling

$SUBJ: "qemu_monitor_json: Introduce qemuMonitorJSONBuildCPUModelInfoFromJSON" On 04/19/2018 12:06 AM, Chris Venteicher wrote:
New function qemuMonitorJSONBuildCPUModelInfoFromJSON created by extracting code from existing function qemuMonitorJSONGetCPUModelExpansion to create a reusable function for extracting cpu model info from json.
Function qemuMonitorJSONGetCPUModelInfoFromJSON returns qemuMonitorCPUModelInfoPtr on success and NULL on failure.
Then in here, just indicate: Extract cpu_model parsing code from qemuMonitorJSONGetCPUModelExpansion into a separate helper for future reuse.
--- src/qemu/qemu_monitor_json.c | 78 +++++++++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 57c2c4de0..4368aaaa0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5337,6 +5337,57 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, return 0; }
Use 2 blank/empty lines between functions - mostly a readability thing.
+/* model_json: {"model": {"name": "IvyBridge", "props": {}}} + */ +static qemuMonitorCPUModelInfoPtr +qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr model_json)
The '_json' is probably redundant.
+{ + virJSONValuePtr cpu_model; + virJSONValuePtr cpu_props; + qemuMonitorCPUModelInfoPtr machine_model = NULL; + qemuMonitorCPUModelInfoPtr model = NULL; + char const *cpu_name; + + if (!(cpu_model = virJSONValueObjectGetObject(model_json, "model"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'model'"));
As a different suggestion from Collin... Since qemuMonitorJSONGetCPUModelExpansion already fetches @cpu_model, why not just pass virJSONValuePtr cpu_model. Then in patch3, fetch cpu_model but use the error for the specific command. then...
+ goto cleanup; + } + + if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'name'"));
s/query-cpu-model-expansion/cpu_model/
+ goto cleanup; + } + + if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'props'"));
s/query-cpu-model-expansion/cpu_model/
+ goto cleanup; + } + + if (VIR_ALLOC(machine_model) < 0) + goto cleanup; + + if (VIR_STRDUP(machine_model->name, cpu_name) < 0) + goto cleanup; + + if (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props)) < 0) + goto cleanup; + + if (virJSONValueObjectForeachKeyValue(cpu_props, + qemuMonitorJSONParseCPUModelProperty, + machine_model) < 0) + goto cleanup; + + VIR_STEAL_PTR(model, machine_model); + + cleanup: + qemuMonitorCPUModelInfoFree(machine_model); + + return model; +} + int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, @@ -5351,9 +5402,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, virJSONValuePtr reply = NULL; virJSONValuePtr data; virJSONValuePtr cpu_model; - virJSONValuePtr cpu_props; qemuMonitorCPUModelInfoPtr machine_model = NULL; - char const *cpu_name; const char *typeStr = "";
*model_info = NULL; @@ -5426,30 +5475,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, goto retry; }
- if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'name'")); - goto cleanup; - } - - if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'props'")); - goto cleanup; - } - - if (VIR_ALLOC(machine_model) < 0) - goto cleanup; - - if (VIR_STRDUP(machine_model->name, cpu_name) < 0) - goto cleanup; - - if (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props)) < 0) - goto cleanup; - - if (virJSONValueObjectForeachKeyValue(cpu_props, - qemuMonitorJSONParseCPUModelProperty, - machine_model) < 0) + if (!(machine_model = qemuMonitorJSONBuildCPUModelInfoFromJSON(data))) goto cleanup;
ret = 0;
Feel free to do the VIR_STEAL_PTR(*model_info, machine_model) or if (!(*machine_info = ...())) and get rid of machine_model from here. John

Function qemuMonitorJSONBuildCPUModelInfoToJSON builds and returns JSON of form {"model": {"name": "IvyBridge", "props": {}}} from qemuMonitorCPUModelInfo. Function qemuMonitorJSONBuildCPUModelInfoToJSON returns virJsonValuePtr on success and NULL on failure. --- src/qemu/qemu_monitor_json.c | 48 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 4368aaaa0..44c1b2f15 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5337,6 +5337,54 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, return 0; } +/* model_json: {"model": {"name": "IvyBridge", "props": {}}} + */ +static virJSONValuePtr +qemuMonitorJSONBuildCPUModelInfoToJSON(qemuMonitorCPUModelInfoPtr model) +{ + virJSONValuePtr cpu_props = NULL; + virJSONValuePtr model_json = NULL; + size_t i; + + if (!(cpu_props = virJSONValueNewObject())) + goto cleanup; + + for (i = 0; i < model->nprops; i++) { + qemuMonitorCPUPropertyPtr prop = model->props + i; + + switch (prop->type) { + case QEMU_MONITOR_CPU_PROPERTY_BOOLEAN: + if (virJSONValueObjectAppendBoolean(cpu_props, prop->name, prop->value.boolean) < 0) + goto cleanup; + break; + + case QEMU_MONITOR_CPU_PROPERTY_STRING: + if (virJSONValueObjectAppendString(cpu_props, prop->name, prop->value.string) < 0) + goto cleanup; + break; + + case QEMU_MONITOR_CPU_PROPERTY_NUMBER: + if (virJSONValueObjectAppendNumberLong(cpu_props, prop->name, prop->value.number) < 0) + goto cleanup; + break; + + case QEMU_MONITOR_CPU_PROPERTY_LAST: + default: + virReportEnumRangeError(qemuMonitorCPUPropertyPtr, prop->type); + goto cleanup; + } + } + + if (virJSONValueObjectCreate(&model_json, "s:name", model->name, + "a:props", &cpu_props, NULL) < 0) { + virJSONValueFree(cpu_props); + goto cleanup; + } + + cleanup: + return model_json; +} + /* model_json: {"model": {"name": "IvyBridge", "props": {}}} */ static qemuMonitorCPUModelInfoPtr -- 2.14.1

$SUBJ: qemu_monitor_json: Introduce qemuMonitorJSONBuildCPUModelInfoToJSON The corollary for qemuMonitorJSONBuildCPUModelInfoFromJSON is to build the JSON data from the qemuMonitorCPUModelInfoPtr. On 04/19/2018 12:06 AM, Chris Venteicher wrote:
Function qemuMonitorJSONBuildCPUModelInfoToJSON builds and returns JSON of form {"model": {"name": "IvyBridge", "props": {}}} from qemuMonitorCPUModelInfo.
Function qemuMonitorJSONBuildCPUModelInfoToJSON returns virJsonValuePtr on success and NULL on failure. --- src/qemu/qemu_monitor_json.c | 48 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 4368aaaa0..44c1b2f15 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5337,6 +5337,54 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, return 0; }
Again 2 blank lines (at end too).
+/* model_json: {"model": {"name": "IvyBridge", "props": {}}} + */ +static virJSONValuePtr +qemuMonitorJSONBuildCPUModelInfoToJSON(qemuMonitorCPUModelInfoPtr model) +{ + virJSONValuePtr cpu_props = NULL; + virJSONValuePtr model_json = NULL; + size_t i; + + if (!(cpu_props = virJSONValueNewObject())) + goto cleanup; + + for (i = 0; i < model->nprops; i++) { + qemuMonitorCPUPropertyPtr prop = model->props + i;
or prop = model->props[i] - something I'm more used to seeing.
+ + switch (prop->type) { + case QEMU_MONITOR_CPU_PROPERTY_BOOLEAN: + if (virJSONValueObjectAppendBoolean(cpu_props, prop->name, prop->value.boolean) < 0)
Long line - consider creating line break after prop->name Prefer to keep lines < 80 if possible unless it's just a character or two.
+ goto cleanup; + break; + + case QEMU_MONITOR_CPU_PROPERTY_STRING: + if (virJSONValueObjectAppendString(cpu_props, prop->name, prop->value.string) < 0) + goto cleanup; + break; + + case QEMU_MONITOR_CPU_PROPERTY_NUMBER: + if (virJSONValueObjectAppendNumberLong(cpu_props, prop->name, prop->value.number) < 0) + goto cleanup; + break; + + case QEMU_MONITOR_CPU_PROPERTY_LAST: + default: + virReportEnumRangeError(qemuMonitorCPUPropertyPtr, prop->type); + goto cleanup; + } + } + + if (virJSONValueObjectCreate(&model_json, "s:name", model->name, + "a:props", &cpu_props, NULL) < 0) { + virJSONValueFree(cpu_props); + goto cleanup; + }
I just know the above will cause Coverity to get all upset, but I think if you go with ignore_value(virJSONValueObjectCreate(&model_json, "s:name", model->name, "a:props", &cpu_props, NULL)); cleanup: virJSONValueFree(cpu_props); return model_json; Then things will be fine. In the long run since @model_json is the returned thing and it'd only be non NULL if virJSONValueObjectCreate succeeded and it woudl consume @cpu_props properly, thus we don't care about the return value because it's not changing what we do. But not having that @cpu_props Free makes coverity believe it's leaked even though it's passed by reference. John
+ + cleanup: + return model_json; +} + /* model_json: {"model": {"name": "IvyBridge", "props": {}}} */ static qemuMonitorCPUModelInfoPtr

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; } +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); +} 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; + goto cleanup; + } + + if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_OBJECT) < 0) + goto cleanup; + + data = virJSONValueObjectGetObject(reply, "return"); + + if (!(*model_baseline = qemuMonitorJSONBuildCPUModelInfoFromJSON(data))) + goto cleanup; + + 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); -- 2.14.1

$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);

Please see responses below. New patchset with all recommended changes will be sent soon. Chris ----- Original Message -----
From: "John Ferlan" <jferlan@redhat.com> To: "Chris Venteicher" <cventeic@redhat.com>, libvir-list@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.
The consumer is in a future patch. Want to get some early feedback on these changes.
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?
A goal is to gracefully handle different levels of query-cpu-model-baseline support as it's only supported by S390 now but other archs will likely be added in the future. Some archs will likely never be supported. Seems like there are scenarios where we would fall back to code in libvirt to provide an answer in cases where query-cpu-model-baseline is not able to answer. With that said, I am primarily following the pattern of checking for "GenericError" that has been established in the code for similar QMP requests. The check might not be needed but I am pretty sure it's not harmful. Please let me know if it's not making sense. For reference, I hacked qemu to generate responses on X86 for testing which involved making sure qemu did not unregister the command. Perhaps that might be useful info. chris@cv1:~/libvirt/qemu_git$ git diff monitor.c diff --git a/monitor.c b/monitor.c index a4417f26cd..760d6ada00 100644 --- a/monitor.c +++ b/monitor.c @@ -994,8 +994,8 @@ static void qmp_unregister_commands_hack(void) qmp_unregister_command(&qmp_commands, "query-cpu-model-expansion"); #endif #if !defined(TARGET_S390X) - qmp_unregister_command(&qmp_commands, "query-cpu-model-baseline"); - qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison"); +// qmp_unregister_command(&qmp_commands, "query-cpu-model-baseline"); +// qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison"); #endif #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \ && !defined(TARGET_S390X)
+ 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 04/30/2018 01:25 PM, Chris Venteicher wrote:
Please see responses below.
New patchset with all recommended changes will be sent soon.
Chris
----- Original Message -----
From: "John Ferlan" <jferlan@redhat.com> To: "Chris Venteicher" <cventeic@redhat.com>, libvir-list@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.
The consumer is in a future patch. Want to get some early feedback on these changes.
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?
A goal is to gracefully handle different levels of query-cpu-model-baseline support as it's only supported by S390 now but other archs will likely be added in the future. Some archs will likely never be supported.
Seems like there are scenarios where we would fall back to code in libvirt to provide an answer in cases where query-cpu-model-baseline is not able to answer.
Please be sure to add comments prior to the function to describe the input, output, and return values. One thing we've done with some commands which need to fallback to some other command is use different error codes so the caller makes the determination... Sometimes it's 1, 0, -1, while others it's 0, -1, -2 Perhaps it's just a matter of describing expectations of usage. If the caller sees a 0 return, then it would also have to check if *model_baseline != NULL in order to make an intelligent decision. Or it could just use the return value and go from there. Your call, since the consumer wasn't posted - I wanted to be sure to ask and make it a known... John
With that said, I am primarily following the pattern of checking for "GenericError" that has been established in the code for similar QMP requests.
The check might not be needed but I am pretty sure it's not harmful.
Please let me know if it's not making sense.
For reference, I hacked qemu to generate responses on X86 for testing which involved making sure qemu did not unregister the command. Perhaps that might be useful info.
chris@cv1:~/libvirt/qemu_git$ git diff monitor.c diff --git a/monitor.c b/monitor.c index a4417f26cd..760d6ada00 100644 --- a/monitor.c +++ b/monitor.c @@ -994,8 +994,8 @@ static void qmp_unregister_commands_hack(void) qmp_unregister_command(&qmp_commands, "query-cpu-model-expansion"); #endif #if !defined(TARGET_S390X) - qmp_unregister_command(&qmp_commands, "query-cpu-model-baseline"); - qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison"); +// qmp_unregister_command(&qmp_commands, "query-cpu-model-baseline"); +// qmp_unregister_command(&qmp_commands, "query-cpu-model-comparison"); #endif #if !defined(TARGET_PPC) && !defined(TARGET_ARM) && !defined(TARGET_I386) \ && !defined(TARGET_S390X)
+ 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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 04/19/2018 12:06 AM, Chris Venteicher wrote:
Implementation of libvirt functions to support the QEMU query-cpu-model-baseline QMP command.
This is part of resolution of: https://bugzilla.redhat.com/show_bug.cgi?id=1511999
Signed-off-by: Chris Venteicher <cventeic@redhat.com>
diff to v1: - Replaced c++ style comments with c style - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} use To/From form - qemuMonitorJSONGetCPUModelInfo{ToJSON,FromJSON} return pointers not integers - VIR_STEAL_PTR form used - switch statement uses virReportEnumRangeError - qemuMonitorJSONHasError(reply, "GenericError") treated as no info, not fatal error - virJSONValueFree(cpu_props) during error case
Chris Venteicher (3): qemu_monitor_json: Populate CPUModelInfo struct from json qemu_monitor_json: Build Json CPU Model Info qemu_monitor: query-cpu-model-baseline QMP command
src/qemu/qemu_monitor.c | 13 ++++ src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 175 +++++++++++++++++++++++++++++++++++++------ src/qemu/qemu_monitor_json.h | 7 ++ 4 files changed, 178 insertions(+), 22 deletions(-)
Please CC me on future posts so they grab my attention easier :) -- Respectfully, - Collin Walling
participants (3)
-
Chris Venteicher
-
Collin Walling
-
John Ferlan