Re: [libvirt] [RFC qom-cpu v2 1/2] target-i386: Convert CPU definitions into X86CPU subclasses
On Tue, Jan 15, 2013 at 09:41:04AM +0100, Igor Mammedov wrote:
On Mon, 10 Dec 2012 23:59:31 +0100
Andreas Färber <afaerber(a)suse.de> wrote:
> TODO: sort classes for -cpu ?, generalize X86CPUListState, more testing
>
> Signed-off-by: Andreas Färber <afaerber(a)suse.de>
> Cc: Eduardo Habkost <ehabkost(a)redhat.com>
> Cc: Igor Mammedov <imammedo(a)redhat.com>
[...]
The patch is just renaming of the current builtin_x86_defs
into a bunch of functions and polluting X86CPUClass
with fields from the former x86_def_t.
object_new() still creates a dummy cpu instance whose defaults
are still manually copied from X86CPUClass instead of x86_def_t.
That's a good thing, isn't it? It means the patch is easier to review.
:-)
No patch alone will do everything we want, because we want to do a lot.
We need to do it one step at a time.
(BTW, why are you looking at this RFC instead of the more recent one,
that I have sent on Jan 4? [that's very similar to this one])
What's the point in having dummy sub-classes?
How it can help in your CPU re-factoring?
It will help us to unify the CPU creation/realization code that's
duplicated over all the architectures.
It will give libvirt an easy mechanism to list the available CPU models
that won't require parsing help output (using "qom-list-types" QMP
command).
On the other hand converting features to static properties first and
then converting X86CPU to sub-classes, yields already initialized to
defaults sub-class completely removing notion of x86_def_t and
not polluting X86CPUClass with redundant fields.
I wouldn't disagree with this approach in principle, but I believe our
main problem today is lack of reviewer bandwidth. I learned the hard way
that trying to clean up everything before implementing something will
make sure the code takes forever to be reviewed.
For example see following patches on
https://github.com/imammedo/qemu/commits/x86-cpu-classes.Jan142013
e9fd18f qdev: extend DEFINE_GENERIC_PROP() to support default values
c65eca9 qdev: make qdev_prop_find_bit return non const so prop default value could be
modified
0311952 allow to expolit default value static props for model, family, stepping &
vendor props
8b3080e target-i386: add helpers to change default values of static properties before
object is created
ed506d3 target-i386: prepare for subclasses to have its own instance of static properties
definitions
a48e252 target-i386: declare subclass for qemu64 cpu model
9c556c2 target-i386: move cpu_x86_init() & cpu_x86_register() into it and switch to
subclasses. PS: implemended only for qemu64
f5dbfe6 CPU_CLASS_NAME(qemu64) hack
00e15b8 target-i386: properties list are per subclass: do not set them in superclass to
avoid defaults set by subclass be over-written
--
Regards,
Igor
--
Eduardo