Re: [Qemu-devel] [for-2.11 PATCH 07/26] spapr_drc: fix realize and unrealize

[Greg Kurz]

On Wed, 26 Jul 2017 14:04:59 +1000
David Gibson <address@hidden> wrote:

> On Tue, Jul 25, 2017 at 07:59:31PM +0200, Greg Kurz wrote:
> > If object_property_add_alias() returns an error in realize(), we should
> > propagate it to the caller and certainly not unref the DRC.
> > 
> > Same thing goes for unrealize(). Since object_property_del() is the last
> > call, we can even get rid of the intermediate Error *.
> > 
> > And finally, unrealize() should undo all registrations performed by
> > realize().
> > 
> > Signed-off-by: Greg Kurz <address@hidden>  
> 
> I've applied this to ppc-for-2.11, but this looks like it could be a
> real bug fix.  So I'm wondering if I should push it for 2.10 (we're in
> hard freeze, but bugfixes can still be applied).
> 

Yeah I guess it should also be merged in 2.10 but it has a dependency
with patch 4 (spapr_drc: use g_strdup_printf() instead  of snprintf()).
If you prefer, maybe I can repost this as a standalone fix for 2.10 ?

> > ---
> >  hw/ppc/spapr_drc.c |   15 ++++++---------
> >  1 file changed, 6 insertions(+), 9 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
> > index e4e8383ec7b5..d72453bcb42f 100644
> > --- a/hw/ppc/spapr_drc.c
> > +++ b/hw/ppc/spapr_drc.c
> > @@ -506,12 +506,12 @@ static void realize(DeviceState *d, Error **errp)
> >      trace_spapr_drc_realize_child(spapr_drc_index(drc), child_name);
> >      object_property_add_alias(root_container, link_name,
> >                                drc->owner, child_name, &err);
> > +    g_free(child_name);
> >      g_free(link_name);
> >      if (err) {
> > -        error_report_err(err);
> > -        object_unref(OBJECT(drc));
> > +        error_propagate(errp, err);
> > +        return;
> >      }
> > -    g_free(child_name);
> >      vmstate_register(DEVICE(drc), spapr_drc_index(drc), &vmstate_spapr_drc,
> >                       drc);
> >      qemu_register_reset(drc_reset, drc);
> > @@ -523,17 +523,14 @@ static void unrealize(DeviceState *d, Error **errp)
> >      sPAPRDRConnector *drc = SPAPR_DR_CONNECTOR(d);
> >      Object *root_container;
> >      gchar *name;
> > -    Error *err = NULL;
> >  
> >      trace_spapr_drc_unrealize(spapr_drc_index(drc));
> > +    qemu_unregister_reset(drc_reset, drc);
> > +    vmstate_unregister(DEVICE(drc), &vmstate_spapr_drc, drc);
> >      root_container = container_get(object_get_root(), DRC_CONTAINER_PATH);
> >      name = g_strdup_printf("%x", spapr_drc_index(drc));
> > -    object_property_del(root_container, name, &err);
> > +    object_property_del(root_container, name, errp);
> >      g_free(name);
> > -    if (err) {
> > -        error_report_err(err);
> > -        object_unref(OBJECT(drc));
> > -    }
> >  }
> >  
> >  sPAPRDRConnector *spapr_dr_connector_new(Object *owner, const char *type,
> >   
>

pgpqlbJnCOhsP.pgp
Description: OpenPGP digital signature