From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51544) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zc3LT-0003LQ-6M for qemu-devel@nongnu.org; Tue, 15 Sep 2015 23:25:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zc3LQ-0007dn-8S for qemu-devel@nongnu.org; Tue, 15 Sep 2015 23:25:19 -0400 Date: Wed, 16 Sep 2015 13:16:35 +1000 From: David Gibson Message-ID: <20150916031635.GY2547@voom.fritz.box> References: <1442194913-26545-1-git-send-email-david@gibson.dropbear.id.au> <1442194913-26545-2-git-send-email-david@gibson.dropbear.id.au> <55F683CB.6000508@ozlabs.ru> <20150914114503.GP2547@voom.fritz.box> <55F6B989.8010900@redhat.com> <55F6D479.4010301@ozlabs.ru> <55F6D897.1090100@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="I2k41U2G4dq8LTBU" Content-Disposition: inline In-Reply-To: <55F6D897.1090100@redhat.com> Subject: Re: [Qemu-devel] [RFCv2 1/2] spapr: Remove unnecessary owner field from sPAPRDRConnector List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: mdroth@linux.vnet.ibm.com, Alexey Kardashevskiy , agraf@suse.de, qemu-devel@nongnu.org, qemu-ppc@nongnu.org, bharata@linux.vnet.ibm.com --I2k41U2G4dq8LTBU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Sep 14, 2015 at 04:24:23PM +0200, Paolo Bonzini wrote: >=20 >=20 > On 14/09/2015 16:06, Alexey Kardashevskiy wrote: > >>>>> > >>>>> =3D=3D=3D * There is no way for a child to determine what its parent > >>>>> is. It is not * a bidirectional relationship. This is by > >>>>> design. =3D=3D=3D > >>>>> > >>>>> 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. > >=20 > > Is this case external? >=20 > I meant external as in qom-get, qom-set, qom-list. There isn't a ".." > property. >=20 > > 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 i= t :) >=20 > Yes, we can add such an API. >=20 > Let's look also at what ->owner is used for. >=20 > > object_property_add_alias(root_container, link_name, > > drc->owner, child_name, &err); >=20 > This can be rewritten as >=20 > object_property_add_const_link(root_container, link_name, > drc, &err); >=20 > > QTAILQ_FOREACH(prop, &root_container->properties, node) { > > Object *obj; > > sPAPRDRConnector *drc; > > sPAPRDRConnectorClass *drck; > > uint32_t drc_index, drc_power_domain; > >=20 > > if (!strstart(prop->type, "link<", NULL)) { > > continue; > > } > >=20 > > obj =3D object_property_get_link(root_container, prop->name, NU= LL); > > drc =3D SPAPR_DR_CONNECTOR(obj); > > drck =3D SPAPR_DR_CONNECTOR_GET_CLASS(drc); > >=20 > > if (owner && (drc->owner !=3D owner)) { >=20 > 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. So, yes, iterating over the connectors under the owner rather than going via the global links is another cleanup I've considered but haven't gotten around to. Except, I've been thinking further, and I'm not sure it makes sense to keep these DR connectors around as full QOM objects anyway. Having them here at least potentially exposes the DR connector stuff to users, when it's really an internal of how the hypervisor communicates with the guest about hotplug. I'm going to have to talk to more QOM-experienced people to thrash out the details, but I'm thinking a better approach would be to add a "DR connector array" QOM interface to the existing PCI host bridge objects and.. to something for CPU and memory. That interface would allow the necessary lookups specifically for DR hotplug events and avoid creating thousands of extra QOM objects. Either way, it makes this little cleanup pretty irrelevant, so I'll drop it for now. --=20 David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson --I2k41U2G4dq8LTBU Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJV+N8SAAoJEGw4ysog2bOSf+EP/15ph2JNhpJXZIaS0yc7CB9Y tuA4xFo1PgbqZlxs1fbRZzCZjGVNdGpYey/InQt2X6TANS1mn8aIX7lEq6iI/CJ/ XuicFi/iROIRLiAYMw8R71WPNgu4UPxWnKZcqulgVjE8r6LYm706cSbdIr+kRjfj dNYCvOvEsl0i13IlXUp6qW6nDuIfRreXa0zztnbbIf3hD5REaJ+99NVR3+1LtEA2 aOAYTF6YoPEfw0VpagjTBHugZWlKf0CDPAiFV0drDprpyMa3buiHtlXUgvlktd4I hmHDjU5yRXk45WhX+CyrKGDojYesbBZRb5QcUJGK3tFityAzbTjN+KwzQeD9rG2+ +AOZN5wHQ4czebSSRKXIXxzrbvRKm462nylzLb2E63jx0uxrRgJKmKBqdE0Cy6LP TOzi/sxTPtTeClAAMPGskPa8KDYwCQQDORyNzVtCUeCzaDPzknmR5swR3tHkkNFk TQgIr8zIGVN4a1GB4vfV2JZjv/tnG321j7aDXx8vLs/tGG2NQZNviM3jYBSmD/d5 dIN0NiH7+h1WVonF+XF9xgc217nw53rAzH49hS2d8ayfzTW8CPlBt3R+4rViUgdn /H0+19KLouX9lBgh+dVluOUryEDriLe2Yd6dteMONJ1ANh/5AcGRgpEqQidfW5t3 NeLncKQiTMbJPi9NtD8a =Y8zC -----END PGP SIGNATURE----- --I2k41U2G4dq8LTBU--