| Message ID | 20231031111059.3407803-2-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Miscellaneous error message improvements | expand |
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); > 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; > } >
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; >> } >> > >
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; >>> } >>> >> >>
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); 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; }
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> --- hw/ppc/spapr_pci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)