On Wed, Apr 17, 2019 at 07:41:23AM +0200, Markus Armbruster wrote: > Eduardo Habkost <ehabk...@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 <ehabk...@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().
Your suggestions make sense. I didn't notice how close lookup_cpu_class() was to the declaration of cpu_class_by_name(). I was seeing lookup_cpu_class() as "this portion of parse_cpu_option() I need", and not as "a wrapper to cpu_class_by_name()". What about: /** * arch_cpu_class_by_name: * @cpu_model: CPU model name * * Arch-specific wrapper around cpu_class_by_name(), uses CPU_RESOLVING_TYPE * for the current architecture. */ CPUClass *arch_cpu_class_by_name(const char *cpu_model, Error **errp); -- Eduardo
