[PATCH for-5.0 0/8] ppc: Consolidate QOM links and pointers to the same object

Greg Kurz posted 8 patches 4 years, 5 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
hw/intc/pnv_xive.c        |   29 +++++++++-----------
hw/intc/spapr_xive.c      |    8 +++---
hw/intc/xive.c            |   65 ++++++++++++++++++++++-----------------------
hw/ppc/pnv.c              |   32 +++++++++++-----------
hw/ppc/pnv_core.c         |   18 +++++++-----
hw/ppc/pnv_homer.c        |   24 +++++++++--------
hw/ppc/pnv_lpc.c          |   23 ++++++++--------
hw/ppc/pnv_occ.c          |   23 ++++++++--------
hw/ppc/pnv_psi.c          |    3 +-
include/hw/ppc/pnv.h      |    2 +
include/hw/ppc/pnv_core.h |    2 +
11 files changed, 115 insertions(+), 114 deletions(-)
[PATCH for-5.0 0/8] ppc: Consolidate QOM links and pointers to the same object
Posted by Greg Kurz 4 years, 5 months ago
There's a recurring pattern in the code where a const link is added to a
newly instanciated object and the link is then used in the object's realize
function to keep a pointer to the QOM entity which the link points to.

void create_obj_B(Object *obj_a)
{
    Object *obj_b;

    obj_b = object_new(TYPE_B);
    object_property_add_const_link(obj_b, "link-to-a", obj_a, &error_abort);
}

void object_B_realize(DeviceState *dev, Error **errp)
{
    Object *obj_a;

    obj_a = object_property_get_link(OBJECT(dev), "link-to-a", errp);
    if (!obj_a) {
        return;
    }

    obj_b->obj_a = A(obj_a); // If obj_b->obj_a is changed, the link property
                             // still points to the original obj_a that was
                             // passed to object_property_add_const_link()
}

Confusing bugs could arise if the pointer and the link go out of sync for
some reason. This can be avoided if the property is defined to directly use
the pointer with object_property_add_link() and object_property_set_link().

This series just does that for all occurences of the fragile pattern in
the XIVE and PNV code.

--
Greg

---

Greg Kurz (8):
      xive: Link "cpu" property to XiveTCTX::cs pointer
      xive: Link "xive" property to XiveSource::xive pointer
      xive: Link "xive" property to XiveEndSource::xrtr pointer
      ppc/pnv: Link "psi" property to PnvLpc::psi pointer
      ppc/pnv: Link "psi" property to PnvOCC::psi pointer
      ppc/pnv: Link "chip" property to PnvHomer::chip pointer
      ppc/pnv: Link "chip" property to PnvCore::chip pointer
      ppc/pnv: Link "chip" property to PnvXive::chip pointer


 hw/intc/pnv_xive.c        |   29 +++++++++-----------
 hw/intc/spapr_xive.c      |    8 +++---
 hw/intc/xive.c            |   65 ++++++++++++++++++++++-----------------------
 hw/ppc/pnv.c              |   32 +++++++++++-----------
 hw/ppc/pnv_core.c         |   18 +++++++-----
 hw/ppc/pnv_homer.c        |   24 +++++++++--------
 hw/ppc/pnv_lpc.c          |   23 ++++++++--------
 hw/ppc/pnv_occ.c          |   23 ++++++++--------
 hw/ppc/pnv_psi.c          |    3 +-
 include/hw/ppc/pnv.h      |    2 +
 include/hw/ppc/pnv_core.h |    2 +
 11 files changed, 115 insertions(+), 114 deletions(-)


Re: [PATCH for-5.0 0/8] ppc: Consolidate QOM links and pointers to the same object
Posted by Cédric Le Goater 4 years, 5 months ago
On 15/11/2019 12:53, Greg Kurz wrote:
> There's a recurring pattern in the code where a const link is added to a
> newly instanciated object and the link is then used in the object's realize
> function to keep a pointer to the QOM entity which the link points to.
> 
> void create_obj_B(Object *obj_a)
> {
>     Object *obj_b;
> 
>     obj_b = object_new(TYPE_B);
>     object_property_add_const_link(obj_b, "link-to-a", obj_a, &error_abort);
> }
> 
> void object_B_realize(DeviceState *dev, Error **errp)
> {
>     Object *obj_a;
> 
>     obj_a = object_property_get_link(OBJECT(dev), "link-to-a", errp);
>     if (!obj_a) {
>         return;
>     }
> 
>     obj_b->obj_a = A(obj_a); // If obj_b->obj_a is changed, the link property
>                              // still points to the original obj_a that was
>                              // passed to object_property_add_const_link()
> }
> 
> Confusing bugs could arise if the pointer and the link go out of sync for
> some reason. This can be avoided if the property is defined to directly use
> the pointer with object_property_add_link() and object_property_set_link().
> 
> This series just does that for all occurences of the fragile pattern in
> the XIVE and PNV code.

