[libvirt] [PATCH v2 03/11] qemu: qmp query-cpu-model-expansion command

[Jiri Denemark]

On Fri, Dec 09, 2016 at 14:38:32 -0500, Jason J. Herne wrote:
> From: "Collin L. Walling" <walling at linux.vnet.ibm.com>
> 
> query-cpu-model-expansion is used to get a list of features for a given cpu
> model name or to get the model and features of the host hardware/environment
> as seen by Qemu/kvm.
> 
> Signed-off-by: Collin L. Walling <walling at linux.vnet.ibm.com>
> Signed-off-by: Jason J. Herne <jjherne at linux.vnet.ibm.com>
> ---
>  src/qemu/qemu_monitor.c      |  62 ++++++++++++++++++++++
>  src/qemu/qemu_monitor.h      |  22 ++++++++
>  src/qemu/qemu_monitor_json.c | 121 +++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_monitor_json.h |  12 +++++
>  4 files changed, 217 insertions(+)
> 
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 648168d..1a9665f 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -3660,6 +3660,68 @@ qemuMonitorCPUDefInfoFree(qemuMonitorCPUDefInfoPtr cpu)
...
> +qemuMonitorCPUModelInfoPtr
> +qemuMonitorCPUModelInfoCopy(const qemuMonitorCPUModelInfo *orig)
> +{
> +    qemuMonitorCPUModelInfoPtr copy;
> +    size_t i;
> +
> +    if (VIR_ALLOC(copy) < 0)
> +        goto cleanup;
> +
> +    if (VIR_ALLOC_N(copy->props, orig->nprops) < 0)
> +        goto cleanup;
> +
> +    if (VIR_STRDUP(copy->name, orig->name) < 0)
> +        goto cleanup;
> +
> +    copy->nprops = orig->nprops;
> +
> +    for (i = 0; i < orig->nprops; i++) {
> +        if (VIR_STRDUP(copy->props[i].name, orig->props[i].name) < 0)
> +            goto cleanup;
> +
> +        copy->props[i].supported = orig->props[i].supported;
> +    }
> +
> +    return copy;
> +
> + cleanup:

"error" would be a better name for this label since it is never used in
a success path.

> +    qemuMonitorCPUModelInfoFree(copy);
> +    return NULL;
> +}
> +
> +
> +int
>  qemuMonitorGetCommands(qemuMonitorPtr mon,
>                         char ***commands)
>  {
...
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 0c38b8f..9189a8b 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -4973,6 +4973,127 @@ qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
>      return ret;
>  }
>  
> +int
> +qemuMonitorJSONParseCPUModelProperty(const char *key,
> +                                     const virJSONValue *value,
> +                                     void *opaque)
> +{
> +    qemuMonitorCPUModelInfoPtr machine_model = opaque;
> +    size_t n = machine_model->nprops;
> +    bool supported;
> +
> +    if (!key) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("query-cpu-model-expansion reply data is missing a"
> +                         " property name"));
> +        return -1;
> +    }
> +    if (VIR_STRDUP(machine_model->props[n].name, key) < 0)
> +        return -1;
> +
> +    if (virJSONValueGetBoolean(virJSONValueCopy(value), &supported) < 0) {

Ouch, calling virJSONValueCopy() would just leak memory here. If we drop
the "const" requirement from virJSONValueObjectForeachKeyValue (pushed
yesterday as v2.5.0-96-gc1cb4cb9f) we can directly pass value.

> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("query-cpu-model-expansion reply data is missing a"
> +                         " feature support value"));
> +        return -1;
> +    }
> +    machine_model->props[n].supported = supported;
> +
> +    machine_model->nprops++;
> +    return 0;
> +}
> +
> +int
> +qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
> +                                    const char *type,
> +                                    const char *model_name,
> +                                    qemuMonitorCPUModelInfoPtr *model_info)
> +{
...
> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> +        goto cleanup;
> +
> +    if (!(data = virJSONValueObjectGetObject(reply, "return"))) {

data can never be NULL here because qemuMonitorJSONCheckError() returned
success.

> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("query-cpu-model-expansion reply data was missing return"));
> +        goto cleanup;
> +    }
...
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index adff0c3..10e4955 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -352,6 +352,18 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon,
>                                       qemuMonitorCPUDefInfoPtr **cpus)
>      ATTRIBUTE_NONNULL(2);
>  
> +int
> +qemuMonitorJSONParseCPUModelProperty(const char *key,
> +                                     const virJSONValue *value,
> +                                     void *opaque)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

There's no reason for exporting this function outside
qemu_monitor_json.c

> +
> +int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon,
> +                                        const char *type,
> +                                        const char *model_name,
> +                                        qemuMonitorCPUModelInfoPtr *model_info)
> +    ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
> +
>  int qemuMonitorJSONGetCommands(qemuMonitorPtr mon,
>                                 char ***commands)
>      ATTRIBUTE_NONNULL(2);

ACK with the suggested changes

Jirka