From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58977) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4k80-0006Dd-8K for qemu-devel@nongnu.org; Fri, 04 Dec 2015 01:46:01 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4k7x-00059Z-0H for qemu-devel@nongnu.org; Fri, 04 Dec 2015 01:46:00 -0500 Date: Fri, 4 Dec 2015 16:50:37 +1100 From: David Gibson Message-ID: <20151204055037.GG9559@voom.redhat.com> References: <1449160660-3777-1-git-send-email-armbru@redhat.com> <1449160660-3777-3-git-send-email-armbru@redhat.com> <20151203235408.GC9559@voom.redhat.com> <5660FD77.90500@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="3607uds81ZQvwCD0" Content-Disposition: inline In-Reply-To: <5660FD77.90500@redhat.com> Subject: Re: [Qemu-devel] [PATCH for-2.5 2/3] spapr_drc: Change value of property "fdt" from null back to {} List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: peter.maydell@linaro.org, agraf@suse.de, qemu-ppc@nongnu.org, Markus Armbruster , qemu-devel@nongnu.org --3607uds81ZQvwCD0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Dec 03, 2015 at 07:41:59PM -0700, Eric Blake wrote: > On 12/03/2015 04:54 PM, David Gibson wrote: > > On Thu, Dec 03, 2015 at 05:37:39PM +0100, Markus Armbruster wrote: > >> prop_get_fdt() misuses the visitor API: when fdt is null, it doesn't > >> visit anything. object_property_get_qobject() happily > >> object_property_get_qobject(). Amazingly, the latter survives the > >> misuse. Turns out we've papered over it long before prop_get_fdt() > >> existed, in commit 1d10b44. > >> > >> However, commit 6c2f9a1 changed how we paper over it, and as a side > >> effect changed qom-get's value from {} to null. Change it right back > >> by fixing the visitor misuse. > >> > >> Signed-off-by: Markus Armbruster > >> --- > >> hw/ppc/spapr_drc.c | 5 +++++ > >> 1 file changed, 5 insertions(+) > >> >=20 > > I'm not entirely convinced by this. IIUC, this makes the output in > > the case of NULL (i.e. missing) fdt identical to the output in the > > case of an empty, valid fdt - in dtc syntax, this: > > / { > > }; > >=20 > > Those are different cases from the point of view of the code which > > actually uses the fdt, and for purposes of debugging it, I suspect we > > want to expose that difference. >=20 > Expressing null may be the right thing, but it should be a conscious > decision, and not a side-effect of an unrelated patch. This patch is > just about avoiding a regression for 2.5, because outputting {} for both > a missing fdt and an empty one was the behavior we had back in 2.4 (that > is, we've already returned {} in at least one release, so it won't hurt > to do it for one more). For 2.6 we can revisit things to actually > express what is wanted. >=20 > >=20 > > I don't know what the QOMishly correct way of doing that is, though. > > Can we somehow make the "fdt" property disappear entirely if fdt is > > NULL? >=20 > In qapi terms, if a variable is marked optional and has_FOO is false, > then the variable disappears completely. But I'm not sure if that maps > over to qom. Maybe you do it by setting errp if drc->fdt is NULL, so > that prop_get_fdt() only succeeds when there is something for it to > return. Or maybe returning qnull() is right after all, but in that > case, explicitly calling 'QObject *n =3D qnull(); visit_type_any(v, &n, > NULL, &err) seems like a nicer way than relying on side effects of how > the qmp output visitor behaves when nothing was visited. Ok, thank makes sense. Acked-by: David Gibson --=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 --3607uds81ZQvwCD0 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJWYSmtAAoJEGw4ysog2bOSISkQAKV0YKVL/J9qU248WCNIJ5Q/ mPdcu+z5jjysmcNa8g4Lt2ZTkSEWlKFQ1Uzul1fnb8/UqrxbYuyqN7AMlB2S7O9u RKvQRunUG6Iv6/gOciuZLKvqbPgvCs+YDg/GwZ8eRxksg2wjhU5ZCZj29lyNhsSk nfexSyPcrnNeliWcFVLMl3EJtkma4Ddfc/DZv0nXnEWlTMT7ay7xdcwRa8iNrogU Ifjw0f5DANg3EQ5ywNzHq0WXCH1VHQHFt/cSi8jqUgEKqsNtBZJcqBlaQ/Xtb5TX tUDBWrt1dCckrTYvxK7XmCBLQk52LklZ5S6K/9va8oP6Dlxge4Ilf9PHLJ583ut+ xNXH5C+7RC1uIaVk75ND7bSomjxzMReugzIoskIlfqPmgw0w8O0Xg4g3CQ35oSU8 L6Lxz1xBo3SrgQJv2q6vtGs54JPhAQiTO/iCr7Hy/LyamTRolRZRjrOn3sN6G0Sg /vIKYi0uL/cRBDOCXh00rOZkVcL3ThXYdDDTBLwEJFX2WIkQDmUd7pnZUZGjmlu8 3Y1svOmdjKn2enzCPvXJrpJ/pcfPSGUJM1DHSfcjINzwRfNiM5SfcoJFocsznOw2 wRCywEN68PYmSeAKjd1YzZ/6H5yWTRcRT5XWzMIKNONG7ZnQ6EmUIavkcrSEwqdO Vy/XsVCZiKSu8OlS2XS0 =2vQs -----END PGP SIGNATURE----- --3607uds81ZQvwCD0--