Re: [Qemu-devel] [PATCH v2 15/18] rtc: add a dynamic property for retrieving the date

[Kevin Wolf]

Am 13.12.2011 14:31, schrieb Anthony Liguori:
> On 12/13/2011 03:27 AM, Kevin Wolf wrote:
>> Am 09.12.2011 15:25, schrieb Anthony Liguori:
>>> On 12/09/2011 08:04 AM, Kevin Wolf wrote:
>>>> Am 09.12.2011 14:08, schrieb Anthony Liguori:
>>>>> On 12/09/2011 05:26 AM, Kevin Wolf wrote:
>>>>>> Am 02.12.2011 21:20, schrieb Anthony Liguori:
>>>>>>> This really shows the power of dynamic object properties compared to 
>>>>>>> qdev
>>>>>>> static properties.
>>>>>>>
>>>>>>> This property represents a complex structure who's format is preserved 
>>>>>>> over the
>>>>>>> wire.  This is enabled by visitors.
>>>>>>>
>>>>>>> It also shows an entirely synthetic property that is not tied to device 
>>>>>>> state.
>>>>>>>
>>>>>>> Signed-off-by: Anthony Liguori<address@hidden>
>>>>>>
>>>>>> There's one thing that I was hoping to find answered when I would have
>>>>>> reviewed the whole series, but it hasn't happened: There is no doubt
>>>>>> that dynamic properties (in the sense of being able to modify them after
>>>>>> constructions) are a useful thing. But you also claim that class-based
>>>>>> properties are not enough for QOM and that we need object-based ones,
>>>>>> which is a requirement not immediately obvious to me.
>>>>>>
>>>>>> Can you provide some examples where we would explicitly need
>>>>>> object-based properties?
>>>>>
>>>>> Sure.  Any property that's dynamic needs to be object based.  A good 
>>>>> example
>>>>> would be PCI slots.
>>>>>
>>>>> Today, we unconditionally advertise 32 slots in our ACPI tables.  It 
>>>>> could be
>>>>> desirable to eventually make this configurable.  So you can imagine where 
>>>>> you
>>>>> would have an 'slot-count' property and if that was set to 16, it would 
>>>>> result
>>>>> in 'slot[0]..slot[15]' being created.
>>>>>
>>>>> There are other good examples too.
>>>>
>>>> So is it mostly about variably sized arrays, which just happen to be
>>>> considered independent properties in your approach? Or are there cases
>>>> where a logically separate property may be there or missing depending on
>>>> some condition, or possibly even that a new property is created during
>>>> runtime?
>>>
>>> So there are three possibilities for properties:
>>>
>>> 1) Properties have no per-object state, and exist entirely within the 
>>> classes.
>>> This is what qdev does today.
>>
>> Not quite sure what you mean by per-object state. The properties are
>> fields in the XyzState, so they certainly are per-object?
> 
> In qdev today, the objects have no state reflecting properties.  They 
> properties 
> refer to object state, but the objects have no knowledge of the properties 
> themselves.

You mean that an object doesn't have access to the property metadata? Or
what else would an object want to know about them?

>>> 2) Properties are defined in the class, but carry per-object state.
>>>
>>> 3) Properties are defined in the object and carry per-object state.
>>>
>>> We definitely can rule out (1).  Stateful properties are needed to implement
>>> links, composition, and just about anything interesting.
>>>
>>> Another way that (3) is useful is that it allows you to create container 
>>> devices
>>> that more or less model a PCB.  That's how peripheral[-anon] is implemented 
>>> and
>>> I imagine that it will also be useful for implementing "machine" devices.
>>
>> What would this look like? The user creates new child/link properties on
>> the board, and some more automatically created properties somehow
>> describe the wiring between them?
> 
> The way I imagine this all working out is that a user creates does something 
> like this:
> 
> (qemu) device_add driver=pc,id=pc
> (qemu) qom_list /pc
> /pc
>    /i440fx
>    /ide
>      /ide1
>        /drive[0]
>    /net[0]
>    /serial[0]

So you instantiated an object of the class pc, right? Which of the
properties qom_list shows is per-object rather than per-class?

>>> Of course, you could find a way to special case this with (2) but that's 
>>> why I
>>> ended up going with (3).  You can avoid having a lot of special cases this 
>>> way.
>>
>> I'm not entirely convinced that we really need this, but on the other
>> hand I don't feel strong enough about it to argue.
>>
>> Actually I think my real problem isn't about per-object properties
>> (although they might add unnecessary complexity), but more about going
>> away from the qdev style of things where you had _one_ struct definition
>> that nicely described all of the properties in a central place. Instead,
>> I'm seeing patches that spread property definitions all over the code.
> 
> There isn't one struct as bus properties get inherited, but I understand your 
> point.  The problem with "legacy" properties isn't how they're express, it's 
> that the mix parsing with the property types.  I have no problem with using 
> DEFINE_ style properties and would expect that we would find a way to make 
> the 
> current properties have a string and non-string interface.
> 
>> Now I understand that for dynamically created properties (like on your
>> PCB) this is necessary and can't be avoided. For about 99% of the
>> devices static definition of properties would be enough, though.
>>
>> So basically what I'm asking for is getting the static structs back for
>> the 99% and have common code that parses them and calls the appropriate
>> functions to actually the properties. The remaining 1% that
>> creates/deletes properties during runtime and isn't covered can directly
>> call whatever it needs.
> 
> If you look at my qom-next branch, the static structs are still there for 
> 99%er 
> devices.  I have no immediate plans of removing them.

Right, when looking at your serial examples, I saw that there are
actually some of them left. This first series doesn't have any of them.

I think the pattern may be that it's only children and links that are
spread across the code and "primitive types" generally still use the
declarative approach. Is there a reason not to include children/links
there as well?

I also think in the serial example there was quite some boilerplate code
related to such things, that could probably be common code for most
devices that can be described statically in tables. But I think I'll get
to that once you post these patches, maybe I'll understand the reasons
better by then.

Kevin