Re: [Qemu-devel] [PATCH 27/28] sysbus: apic: ioapic: convert to QEMU Object Model

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 3157 bytes --]

On 2012-01-24 22:53, Anthony Liguori wrote:
> On 01/24/2012 03:31 PM, Jan Kiszka wrote:
>> On 2012-01-24 22:11, Anthony Liguori wrote:
>>> On 01/24/2012 03:01 PM, Jan Kiszka wrote:
>>>> On 2012-01-24 21:21, Anthony Liguori wrote:
>>>>>> Also, I see a lot of programmatic initialization and a lot of
>>>>>> repeating
>>>>>> patterns (specifically regarding trivial class initialization) -
>>>>>> there
>>>>>> is no better alternative?
>>>>>
>>>>> Not really, no.  It looks bad now because you have DeviceInfo still.
>>>>> Once DeviceInfo goes away, all of the initialization will happen in
>>>>> the
>>>>> class_init function.
>>>>>
>>>>> The design of QOM is such that a lot of what was previously done via
>>>>> declarative structures is now done imperatively.  But the code bloat
>>>>> that came in this patch series will decrease significantly with the
>>>>> next
>>>>> series as we eliminate DeviceInfo.
>>>>
>>>> Are there examples of fully converted devices to get an impression?
>>>
>>> https://github.com/aliguori/qemu/tree/qom-rebase.8
>>>
>>> Has everything fully converted (including BusState).
>>>
>>> If you look at qdev.[ch], you'll notice the remaining qdev
>>> infrastructure becomes greatly simplified.
>>
>> But I don't get yet why all these repeating initialization tasks need to
>> be open-coded instead of remaining declarative.
> 
> It would look like:
> 
> static void device_generic_class_init(ObjectClass *klass, void *data)
> {
>     DeviceClass *dc = DEVICE_CLASS(klass);
>     DeviceTypeInfo *ti = data;
> 
>     if (ti->reset) {
>        dc->reset = ti->reset;
>     }
>     if (ti->vmsd) {
>        dc->vmsd = ti->vmsd;
>     }
> }
> 
> void device_type_register_static(DeviceTypeInfo *ti)
> {
>     TypeInfo ti = {
>       .class_init = device_generic_class_init,
>       .class_data = ti,
>       // ...
>     }
>     type_register_static(&ti);
> }
> 
> But I don't like this.  The problem is that the declarative syntax we
> have doesn't distinguish between "not-specified" and
> "zero-initialized".

That's surely solvable.

>  It's also tempting to just drop the if (ti->reset)
> check but that means that you unconditionally override the base class
> implementation even if it's not specified.

That as well, just wrap this repeating pattern in a macro.

> 
> I don't see any tangible benefits to a declarative syntax except that it
> makes it harder to get right because you have to write per-base class
> initialization functions and it's very easy to get polymorphism wrong
> there.

Now you have to write way more derived class init functions - provided a
base class is used more than once.

> 
> We're not talking about a code size difference.  It's a wash in terms of
> number of lines of code.

We are. Code size will be smaller, so the number of lines. How much,
that depends on the number of users per base class.

> 
> Imperative allows you to explicit zero-initialize, accept the previous
> version (from the base class), or override with a new value.

None of this is impossible with a declarative approach.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]