From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34977) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4gK4-0005HV-PD for qemu-devel@nongnu.org; Thu, 03 Dec 2015 21:42:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4gJz-0006OO-Pg for qemu-devel@nongnu.org; Thu, 03 Dec 2015 21:42:12 -0500 References: <1449160660-3777-1-git-send-email-armbru@redhat.com> <1449160660-3777-3-git-send-email-armbru@redhat.com> <20151203235408.GC9559@voom.redhat.com> From: Eric Blake Message-ID: <5660FD77.90500@redhat.com> Date: Thu, 3 Dec 2015 19:41:59 -0700 MIME-Version: 1.0 In-Reply-To: <20151203235408.GC9559@voom.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="c2bB8tOdew9J8SaKu0jSlqpE8t6uKGcaL" 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: David Gibson , Markus Armbruster Cc: peter.maydell@linaro.org, qemu-ppc@nongnu.org, qemu-devel@nongnu.org, agraf@suse.de This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --c2bB8tOdew9J8SaKu0jSlqpE8t6uKGcaL Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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(+) >> > 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. 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 > 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? 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. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --c2bB8tOdew9J8SaKu0jSlqpE8t6uKGcaL Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWYP13AAoJEKeha0olJ0NqaLMH/RWU17hYiZs5ZkiJ4qaivMTa RQV2ejgwk24RZm9UwbUqwRiyU2MsIRJ3e2W+m4Y5U5u+f3/5niM7ExZjR81CNMy0 K3xtveUI4NKCrLts7N4UqDvcT9NIPi8znRNgNBFGYZKvL6XPhekvbqs+nHkxYcET Kia0I3A4nrkwTkU9t3zsIqs/yyxNWeJq7A4EhjZtDqFCGAS3xB83LwQX18BbpijA OS0gj2qoF+LLIPbXqfvSvfHEmq3Sp+/C+GyxAIJBrNr3UNuQgfYeWzTxsqYVydIJ +O3EEK0Nq45CNeanq5reIa1FCZ1Cyfe286NaKJC8R6DCELvHDVu0vahoAOxJZgM= =mwEa -----END PGP SIGNATURE----- --c2bB8tOdew9J8SaKu0jSlqpE8t6uKGcaL--