Re: [Qemu-devel] [RFC PATCH v1 0/7] QOM Super class access

[Peter Crosthwaite]

Hi Michael,

On Tue, Jun 18, 2013 at 8:23 PM, Michael S. Tsirkin <address@hidden> wrote:
> On Tue, Jun 18, 2013 at 07:43:11PM +1000, address@hidden wrote:
>> From: Peter Crosthwaite <address@hidden>
>>
>>
>> This series enables QOM super class access and demostrates some usages.
>> Replaces the save->override->call via FooClass technique, to reduce
>> some of the boiler plate in recently fully QOMified devices.
>>
>> Applied the change to ARM CPU, MB CPU and some of Andreas's recently
>> QOMified i386 devices, all which have the save->override->call issue.
>> ARMCPU I've done a brief test on and seems to work.
>>
>> ARM CPU was particularly difficult, as it has 3 layers of heirachy,
>> where a non-concrete class (TYPE_ARM_CPU) need to super class itself
>> (to TYPE_CPU). This sees the need for super-classers to specify their
>> expected base class level. See patches for illustration.
>>
>> The main future work to the series is to apply the change pattern to
>> the reset of the tree
>
> Looks good to me overall.
> Some nits:
> - Super is an immediate parent in java and python.

s/Super/parent might be the go. But it is designed to work like
py/java. Its the immediate parent of the specified level, and it is
analogous to the java super.

> - One of the design points of QOM is that it let
>   you ignore which class is a parent and which is a child.
>   All casts look the same.
>
> So, why do we need the new APIs with _SUPER?

The SUPER APIs have a nice consistent appearance with the GET_CLASS
and FOO_CLASS APIs and they are likely to live alongside each other in
QOM fns.

> What's wrong with simple
>         object_class_by_name()
> and casting to that?
>

There a performance consequence - object_class_by_name is a lookup,
whereas this approach is able to just walk the pointer through the
inheritance heirachy till it hits. Tying it to an object also brings
into play the possibility of a cast cache should that be needed.

Thanks for the review.

Regards,
Peter

>
>>
>> Peter Crosthwaite (7):
>>   target-arm/cpu.c: delete un-needed instance/class sizes
>>   qom: Add super class accessor
>>   qdev-core: Introduce DEVICE super class cast macros
>>   qom/cpu: Introduce CPU super class cast macros
>>   target-arm: Remove ARMCPUClass
>>   target-microblaze: Remove MicroblazeCPUClass
>>   i8254: Remove [KVM]PITClass
>>
>>  hw/i386/kvm/i8254.c         | 17 ++---------------
>>  hw/timer/i8254.c            | 16 ++--------------
>>  include/hw/qdev-core.h      |  4 ++++
>>  include/qom/cpu.h           |  4 ++++
>>  include/qom/object.h        | 18 ++++++++++++++++++
>>  qom/object.c                | 15 +++++++++++++++
>>  target-arm/cpu-qom.h        | 20 --------------------
>>  target-arm/cpu.c            | 16 +++++-----------
>>  target-microblaze/cpu-qom.h | 20 --------------------
>>  target-microblaze/cpu.c     | 13 ++++---------
>>  10 files changed, 54 insertions(+), 89 deletions(-)
>>
>> --
>> 1.8.3.rc1.44.gb387c77.dirty
>