On 4/26/19 5:22 PM, walling@linux.ibm.com wrote:
From: Collin Walling<walling@linux.ibm.com>
Refactor some code in qemuMonitorJSONGetCPUModelExpansion to be later used for the comparison and baseline functions.
This does not alter any functionality.
Signed-off-by: Collin Walling<walling@linux.ibm.com> Reviewed-by: Bjoern Walk<bwalk@linux.ibm.com> --- src/qemu/qemu_monitor_json.c | 170 ++++++++++++++++++++++++++++++------------- 1 file changed, 118 insertions(+), 52 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 908967f..9618d8e 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5513,6 +5513,118 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, return 0; }
+ +static virJSONValuePtr +qemuMonitorJSONMakeCPUModel(const char *model_name, + size_t nprops, + virCPUFeatureDefPtr props, + bool migratable) +{ + virJSONValuePtr value; + virJSONValuePtr feats = NULL; + size_t i; + + if (!(value = virJSONValueNewObject())) + goto cleanup; + + if (virJSONValueObjectAppendString(value, "name", model_name) < 0) + goto cleanup; + + if (nprops || !migratable) { + if (!(feats = virJSONValueNewObject())) + goto cleanup; + + for (i = 0; i < nprops; i++) { + char *name = props[i].name; + bool enabled = false; + + if (props[i].policy == VIR_CPU_FEATURE_REQUIRE || + props[i].policy == VIR_CPU_FEATURE_FORCE) + enabled = true; + + if (virJSONValueObjectAppendBoolean(feats, name, enabled) < 0) + goto cleanup; + } + + if (!migratable) { + if (virJSONValueObjectAppendBoolean(feats, "migratable", false) < 0) + goto cleanup;
Missing indentation of 'goto cleanup' after the if clause.
+ } + + if (virJSONValueObjectAppend(value, "props", feats) < 0) + goto cleanup; + } + + return value; + + cleanup: + virJSONValueFree(value); + virJSONValueFree(feats); + return NULL; +} + + +static int +qemuMonitorJSONParseCPUModelData(virJSONValuePtr data, + virJSONValuePtr *cpu_model, + virJSONValuePtr *cpu_props, + const char **cpu_name) +{ + if (!(*cpu_model = virJSONValueObjectGetObject(data, "model"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QMP command reply data was missing 'model'")); + return -1; + } + + if (!(*cpu_name = virJSONValueObjectGetString(*cpu_model, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QMP command reply data was missing 'name'")); + return -1; + } + + if (!(*cpu_props = virJSONValueObjectGetObject(*cpu_model, "props"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("QMP command reply data was missing 'props'")); + return -1; + } + + return 0; +} + + +static int +qemuMonitorJSONParseCPUModel(const char *cpu_name, + virJSONValuePtr cpu_props, + qemuMonitorCPUModelInfoPtr *model_info) +{ + qemuMonitorCPUModelInfoPtr machine_model = NULL; + int ret = -1; + + if (VIR_ALLOC(machine_model) < 0) + goto cleanup; + + if (VIR_STRDUP(machine_model->name, cpu_name) < 0) + goto cleanup; + + if (cpu_props) { + 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_info, machine_model); + ret = 0; + + cleanup: + qemuMonitorCPUModelInfoFree(machine_model); + return ret; +} + + int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, @@ -5521,33 +5633,20 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelInfoPtr *model_info) { int ret = -1; - virJSONValuePtr model = NULL; - virJSONValuePtr props = NULL; + virJSONValuePtr model; virJSONValuePtr cmd = NULL; virJSONValuePtr reply = NULL; virJSONValuePtr data; virJSONValuePtr cpu_model; - virJSONValuePtr cpu_props; - qemuMonitorCPUModelInfoPtr machine_model = NULL; - char const *cpu_name; + virJSONValuePtr cpu_props = NULL; + const char *cpu_name = ""; const char *typeStr = "";
*model_info = NULL;
- if (!(model = virJSONValueNewObject())) + if (!(model = qemuMonitorJSONMakeCPUModel(model_name, 0, NULL, migratable))) goto cleanup;
- if (virJSONValueObjectAppendString(model, "name", model_name) < 0) - goto cleanup; - - if (!migratable) { - if (!(props = virJSONValueNewObject()) || - virJSONValueObjectAppendBoolean(props, "migratable", false) < 0 || - virJSONValueObjectAppend(model, "props", props) < 0) - goto cleanup; - props = NULL; - } - retry: switch (type) { case QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC: @@ -5583,11 +5682,8 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
data = virJSONValueObjectGetObject(reply, "return");
- if (!(cpu_model = virJSONValueObjectGetObject(data, "model"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'model'")); + if (qemuMonitorJSONParseCPUModelData(data, &cpu_model, &cpu_props, &cpu_name) < 0) goto cleanup; - }
/* QEMU_MONITOR_CPU_MODEL_EXPANSION_STATIC_FULL requests "full" expansion * on the result of the initial "static" expansion. @@ -5602,42 +5698,12 @@ 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; - }
These error messages 'query-cpu-model-expansion reply data was missing ...' were replaced inside qemuMonitorJSONParseCPUModelData by error messages of the type 'QMP command reply data was missing ...'. I see that this was done because the function is used in later patches with different QMP commands, hence this change to a more generic error message. Which is fine. A suggestion here would be to pass an extra string 'qmp-command-name' to qemuMonitorJSONParseCPUModelData. Then these error messages can be tuned to show which QMP command failed to parse. This can be useful if this parse function is used multiple times with several QMP commands. Thanks, DHB
- - 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; - - ret = 0; - *model_info = machine_model; - machine_model = NULL; + ret = qemuMonitorJSONParseCPUModel(cpu_name, cpu_props, model_info);
cleanup: - qemuMonitorCPUModelInfoFree(machine_model); virJSONValueFree(cmd); virJSONValueFree(reply); virJSONValueFree(model); - virJSONValueFree(props); return ret; }