Re: [PATCH 1/7] spapr/pci: Correct "does not support hotplugging error messages

[BALATON Zoltan]

On Wed, 1 Nov 2023, Daniel Henrique Barboza wrote:
> On 10/31/23 08:10, Markus Armbruster wrote:
> > When dynamic-reconfiguration is off, hot plug / unplug can fail with
> > "Bus 'spapr-pci-host-bridge' does not support hotplugging".
> > spapr-pci-host-bridge is a device, not a bus.  Report the name of the
> > bus it provides instead: 'pci.0'.
> >
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > ---
>
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>
> Feel free to queue it up. Thanks,
>
>
> Daniel
>
> >   hw/ppc/spapr_pci.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 370c5a90f2..ebb32ad90b 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1551,7 +1551,7 @@ static void spapr_pci_pre_plug(HotplugHandler 
> > *plug_handler,
> >            */
> >           if (plugged_dev->hotplugged) {
> >               error_setg(errp, QERR_BUS_NO_HOTPLUG,
> > -                       object_get_typename(OBJECT(phb)));
> > +                       phb->parent_obj.bus->qbus.name);

I could not find it mentioned in the docs but it was said the parent 
pointer is private and one should not access it but cast to the parent 
object instead. Or here may even use pci_get_bus(pdev) maybe after moving 
the asserts before it to make sure the device is valid. But I don't mind 
so you can commit it as it is if nobody notices.

Regards,
BALATON Zoltan

> >               return;
> >           }
> >       }
> > @@ -1672,7 +1672,7 @@ static void spapr_pci_unplug_request(HotplugHandler 
> > *plug_handler,
> >         if (!phb->dr_enabled) {
> >           error_setg(errp, QERR_BUS_NO_HOTPLUG,
> > -                   object_get_typename(OBJECT(phb)));
> > +                   phb->parent_obj.bus->qbus.name);
> >           return;
> >       }