Re: [Qemu-devel] [PATCH for-2.5 2/3] spapr_drc: Change value of property "fdt" from null back to {}

[David Gibson]

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(+)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 4e7a1d3..dad157f 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -259,6 +259,11 @@ static void prop_get_fdt(Object *obj, Visitor *v, void 
> *opaque,
>      void *fdt;
>  
>      if (!drc->fdt) {
> +        visit_start_struct(v, NULL, NULL, name, 0, &err);
> +        if (!err) {
> +            visit_end_struct(v, &err);
> +        }
> +        error_propagate(errp, err);
>          return;
>      }
>  

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.

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?

-- 
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

signature.asc
Description: PGP signature