On 4/26/19 5:22 PM, walling@xxxxxxxxxxxxx wrote:
From: Collin Walling<walling@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxx>
Reviewed-by: Bjoern Walk<bwalk@xxxxxxxxxxxxx>
---
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;
}
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