Can we use DEFINE_PROP_LINK() instead ?

C. 


> 
> --
> Greg
> 
> ---
> 
> Greg Kurz (8):
>       xive: Link "cpu" property to XiveTCTX::cs pointer
>       xive: Link "xive" property to XiveSource::xive pointer
>       xive: Link "xive" property to XiveEndSource::xrtr pointer
>       ppc/pnv: Link "psi" property to PnvLpc::psi pointer
>       ppc/pnv: Link "psi" property to PnvOCC::psi pointer
>       ppc/pnv: Link "chip" property to PnvHomer::chip pointer
>       ppc/pnv: Link "chip" property to PnvCore::chip pointer
>       ppc/pnv: Link "chip" property to PnvXive::chip pointer
> 
> 
>  hw/intc/pnv_xive.c        |   29 +++++++++-----------
>  hw/intc/spapr_xive.c      |    8 +++---
>  hw/intc/xive.c            |   65 ++++++++++++++++++++++-----------------------
>  hw/ppc/pnv.c              |   32 +++++++++++-----------
>  hw/ppc/pnv_core.c         |   18 +++++++-----
>  hw/ppc/pnv_homer.c        |   24 +++++++++--------
>  hw/ppc/pnv_lpc.c          |   23 ++++++++--------
>  hw/ppc/pnv_occ.c          |   23 ++++++++--------
>  hw/ppc/pnv_psi.c          |    3 +-
>  include/hw/ppc/pnv.h      |    2 +
>  include/hw/ppc/pnv_core.h |    2 +
>  11 files changed, 115 insertions(+), 114 deletions(-)
> 


Re: [PATCH for-5.0 0/8] ppc: Consolidate QOM links and pointers to the same object
Posted by Greg Kurz 4 years, 5 months ago
On Fri, 15 Nov 2019 13:16:22 +0100
Cédric Le Goater <clg@kaod.org> wrote:

> On 15/11/2019 12:53, Greg Kurz wrote:
> > There's a recurring pattern in the code where a const link is added to a
> > newly instanciated object and the link is then used in the object's realize
> > function to keep a pointer to the QOM entity which the link points to.
> > 
> > void create_obj_B(Object *obj_a)
> > {
> >     Object *obj_b;
> > 
> >     obj_b = object_new(TYPE_B);
> >     object_property_add_const_link(obj_b, "link-to-a", obj_a, &error_abort);
> > }
> > 
> > void object_B_realize(DeviceState *dev, Error **errp)
> > {
> >     Object *obj_a;
> > 
> >     obj_a = object_property_get_link(OBJECT(dev), "link-to-a", errp);
> >     if (!obj_a) {
> >         return;
> >     }
> > 
> >     obj_b->obj_a = A(obj_a); // If obj_b->obj_a is changed, the link property
> >                              // still points to the original obj_a that was
> >                              // passed to object_property_add_const_link()
> > }
> > 
> > Confusing bugs could arise if the pointer and the link go out of sync for
> > some reason. This can be avoided if the property is defined to directly use
> > the pointer with object_property_add_link() and object_property_set_link().
> > 
> > This series just does that for all occurences of the fragile pattern in
> > the XIVE and PNV code.
> 
> Can we use DEFINE_PROP_LINK() instead ?
> 

Heh this seems to be even better indeed. I'll give a try.

> C. 
> 
> 
> > 
> > --
> > Greg
> > 
> > ---
> > 
> > Greg Kurz (8):
> >       xive: Link "cpu" property to XiveTCTX::cs pointer
> >       xive: Link "xive" property to XiveSource::xive pointer
> >       xive: Link "xive" property to XiveEndSource::xrtr pointer
> >       ppc/pnv: Link "psi" property to PnvLpc::psi pointer
> >       ppc/pnv: Link "psi" property to PnvOCC::psi pointer
> >       ppc/pnv: Link "chip" property to PnvHomer::chip pointer
> >       ppc/pnv: Link "chip" property to PnvCore::chip pointer
> >       ppc/pnv: Link "chip" property to PnvXive::chip pointer
> > 
> > 
> >  hw/intc/pnv_xive.c        |   29 +++++++++-----------
> >  hw/intc/spapr_xive.c      |    8 +++---
> >  hw/intc/xive.c            |   65 ++++++++++++++++++++++-----------------------
> >  hw/ppc/pnv.c              |   32 +++++++++++-----------
> >  hw/ppc/pnv_core.c         |   18 +++++++-----
> >  hw/ppc/pnv_homer.c        |   24 +++++++++--------
> >  hw/ppc/pnv_lpc.c          |   23 ++++++++--------
> >  hw/ppc/pnv_occ.c          |   23 ++++++++--------
> >  hw/ppc/pnv_psi.c          |    3 +-
> >  include/hw/ppc/pnv.h      |    2 +
> >  include/hw/ppc/pnv_core.h |    2 +
> >  11 files changed, 115 insertions(+), 114 deletions(-)
> > 
>