qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: Alexey Kardashevskiy <aik@ozlabs.ru>,
	David Gibson <david@gibson.dropbear.id.au>
Cc: qemu-devel@nongnu.org, agraf@suse.de, qemu-ppc@nongnu.org,
	mdroth@linux.vnet.ibm.com, bharata@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [RFCv2 1/2] spapr: Remove unnecessary owner field from sPAPRDRConnector
Date: Mon, 14 Sep 2015 16:24:23 +0200	[thread overview]
Message-ID: <55F6D897.1090100@redhat.com> (raw)
In-Reply-To: <55F6D479.4010301@ozlabs.ru>



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

  reply	other threads:[~2015-09-14 14:24 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-14  1:41 [Qemu-devel] [RFCv2 0/2] spapr: Cleanups to dynamic reconfiguration mechanism David Gibson
2015-09-14  1:41 ` [Qemu-devel] [RFCv2 1/2] spapr: Remove unnecessary owner field from sPAPRDRConnector David Gibson
2015-09-14  6:23   ` Bharata B Rao
2015-09-14  8:22   ` Alexey Kardashevskiy
2015-09-14 11:45     ` David Gibson
2015-09-14 12:11       ` Paolo Bonzini
2015-09-14 14:06         ` Alexey Kardashevskiy
2015-09-14 14:24           ` Paolo Bonzini [this message]
2015-09-16  3:16             ` David Gibson
2015-09-17 15:50               ` Michael Roth
2015-09-17 15:53                 ` Paolo Bonzini
2015-09-17 23:03                   ` Michael Roth
2015-09-17 15:01             ` Michael Roth
2015-09-15  0:31           ` David Gibson
2015-09-15  0:30         ` David Gibson
2015-09-15 11:05           ` Paolo Bonzini
2015-09-14  1:41 ` [Qemu-devel] [RFCv2 2/2] spapr: Don't use QOM [*] syntax for DR connectors David Gibson
2015-09-14  4:07   ` Bharata B Rao
2015-09-14  4:14     ` David Gibson
2015-09-14  4:41       ` Bharata B Rao
2015-09-14  5:20         ` David Gibson
2015-09-14  8:13   ` Alexey Kardashevskiy
2015-09-15  4:03   ` Michael Roth
2015-09-15  4:21     ` Michael Roth
2015-09-16  3:18     ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55F6D897.1090100@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=agraf@suse.de \
    --cc=aik@ozlabs.ru \
    --cc=bharata@linux.vnet.ibm.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).