Re: [PATCH v6] qapi: introduce 'query-cpu-model-cpuid' action

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

20.04.2021 19:19, Valeriy Vdovin wrote:
> Introducing new qapi method 'query-cpu-model-cpuid'. This method can be used to
> get virtualized cpu model info generated by QEMU during VM initialization in
> the form of cpuid representation.
> 

[..]

> +CpuModelCpuidDescription *qmp_query_cpu_model_cpuid(Error **errp)
> +{
> +    MachineState *ms = MACHINE(qdev_get_machine());
> +    const char *class_name;
> +    CpuModelCpuidDescription *info;
> +    Object *cpu;
> +    char *model_id, *vendor;
> +
> +    /*
> +     * Method requires initialized machine and cpu
> +     */
> +    if (!ms || !ms->possible_cpus) {
> +      error_setg(errp, "Nothing to report");
> +      return NULL;

indentation should be fixed to 4+4=8 spaces totally, as patchew already said.

> +    }
> +
> +    cpu = ms->possible_cpus->cpus[0].cpu;
> +
> +    class_name = object_class_get_name(object_get_class(cpu));
> +    model_id = object_property_get_str(cpu, "model-id", errp);
> +    if (!model_id) {
> +        error_setg(errp, "'model-id' property not found");

object_property_get_str has errp argument, so it should care to set it on error path. You shouldn't call error_setg by hand here, it will crash (trying to set errp which is already set).

> +        return NULL;
> +    }
> +    vendor = object_property_get_str(cpu, "vendor", errp);
> +    if (!vendor) {
> +        error_setg(errp, "'vendor' property not found");

and here

> +        return NULL;
> +    }
> +
> +    info = g_malloc0(sizeof(*info));
> +    info->model_id = g_strdup(model_id);
> +    info->vendor = g_strdup(vendor);
> +    info->class_name = g_strdup(class_name);
> +    cpu_model_fill_cpuid(cpu, info);
> +
> +    return info;
> +}
> +

With fixed style and dropped extra error_setg() calls:

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>


-- 
Best regards,
Vladimir