On Thu, Mar 26, 2020 at 01:18:24PM +0100, Greg Kurz wrote:
> On Thu, 26 Mar 2020 16:40:08 +1100
> David Gibson <da...@gibson.dropbear.id.au> wrote:
>
> > For various technical reasons we can't currently allow unplug a PCI to PCI
> > bridge on the pseries machine. spapr_pci_unplug_request() correctly
> > generates an error message if that's attempted.
> >
> > But.. if the given errp is not error_abort or error_fatal,
>
> Which is the always case when trying to unplug a device AFAICT:
>
> void qdev_unplug(DeviceState *dev, Error **errp)
> {
> DeviceClass *dc = DEVICE_GET_CLASS(dev);
> HotplugHandler *hotplug_ctrl;
> HotplugHandlerClass *hdc;
> Error *local_err = NULL;
>
> [...]
> hdc = HOTPLUG_HANDLER_GET_CLASS(hotplug_ctrl);
> if (hdc->unplug_request) {
> hotplug_handler_unplug_request(hotplug_ctrl, dev, &local_err);
>
> And anyway, spapr_pci_unplug_request() shouldn't rely on the caller
> passing &error_fatal or &error_abort to do the right thing. Calling
> error_setg() without returning right away is a dangerous practice
> since it would cause a subsequent call to error_setg() with the
> same errp to abort QEMU.
>
> > it doesn't actually stop trying to unplug the bridge anyway.
> >
>
> This looks like a bug fix that could go to 5.0 IMHO.
Fair point. I've added the tag and moved to ppc-for-5.0.
>
> Maybe add this tag ?
>
> Fixes: 14e714900f6b "spapr: Allow hot plug/unplug of PCI bridges and
> devices under PCI bridges"
>
> > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> > ---
>
> Reviewed-by: Greg Kurz <gr...@kaod.org>
>
> > hw/ppc/spapr_pci.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 709a52780d..55ca9dee1e 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1663,6 +1663,7 @@ static void spapr_pci_unplug_request(HotplugHandler
> > *plug_handler,
> >
> > if (pc->is_bridge) {
> > error_setg(errp, "PCI: Hot unplug of PCI bridges not
> > supported");
> > + return;
> > }
> >
> > /* ensure any other present functions are pending unplug */
>
--
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
