On Sun, Dec 02, 2018 at 23:10:18 -0600, Chris Venteicher wrote:
Conversion functions are used convert CPUModelInfo structs into QMP JSON and the reverse.
QMP JSON is of form: {"model": {"name": "IvyBridge", "props": {}}}
qemuMonitorCPUModelInfoBoolPropAdd is used to add boolean properties to CPUModelInfo struct.
qemuMonitorJSONGetCPUModelExpansion makes full use of conversions and propAdd in prep to support input of full cpu model in future.
I think this patch is doing too much at once and it's quite easy to get lost between the FromJSON and ToJSON converters. Introducing each converter in a separate patch would be better.
Signed-off-by: Chris Venteicher <cventeic@redhat.com> --- src/qemu/qemu_monitor.c | 24 ++++++ src/qemu/qemu_monitor.h | 5 ++ src/qemu/qemu_monitor_json.c | 154 +++++++++++++++++++++++++---------- 3 files changed, 138 insertions(+), 45 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 5effc74736..ddf4d96799 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -3764,6 +3764,30 @@ qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig) }
+int +qemuMonitorCPUModelInfoBoolPropAdd(qemuMonitorCPUModelInfoPtr model, + const char *prop_name, + bool prop_value)
It looks a bit strange to have this wrapper for a code that would only be in one place...
+{ + int ret = -1; + qemuMonitorCPUProperty prop;
Anyway, either put an empty line here or change this to qemuMonitorCPUProperty prop = { .type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN, .value.boolean = prop_value };
+ prop.type = QEMU_MONITOR_CPU_PROPERTY_BOOLEAN; + prop.value.boolean = prop_value; + + if (VIR_STRDUP(prop.name, prop_name) < 0) + goto cleanup; + + if (VIR_APPEND_ELEMENT(model->props, model->nprops, prop) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(prop.name); + return ret; +} + + int qemuMonitorGetCommands(qemuMonitorPtr mon, char ***commands) ... diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 5a806f6c0e..abfa4155ee 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5505,6 +5505,101 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, ... +static qemuMonitorCPUModelInfoPtr +qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr cpu_model) +{ + virJSONValuePtr cpu_props; + qemuMonitorCPUModelInfoPtr model = NULL; + qemuMonitorCPUModelInfoPtr ret = NULL; + char const *cpu_name; + + if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Parsed JSON reply missing 'name'")); + goto cleanup; + } + + if (VIR_ALLOC(model) < 0) + goto cleanup; + + if (VIR_STRDUP(model->name, cpu_name) < 0) + goto cleanup; + + if ((cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) { + if (VIR_ALLOC_N(model->props, + virJSONValueObjectKeysNumber(cpu_props)) < 0) + goto cleanup; + + if (virJSONValueObjectForeachKeyValue(cpu_props, + qemuMonitorJSONParseCPUModelProperty, + model) < 0) + goto cleanup; + }
The patch looks like a refactoring, but it actually changes a bit more. Previously "props" was required, while now it is silently ignored if missing. Perhaps it's a valid change, but it should not be hidden inside a refactoring patch.
+ + VIR_STEAL_PTR(ret, model); + + cleanup: + qemuMonitorCPUModelInfoFree(model); + + return ret; +} + int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, ...
Jirka