Re: [Qemu-devel] [RFCv2 1/2] spapr: Remove unnecessary owner field from sPAPRDRConnector

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 09/14/2015 11:41 AM, David Gibson wrote:
> The sPAPRDRConnector pseudo-device contains an owner field which is
> set in spapr_dr_connector_new().  However, that function also calls
> object_property_add_child() to set the DRConnector as the QOM child of
> the owner object.  That means that owner is always the same as the QOM
> parent, and so redundant.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   hw/ppc/spapr_drc.c         | 5 ++---
>   include/hw/ppc/spapr_drc.h | 1 -
>   2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index 9ce844a..68e0c3e 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -416,7 +416,7 @@ static void realize(DeviceState *d, Error **errp)
>       child_name = object_get_canonical_path_component(OBJECT(drc));
>       DPRINTFN("drc child name: %s", child_name);
>       object_property_add_alias(root_container, link_name,
> -                              drc->owner, child_name, &err);
> +                              OBJECT(drc)->parent, child_name, &err);
>       if (err) {
>           error_report("%s", error_get_pretty(err));
>           error_free(err);
> @@ -456,7 +456,6 @@ sPAPRDRConnector *spapr_dr_connector_new(Object *owner,
>
>       drc->type = type;
>       drc->id = id;
> -    drc->owner = owner;
>       object_property_add_child(owner, "dr-connector[*]", OBJECT(drc), NULL);


The comment for object_property_add_child() in include/qom/object.h says:

===
* There is no way for a child to determine what its parent is.  It is not
* a bidirectional relationship.  This is by design.
===

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.



>       object_property_set_bool(OBJECT(drc), true, "realized", NULL);
>
> @@ -669,7 +668,7 @@ int spapr_drc_populate_dt(void *fdt, int fdt_offset, Object *owner,
>           drc = SPAPR_DR_CONNECTOR(obj);
>           drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
>
> -        if (owner && (drc->owner != owner)) {
> +        if (owner && (OBJECT(drc)->parent != owner)) {
>               continue;
>           }
>
> diff --git a/include/hw/ppc/spapr_drc.h b/include/hw/ppc/spapr_drc.h
> index 28ffeae..16e2d4b 100644
> --- a/include/hw/ppc/spapr_drc.h
> +++ b/include/hw/ppc/spapr_drc.h
> @@ -137,7 +137,6 @@ typedef struct sPAPRDRConnector {
>
>       sPAPRDRConnectorType type;
>       uint32_t id;
> -    Object *owner;
>       const char *name;
>
>       /* sensor/indicator states */
>


-- 
Alexey