Quoting David Gibson (2017-06-06 08:05:32)
> This function is fairly short, and so is its only caller. There's no
> particular logical distinction between them, so fold them together.
>
> Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
> hw/ppc/spapr_pci.c | 53 ++++++++++++++++++++++-------------------------------
> 1 file changed, 22 insertions(+), 31 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 46e736d..a216f61 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1344,30 +1344,6 @@ static int spapr_create_pci_child_dt(sPAPRPHBState
> *phb, PCIDevice *dev,
> return offset;
> }
>
> -static void spapr_phb_add_pci_device(sPAPRDRConnector *drc,
> - sPAPRPHBState *phb,
> - PCIDevice *pdev,
> - Error **errp)
> -{
> - DeviceState *dev = DEVICE(pdev);
> - void *fdt = NULL;
> - int fdt_start_offset = 0, fdt_size;
> -
> - fdt = create_device_tree(&fdt_size);
> - fdt_start_offset = spapr_create_pci_child_dt(phb, pdev, fdt, 0);
> - if (!fdt_start_offset) {
> - error_setg(errp, "Failed to create pci child device tree node");
> - goto out;
> - }
> -
> - spapr_drc_attach(drc, DEVICE(pdev),
> - fdt, fdt_start_offset, !dev->hotplugged, errp);
> -out:
> - if (*errp) {
> - g_free(fdt);
> - }
> -}
> -
> /* Callback to be called during DRC release. */
> void spapr_phb_remove_pci_device_cb(DeviceState *dev)
> {
> @@ -1429,6 +1405,8 @@ static void spapr_phb_hot_plug_child(HotplugHandler
> *plug_handler,
> Error *local_err = NULL;
> PCIBus *bus = PCI_BUS(qdev_get_parent_bus(DEVICE(pdev)));
> uint32_t slotnr = PCI_SLOT(pdev->devfn);
> + void *fdt = NULL;
> + int fdt_start_offset, fdt_size;
>
> /* if DR is disabled we don't need to do anything in the case of
> * hotplug or coldplug callbacks
> @@ -1438,10 +1416,10 @@ static void spapr_phb_hot_plug_child(HotplugHandler
> *plug_handler,
> * we need to let them know it's not enabled
> */
> if (plugged_dev->hotplugged) {
> - error_setg(errp, QERR_BUS_NO_HOTPLUG,
> + error_setg(&local_err, QERR_BUS_NO_HOTPLUG,
> object_get_typename(OBJECT(phb)));
> }
> - return;
> + goto out;
> }
>
> g_assert(drc);
> @@ -1452,16 +1430,23 @@ static void spapr_phb_hot_plug_child(HotplugHandler
> *plug_handler,
> */
> if (plugged_dev->hotplugged && bus->devices[PCI_DEVFN(slotnr, 0)] &&
> PCI_FUNC(pdev->devfn) != 0) {
> - error_setg(errp, "PCI: slot %d function 0 already ocuppied by %s,"
> + error_setg(&local_err, "PCI: slot %d function 0 already ocuppied by
> %s,"
> " additional functions can no longer be exposed to
> guest.",
> slotnr, bus->devices[PCI_DEVFN(slotnr, 0)]->name);
> - return;
> + goto out;
> }
>
> - spapr_phb_add_pci_device(drc, phb, pdev, &local_err);
Since we never used local_err outside propagating it and immediately
bailing, and since we bail on errp in all prior callers, maybe we
should just drop local_err completely in favor errp.
> + fdt = create_device_tree(&fdt_size);
> + fdt_start_offset = spapr_create_pci_child_dt(phb, pdev, fdt, 0);
> + if (!fdt_start_offset) {
> + error_setg(&local_err, "Failed to create pci child device tree
> node");
> + goto out;
> + }
> +
> + spapr_drc_attach(drc, DEVICE(pdev), fdt, fdt_start_offset,
> + !plugged_dev->hotplugged, &local_err);
> if (local_err) {
> - error_propagate(errp, local_err);
> - return;
> + goto out;
> }
>
> /* If this is function 0, signal hotplug for all the device functions.
> @@ -1485,6 +1470,12 @@ static void spapr_phb_hot_plug_child(HotplugHandler
> *plug_handler,
> }
> }
> }
> +
> +out:
> + if (local_err) {
> + error_propagate(errp, local_err);
> + g_free(fdt);
> + }
> }
>
> static void spapr_phb_hot_unplug_child(HotplugHandler *plug_handler,
> --
> 2.9.4
>
