Re: [Qemu-devel] [RFCv2 1/2] spapr: Remove unnecessary owner field from sPAPRDRConnector

[Paolo Bonzini]

On 14/09/2015 16:06, Alexey Kardashevskiy wrote:
>>>>>
>>>>> === * There is no way for a child to determine what its parent
>>>>> is.  It is not * a bidirectional relationship.  This is by
>>>>> design. ===
>>>>>
>>>>> This part always confused me as there is "Object *parent" in
>>>>> the "struct Object". So there is way to determine but it must
>>>>> not be used? Is it debug only?
>>>>>
>>>>> Anyway, all members of the Object class are under /*< private
>>>>>> */ so they should not be accesses in sPAPR code, I believe.
>>> Ah, good point, I missed that.  I guess we have to keep the owner
>>> field, redundant though it seems.  Blech.
>>
>> I think the comment is wrong or at least inaccurate; it only applies
>> to the external QOM interface.
> 
> Is this case external?

I meant external as in qom-get, qom-set, qom-list.  There isn't a ".."
property.

> Originally I was looking for a object_get_parent() but it is not there
> so I decided that the comment is correct or I just fail to understand it :)

Yes, we can add such an API.

Let's look also at what ->owner is used for.

> object_property_add_alias(root_container, link_name,
>                           drc->owner, child_name, &err);

This can be rewritten as

     object_property_add_const_link(root_container, link_name,
                                    drc, &err);

>     QTAILQ_FOREACH(prop, &root_container->properties, node) {
>         Object *obj;
>         sPAPRDRConnector *drc;
>         sPAPRDRConnectorClass *drck;
>         uint32_t drc_index, drc_power_domain;
> 
>         if (!strstart(prop->type, "link<", NULL)) {
>             continue;
>         }
> 
>         obj = object_property_get_link(root_container, prop->name, NULL);
>         drc = SPAPR_DR_CONNECTOR(obj);
>         drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> 
>         if (owner && (drc->owner != owner)) {

Could the PCI host bridge instead store the DR connectors when it
creates them with spapr_dr_connector_new?  Then you can just call
spapr_drc_populate_dt directly with the right objects, and avoid another
O(n^2) loop.

Paolo