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

[Markus Armbruster <armbru@redhat.com>]

BALATON Zoltan <balaton@eik.bme.hu> writes:

> 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.

pci_get_bus() returns the bus the device is plugged into as a PCI bus.
We need the bus the device provides.  Besides, @phb is plugged into the
main system bus.  pci_get_bus(PCI_DEVICE(phb)) would pass the main
system bus to PCI_BUS(), which is not good.

I can offer

                            PCI_HOST_BRIDGE(phb)->bus->qbus.name);

Looks like a wash to me, but if maintainers like it better, I'll change
to it.

>
> 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;
>>>       }
>>> 
>>
>>