[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [for-2.10 PATCH] spapr_drc: fix realize and
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [for-2.10 PATCH] spapr_drc: fix realize and unrealize |
Date: |
Thu, 3 Aug 2017 00:22:13 +1000 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
On Wed, Aug 02, 2017 at 10:14:56AM +0200, Greg Kurz wrote:
> On Fri, 28 Jul 2017 14:27:45 +1000
> David Gibson <address@hidden> wrote:
>
> > On Thu, Jul 27, 2017 at 03:50:37PM -0500, Michael Roth wrote:
> > > Quoting Greg Kurz (2017-07-27 08:45:47)
> > > > If object_property_add_alias() returns an error in realize(), we should
> > > > propagate it to the caller and certainly not unref the DRC.
> > >
> > > Indeed. I think that was the result of this part of the code
> > > originally living in spapr_dr_connector_new() during development,
> > > where it had previously made sense to free the object if there was a
> > > failure and then return NULL. We definitely shouldn't be during it
> > > now...
> >
> > Yes, I think that's right.
> >
> > I've applied this fix to ppc-for-2.10.
> >
> > >
> > > >
> > > > 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>
> > > > ---
> > > >
> > > > David,
> > > >
> > > > As agreed on the list, here's the version of the fix for 2.10. I'll
> > > > respin
> > > > my PHB hotplug series on top of it.
> > > >
> > >
> > >
> > > Since spapr_dr_connector_new() calls realize() with errp = NULL, we
> > > basically lose the error now. I think maybe we should at least report it
> > > still, but even better would be to have _new() call object_property_* with
> > > error_abort, since callers of spapr_dr_connector_new() don't all check
> > > for the return value and even if they did the appropriate action would
> > > always be to report+abort() anyway.
> >
> > I agree. Well, except I think it should be &error_fatal, not
> > &error_abort. But that can be a follow up patch.
> >
>
> Hmmm... when we'll hotplug a PHB, an error during the realization of
> a PCI DRC would then terminate QEMU. I'd rather add an errp argument
> to spapr_dr_connector_new() and propagate the error instead.
Ah, good point.
--
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