On Wed, Jul 26, 2017 at 11:36:43AM +0200, Greg Kurz wrote: > On Wed, 26 Jul 2017 14:04:59 +1000 > David Gibson <da...@gibson.dropbear.id.au> 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 <gr...@kaod.org> > > > > 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 > ?
Yes, that would be helpful, thank you.
>
> > > ---
> > > 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,
> > >
> >
>
--
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
