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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list