[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH for-2.5 2/3] spapr_drc: Change value
From: |
Eric Blake |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH for-2.5 2/3] spapr_drc: Change value of property "fdt" from null back to {} |
Date: |
Thu, 3 Dec 2015 19:41:59 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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 <address@hidden>
>> ---
>> 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:
> / {
> };
>
> 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.
>
> 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 = 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.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-ppc] [PATCH for-2.5 2/3] spapr_drc: Change value of property "fdt" from null back to {}, David Gibson, 2015/12/03
[Qemu-ppc] [PATCH for-2.5 1/3] spapr_drc: Handle visitor errors properly, Markus Armbruster, 2015/12/03
[Qemu-ppc] [PATCH for-2.5 3/3] spapr_drc: Make device "spapr-dr-connector" unavailable with -device, Markus Armbruster, 2015/12/03
Re: [Qemu-ppc] [PATCH for-2.5 0/3] spapr_drc: Last minute bug fixes, Markus Armbruster, 2015/12/03