[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/7] spapr/pci: Correct "does not support hotplugging error m
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 1/7] spapr/pci: Correct "does not support hotplugging error messages |
Date: |
Wed, 15 Nov 2023 08:07:41 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
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;
>>> }
>>>
>>
>>