Re: [Qemu-devel] [PATCH 2/5] cpu: Extract CPU class lookup from parse_cpu_option()

[Markus Armbruster <armbru@redhat.com>]

Eduardo Habkost <ehabkost@redhat.com> writes:

> The new function will be useful in user mode, when we already
> have a CPU model and don't need to parse any extra options.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  include/qom/cpu.h |  9 +++++++++
>  exec.c            | 22 ++++++++++++----------
>  2 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index d28c690b27..e11b14d9ac 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -699,6 +699,15 @@ CPUState *cpu_create(const char *typename);
>   */
>  const char *parse_cpu_option(const char *cpu_option);
>  
> +/**
> + * lookup_cpu_class:
> + * @cpu_model: CPU model name
> + *
> + * Look up CPU class corresponding to a given CPU model name.
> + */
> +CPUClass *lookup_cpu_class(const char *cpu_model, Error **errp);

Nitpicks, feel free to ignore.

Naming: lookup_cpu_class() makes my reading circuits stall momentarily,
because lookup is a noun. The verb is spelled look up.  No stall:
cpu_class_lookup(), look_up_cpu_class(), cpu_class_by_name_err(),
cpu_class_parse(), ...

Doc string: this is a wrapper around cpu_class_by_name().  Perhaps that
should be spelled out.

Apropos cpu_class_by_name(): it returns ObjectClass, not CPUClass.
Isn't that odd?

Position: If you put it before cpu_create() rather than after, it's next
to the function it wraps.  But perhaps you prefer to keep it next to
parse_cpu_option().

[...]