Re: [Qemu-devel] [PATCH 5/5] spapr: Remove some non-useful properties on DRC objects

[Michael Roth]

Quoting David Gibson (2017-06-02 02:29:52)
>  * 'connector_type' is easily derived from the 'index' property, so there's
>    no point to it (it's also implicit in the QOM type of the DRC)
>  * 'isolation-state', 'indicator-state' and 'allocation-state' are
>    part of the transaction between qemu and guest during PAPR hotplug
>    operations, and outside tools really have no business looking at it
>    (especially not changing, and these were RW properties)
>  * 'entity-sense' is basically just a weird PAPR encoding of whether there
>    is a device connected to this DRC
> 
> Strictly speaking removing these properties is breaking the qemu interface.
> However, I'm pretty sure no management tools have ever used these.  For
> debugging there are better alternatives.  Therefore, I think removing these
> broken interfaces is the better option.

Debugging was indeed the primary motivation behind these. I agree these
don't really serve any useful purpose now and probably should've just
been traces from the start.

> 
> Signed-off-by: David Gibson <address@hidden>

Reviewed-by: Michael Roth <address@hidden>

> ---
>  hw/ppc/spapr_drc.c | 29 -----------------------------
>  1 file changed, 29 deletions(-)
> 
> diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> index d43c9cd..4dd26a8 100644
> --- a/hw/ppc/spapr_drc.c
> +++ b/hw/ppc/spapr_drc.c
> @@ -228,14 +228,6 @@ static void prop_get_index(Object *obj, Visitor *v, 
> const char *name,
>      visit_type_uint32(v, name, &value, errp);
>  }
> 
> -static void prop_get_type(Object *obj, Visitor *v, const char *name,
> -                          void *opaque, Error **errp)
> -{
> -    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
> -    uint32_t value = (uint32_t)spapr_drc_type(drc);
> -    visit_type_uint32(v, name, &value, errp);
> -}
> -
>  static char *prop_get_name(Object *obj, Error **errp)
>  {
>      sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
> @@ -243,17 +235,6 @@ static char *prop_get_name(Object *obj, Error **errp)
>      return g_strdup(drck->get_name(drc));
>  }
> 
> -static void prop_get_entity_sense(Object *obj, Visitor *v, const char *name,
> -                                  void *opaque, Error **errp)
> -{
> -    sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
> -    sPAPRDRConnectorClass *drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> -    uint32_t value;
> -
> -    drck->entity_sense(drc, &value);
> -    visit_type_uint32(v, name, &value, errp);
> -}
> -
>  static void prop_get_fdt(Object *obj, Visitor *v, const char *name,
>                           void *opaque, Error **errp)
>  {
> @@ -670,20 +651,10 @@ static void spapr_dr_connector_instance_init(Object 
> *obj)
>  {
>      sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(obj);
> 
> -    object_property_add_uint32_ptr(obj, "isolation-state",
> -                                   &drc->isolation_state, NULL);
> -    object_property_add_uint32_ptr(obj, "indicator-state",
> -                                   &drc->indicator_state, NULL);
> -    object_property_add_uint32_ptr(obj, "allocation-state",
> -                                   &drc->allocation_state, NULL);
>      object_property_add_uint32_ptr(obj, "id", &drc->id, NULL);
>      object_property_add(obj, "index", "uint32", prop_get_index,
>                          NULL, NULL, NULL, NULL);
> -    object_property_add(obj, "connector_type", "uint32", prop_get_type,
> -                        NULL, NULL, NULL, NULL);
>      object_property_add_str(obj, "name", prop_get_name, NULL, NULL);
> -    object_property_add(obj, "entity-sense", "uint32", prop_get_entity_sense,
> -                        NULL, NULL, NULL, NULL);
>      object_property_add(obj, "fdt", "struct", prop_get_fdt,
>                          NULL, NULL, NULL, NULL);
>  }
> -- 
> 2.9.4
>