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