Re: [Qemu-devel] [qom-cpu PATCH 7/7 v9] target-i386: CPU model subclasses

[Andreas Färber]

Am 10.02.2014 11:21, schrieb Eduardo Habkost:
> +static const TypeInfo x86_cpu_host_type_info = {
> +    .name = CPU_CLASS_NAME("host"),
> +    .parent = TYPE_X86_CPU,
> +    .instance_size = sizeof(X86CPU),
> +    .instance_init = x86_cpu_instance_init_host,
> +    .abstract = false,
> +    .class_size = sizeof(X86CPUClass),
> +    .class_init = x86_cpu_class_init_host,
> +};

This looks broken: .class_data is not set but the loading of the cpudef
happens in the TYPE_X86_CPU initfn. My preferred solution would be to
move the cpudef-loading from TYPE_X86_CPU's instance_init to a separate
one specified for the models only, allowing non-cpudef-based models. Not
finished investigating yet.

For now I've prepended a patch implementing my generalized
CPUClass::class_by_name instead of a custom x86_cpu_class_by_name().

Other style nits that I'm working on cleaning up are declarations in the
middle of blocks, keeping _class_init naming convention (pretty sure my
patches always had the most-specific-to-least-specific naming), strictly
distinguishing between type and class, adding to my gtk-style
documentation rather than new custom comments, placing struct
documentation in the header and keeping the diff nicely readable AFAP.
I'd further like to keep some other conventions from previous CPU
subclasses, like pulling the model for loop out of the model
registration function.

My patches had always tried to turn what is now x86_cpu_load_def() into
an instance_init function rather than calling it from one - did you have
reasons not to?

Did you consider converting the host model in a first step to make the
patch smaller?

I'd rather finish my investigations and discuss my v10 patches but Paolo
is already asking whether Eduardo should send a PULL, so here's my
textual reply informing of some thoughts and WIP. ;)

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg