diff mbox series

[16/17] spapr_pci: Drop some dead error handling

Message ID 20200428163419.4483-17-armbru@redhat.com
State New
Headers show
Series qom: Spring cleaning | expand

Commit Message

Markus Armbruster April 28, 2020, 4:34 p.m. UTC 
chassis_from_bus() uses object_property_get_uint() to get property
"chassis_nr" of the bridge device.  Failure would be a programming
error.  Pass &error_abort, and simplify its callers.

Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/ppc/spapr_pci.c | 86 ++++++++++------------------------------------
 1 file changed, 18 insertions(+), 68 deletions(-)

Comments

David Gibson April 29, 2020, 1:22 a.m. UTC | #1 
On Tue, Apr 28, 2020 at 06:34:18PM +0200, Markus Armbruster wrote:
> chassis_from_bus() uses object_property_get_uint() to get property
> "chassis_nr" of the bridge device.  Failure would be a programming
> error.  Pass &error_abort, and simplify its callers.
> 
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/ppc/spapr_pci.c | 86 ++++++++++------------------------------------
>  1 file changed, 18 insertions(+), 68 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 1d73d05a0a..b6036be51c 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1203,46 +1203,36 @@ static SpaprDrc *drc_from_devfn(SpaprPhbState *phb,
>                             drc_id_from_devfn(phb, chassis, devfn));
>  }
>  
> -static uint8_t chassis_from_bus(PCIBus *bus, Error **errp)
> +static uint8_t chassis_from_bus(PCIBus *bus)
>  {
>      if (pci_bus_is_root(bus)) {
>          return 0;
>      } else {
>          PCIDevice *bridge = pci_bridge_get_device(bus);
>  
> -        return object_property_get_uint(OBJECT(bridge), "chassis_nr", errp);
> +        return object_property_get_uint(OBJECT(bridge), "chassis_nr",
> +                                        &error_abort);
>      }
>  }
>  
>  static SpaprDrc *drc_from_dev(SpaprPhbState *phb, PCIDevice *dev)
>  {
> -    Error *local_err = NULL;
> -    uint8_t chassis = chassis_from_bus(pci_get_bus(dev), &local_err);
> -
> -    if (local_err) {
> -        error_report_err(local_err);
> -        return NULL;
> -    }
> +    uint8_t chassis = chassis_from_bus(pci_get_bus(dev));
>  
>      return drc_from_devfn(phb, chassis, dev->devfn);
>  }
>  
> -static void add_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
> +static void add_drcs(SpaprPhbState *phb, PCIBus *bus)
>  {
>      Object *owner;
>      int i;
>      uint8_t chassis;
> -    Error *local_err = NULL;
>  
>      if (!phb->dr_enabled) {
>          return;
>      }
>  
> -    chassis = chassis_from_bus(bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    chassis = chassis_from_bus(bus);
>  
>      if (pci_bus_is_root(bus)) {
>          owner = OBJECT(phb);
> @@ -1256,21 +1246,16 @@ static void add_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
>      }
>  }
>  
> -static void remove_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
> +static void remove_drcs(SpaprPhbState *phb, PCIBus *bus)
>  {
>      int i;
>      uint8_t chassis;
> -    Error *local_err = NULL;
>  
>      if (!phb->dr_enabled) {
>          return;
>      }
>  
> -    chassis = chassis_from_bus(bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    chassis = chassis_from_bus(bus);
>  
>      for (i = PCI_SLOT_MAX * PCI_FUNC_MAX - 1; i >= 0; i--) {
>          SpaprDrc *drc = drc_from_devfn(phb, chassis, i);
> @@ -1488,17 +1473,11 @@ int spapr_pci_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr,
>  }
>  
>  static void spapr_pci_bridge_plug(SpaprPhbState *phb,
> -                                  PCIBridge *bridge,
> -                                  Error **errp)
> +                                  PCIBridge *bridge)
>  {
> -    Error *local_err = NULL;
>      PCIBus *bus = pci_bridge_get_sec_bus(bridge);
>  
> -    add_drcs(phb, bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    add_drcs(phb, bus);
>  }
>  
>  static void spapr_pci_plug(HotplugHandler *plug_handler,
> @@ -1529,11 +1508,7 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
>      g_assert(drc);
>  
>      if (pc->is_bridge) {
> -        spapr_pci_bridge_plug(phb, PCI_BRIDGE(plugged_dev), &local_err);
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> -            return;
> -        }
> +        spapr_pci_bridge_plug(phb, PCI_BRIDGE(plugged_dev));
>      }
>  
>      /* Following the QEMU convention used for PCIe multifunction
> @@ -1560,12 +1535,7 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
>          spapr_drc_reset(drc);
>      } else if (PCI_FUNC(pdev->devfn) == 0) {
>          int i;
> -        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev), &local_err);
> -
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> -            return;
> -        }
> +        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev));
>  
>          for (i = 0; i < 8; i++) {
>              SpaprDrc *func_drc;
> @@ -1587,17 +1557,11 @@ out:
>  }
>  
>  static void spapr_pci_bridge_unplug(SpaprPhbState *phb,
> -                                    PCIBridge *bridge,
> -                                    Error **errp)
> +                                    PCIBridge *bridge)
>  {
> -    Error *local_err = NULL;
>      PCIBus *bus = pci_bridge_get_sec_bus(bridge);
>  
> -    remove_drcs(phb, bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    remove_drcs(phb, bus);
>  }
>  
>  static void spapr_pci_unplug(HotplugHandler *plug_handler,
> @@ -1619,11 +1583,7 @@ static void spapr_pci_unplug(HotplugHandler *plug_handler,
>      pci_device_reset(PCI_DEVICE(plugged_dev));
>  
>      if (pc->is_bridge) {
> -        Error *local_err = NULL;
> -        spapr_pci_bridge_unplug(phb, PCI_BRIDGE(plugged_dev), &local_err);
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> -        }
> +        spapr_pci_bridge_unplug(phb, PCI_BRIDGE(plugged_dev));
>          return;
>      }
>  
> @@ -1654,13 +1614,7 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
>          SpaprDrcClass *func_drck;
>          SpaprDREntitySense state;
>          int i;
> -        Error *local_err = NULL;
> -        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev), &local_err);
> -
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> -            return;
> -        }
> +        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev));
>  
>          if (pc->is_bridge) {
>              error_setg(errp, "PCI: Hot unplug of PCI bridges not supported");
> @@ -1741,7 +1695,7 @@ static void spapr_phb_unrealize(DeviceState *dev)
>          }
>      }
>  
> -    remove_drcs(sphb, phb->bus, &error_abort);
> +    remove_drcs(sphb, phb->bus);
>  
>      for (i = PCI_NUM_PINS - 1; i >= 0; i--) {
>          if (sphb->lsi_table[i].irq) {
> @@ -1980,11 +1934,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>      }
>  
>      /* allocate connectors for child PCI devices */
> -    add_drcs(sphb, phb->bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        goto unrealize;
> -    }
> +    add_drcs(sphb, phb->bus);
>  
>      /* DMA setup */
>      for (i = 0; i < windows_supported; ++i) {
Greg Kurz April 29, 2020, 7:03 a.m. UTC | #2 
On Tue, 28 Apr 2020 18:34:18 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> chassis_from_bus() uses object_property_get_uint() to get property
> "chassis_nr" of the bridge device.  Failure would be a programming
> error.  Pass &error_abort, and simplify its callers.
> 
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr_pci.c | 86 ++++++++++------------------------------------
>  1 file changed, 18 insertions(+), 68 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 1d73d05a0a..b6036be51c 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1203,46 +1203,36 @@ static SpaprDrc *drc_from_devfn(SpaprPhbState *phb,
>                             drc_id_from_devfn(phb, chassis, devfn));
>  }
>  
> -static uint8_t chassis_from_bus(PCIBus *bus, Error **errp)
> +static uint8_t chassis_from_bus(PCIBus *bus)
>  {
>      if (pci_bus_is_root(bus)) {
>          return 0;
>      } else {
>          PCIDevice *bridge = pci_bridge_get_device(bus);
>  
> -        return object_property_get_uint(OBJECT(bridge), "chassis_nr", errp);
> +        return object_property_get_uint(OBJECT(bridge), "chassis_nr",
> +                                        &error_abort);
>      }
>  }
>  
>  static SpaprDrc *drc_from_dev(SpaprPhbState *phb, PCIDevice *dev)
>  {
> -    Error *local_err = NULL;
> -    uint8_t chassis = chassis_from_bus(pci_get_bus(dev), &local_err);
> -
> -    if (local_err) {
> -        error_report_err(local_err);
> -        return NULL;
> -    }
> +    uint8_t chassis = chassis_from_bus(pci_get_bus(dev));
>  
>      return drc_from_devfn(phb, chassis, dev->devfn);
>  }
>  
> -static void add_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
> +static void add_drcs(SpaprPhbState *phb, PCIBus *bus)
>  {
>      Object *owner;
>      int i;
>      uint8_t chassis;
> -    Error *local_err = NULL;
>  
>      if (!phb->dr_enabled) {
>          return;
>      }
>  
> -    chassis = chassis_from_bus(bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    chassis = chassis_from_bus(bus);
>  
>      if (pci_bus_is_root(bus)) {
>          owner = OBJECT(phb);
> @@ -1256,21 +1246,16 @@ static void add_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
>      }
>  }
>  
> -static void remove_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
> +static void remove_drcs(SpaprPhbState *phb, PCIBus *bus)
>  {
>      int i;
>      uint8_t chassis;
> -    Error *local_err = NULL;
>  
>      if (!phb->dr_enabled) {
>          return;
>      }
>  
> -    chassis = chassis_from_bus(bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    chassis = chassis_from_bus(bus);
>  
>      for (i = PCI_SLOT_MAX * PCI_FUNC_MAX - 1; i >= 0; i--) {
>          SpaprDrc *drc = drc_from_devfn(phb, chassis, i);
> @@ -1488,17 +1473,11 @@ int spapr_pci_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr,
>  }
>  
>  static void spapr_pci_bridge_plug(SpaprPhbState *phb,
> -                                  PCIBridge *bridge,
> -                                  Error **errp)
> +                                  PCIBridge *bridge)
>  {
> -    Error *local_err = NULL;
>      PCIBus *bus = pci_bridge_get_sec_bus(bridge);
>  
> -    add_drcs(phb, bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    add_drcs(phb, bus);
>  }
>  
>  static void spapr_pci_plug(HotplugHandler *plug_handler,
> @@ -1529,11 +1508,7 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
>      g_assert(drc);
>  
>      if (pc->is_bridge) {
> -        spapr_pci_bridge_plug(phb, PCI_BRIDGE(plugged_dev), &local_err);
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> -            return;
> -        }
> +        spapr_pci_bridge_plug(phb, PCI_BRIDGE(plugged_dev));
>      }
>  
>      /* Following the QEMU convention used for PCIe multifunction
> @@ -1560,12 +1535,7 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
>          spapr_drc_reset(drc);
>      } else if (PCI_FUNC(pdev->devfn) == 0) {
>          int i;
> -        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev), &local_err);
> -
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> -            return;
> -        }
> +        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev));
>  
>          for (i = 0; i < 8; i++) {
>              SpaprDrc *func_drc;
> @@ -1587,17 +1557,11 @@ out:
>  }
>  
>  static void spapr_pci_bridge_unplug(SpaprPhbState *phb,
> -                                    PCIBridge *bridge,
> -                                    Error **errp)
> +                                    PCIBridge *bridge)
>  {
> -    Error *local_err = NULL;
>      PCIBus *bus = pci_bridge_get_sec_bus(bridge);
>  
> -    remove_drcs(phb, bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    remove_drcs(phb, bus);
>  }
>  
>  static void spapr_pci_unplug(HotplugHandler *plug_handler,
> @@ -1619,11 +1583,7 @@ static void spapr_pci_unplug(HotplugHandler *plug_handler,
>      pci_device_reset(PCI_DEVICE(plugged_dev));
>  
>      if (pc->is_bridge) {
> -        Error *local_err = NULL;
> -        spapr_pci_bridge_unplug(phb, PCI_BRIDGE(plugged_dev), &local_err);
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> -        }
> +        spapr_pci_bridge_unplug(phb, PCI_BRIDGE(plugged_dev));
>          return;
>      }
>  
> @@ -1654,13 +1614,7 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
>          SpaprDrcClass *func_drck;
>          SpaprDREntitySense state;
>          int i;
> -        Error *local_err = NULL;
> -        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev), &local_err);
> -
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> -            return;
> -        }
> +        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev));
>  
>          if (pc->is_bridge) {
>              error_setg(errp, "PCI: Hot unplug of PCI bridges not supported");
> @@ -1741,7 +1695,7 @@ static void spapr_phb_unrealize(DeviceState *dev)
>          }
>      }
>  
> -    remove_drcs(sphb, phb->bus, &error_abort);
> +    remove_drcs(sphb, phb->bus);
>  
>      for (i = PCI_NUM_PINS - 1; i >= 0; i--) {
>          if (sphb->lsi_table[i].irq) {
> @@ -1980,11 +1934,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>      }
>  
>      /* allocate connectors for child PCI devices */
> -    add_drcs(sphb, phb->bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        goto unrealize;
> -    }
> +    add_drcs(sphb, phb->bus);
>  
>      /* DMA setup */
>      for (i = 0; i < windows_supported; ++i) {
Philippe Mathieu-Daudé April 29, 2020, 7:35 a.m. UTC | #3 
On 4/28/20 6:34 PM, Markus Armbruster wrote:
> chassis_from_bus() uses object_property_get_uint() to get property
> "chassis_nr" of the bridge device.  Failure would be a programming
> error.  Pass &error_abort, and simplify its callers.
> 
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   hw/ppc/spapr_pci.c | 86 ++++++++++------------------------------------
>   1 file changed, 18 insertions(+), 68 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 1d73d05a0a..b6036be51c 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1203,46 +1203,36 @@ static SpaprDrc *drc_from_devfn(SpaprPhbState *phb,
>                              drc_id_from_devfn(phb, chassis, devfn));
>   }
>   
> -static uint8_t chassis_from_bus(PCIBus *bus, Error **errp)
> +static uint8_t chassis_from_bus(PCIBus *bus)
>   {
>       if (pci_bus_is_root(bus)) {
>           return 0;
>       } else {
>           PCIDevice *bridge = pci_bridge_get_device(bus);
>   
> -        return object_property_get_uint(OBJECT(bridge), "chassis_nr", errp);
> +        return object_property_get_uint(OBJECT(bridge), "chassis_nr",
> +                                        &error_abort);
>       }
>   }
>   
>   static SpaprDrc *drc_from_dev(SpaprPhbState *phb, PCIDevice *dev)
>   {
> -    Error *local_err = NULL;
> -    uint8_t chassis = chassis_from_bus(pci_get_bus(dev), &local_err);
> -
> -    if (local_err) {
> -        error_report_err(local_err);
> -        return NULL;
> -    }
> +    uint8_t chassis = chassis_from_bus(pci_get_bus(dev));
>   
>       return drc_from_devfn(phb, chassis, dev->devfn);
>   }
>   
> -static void add_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
> +static void add_drcs(SpaprPhbState *phb, PCIBus *bus)
>   {
>       Object *owner;
>       int i;
>       uint8_t chassis;
> -    Error *local_err = NULL;
>   
>       if (!phb->dr_enabled) {
>           return;
>       }
>   
> -    chassis = chassis_from_bus(bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    chassis = chassis_from_bus(bus);
>   
>       if (pci_bus_is_root(bus)) {
>           owner = OBJECT(phb);
> @@ -1256,21 +1246,16 @@ static void add_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
>       }
>   }
>   
> -static void remove_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
> +static void remove_drcs(SpaprPhbState *phb, PCIBus *bus)
>   {
>       int i;
>       uint8_t chassis;
> -    Error *local_err = NULL;
>   
>       if (!phb->dr_enabled) {
>           return;
>       }
>   
> -    chassis = chassis_from_bus(bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    chassis = chassis_from_bus(bus);
>   
>       for (i = PCI_SLOT_MAX * PCI_FUNC_MAX - 1; i >= 0; i--) {
>           SpaprDrc *drc = drc_from_devfn(phb, chassis, i);
> @@ -1488,17 +1473,11 @@ int spapr_pci_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr,
>   }
>   
>   static void spapr_pci_bridge_plug(SpaprPhbState *phb,
> -                                  PCIBridge *bridge,
> -                                  Error **errp)
> +                                  PCIBridge *bridge)
>   {
> -    Error *local_err = NULL;
>       PCIBus *bus = pci_bridge_get_sec_bus(bridge);
>   
> -    add_drcs(phb, bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    add_drcs(phb, bus);
>   }
>   
>   static void spapr_pci_plug(HotplugHandler *plug_handler,
> @@ -1529,11 +1508,7 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
>       g_assert(drc);
>   
>       if (pc->is_bridge) {
> -        spapr_pci_bridge_plug(phb, PCI_BRIDGE(plugged_dev), &local_err);
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> -            return;
> -        }
> +        spapr_pci_bridge_plug(phb, PCI_BRIDGE(plugged_dev));
>       }
>   
>       /* Following the QEMU convention used for PCIe multifunction
> @@ -1560,12 +1535,7 @@ static void spapr_pci_plug(HotplugHandler *plug_handler,
>           spapr_drc_reset(drc);
>       } else if (PCI_FUNC(pdev->devfn) == 0) {
>           int i;
> -        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev), &local_err);
> -
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> -            return;
> -        }
> +        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev));
>   
>           for (i = 0; i < 8; i++) {
>               SpaprDrc *func_drc;
> @@ -1587,17 +1557,11 @@ out:
>   }
>   
>   static void spapr_pci_bridge_unplug(SpaprPhbState *phb,
> -                                    PCIBridge *bridge,
> -                                    Error **errp)
> +                                    PCIBridge *bridge)
>   {
> -    Error *local_err = NULL;
>       PCIBus *bus = pci_bridge_get_sec_bus(bridge);
>   
> -    remove_drcs(phb, bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    remove_drcs(phb, bus);
>   }
>   
>   static void spapr_pci_unplug(HotplugHandler *plug_handler,
> @@ -1619,11 +1583,7 @@ static void spapr_pci_unplug(HotplugHandler *plug_handler,
>       pci_device_reset(PCI_DEVICE(plugged_dev));
>   
>       if (pc->is_bridge) {
> -        Error *local_err = NULL;
> -        spapr_pci_bridge_unplug(phb, PCI_BRIDGE(plugged_dev), &local_err);
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> -        }
> +        spapr_pci_bridge_unplug(phb, PCI_BRIDGE(plugged_dev));
>           return;
>       }
>   
> @@ -1654,13 +1614,7 @@ static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
>           SpaprDrcClass *func_drck;
>           SpaprDREntitySense state;
>           int i;
> -        Error *local_err = NULL;
> -        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev), &local_err);
> -
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> -            return;
> -        }
> +        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev));
>   
>           if (pc->is_bridge) {
>               error_setg(errp, "PCI: Hot unplug of PCI bridges not supported");
> @@ -1741,7 +1695,7 @@ static void spapr_phb_unrealize(DeviceState *dev)
>           }
>       }
>   
> -    remove_drcs(sphb, phb->bus, &error_abort);
> +    remove_drcs(sphb, phb->bus);
>   
>       for (i = PCI_NUM_PINS - 1; i >= 0; i--) {
>           if (sphb->lsi_table[i].irq) {
> @@ -1980,11 +1934,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>       }
>   
>       /* allocate connectors for child PCI devices */
> -    add_drcs(sphb, phb->bus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        goto unrealize;
> -    }
> +    add_drcs(sphb, phb->bus);
>   
>       /* DMA setup */
>       for (i = 0; i < windows_supported; ++i) {
> 

Very nice cleanup.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
diff mbox series

Patch

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 1d73d05a0a..b6036be51c 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1203,46 +1203,36 @@  static SpaprDrc *drc_from_devfn(SpaprPhbState *phb,
                            drc_id_from_devfn(phb, chassis, devfn));
 }
 
-static uint8_t chassis_from_bus(PCIBus *bus, Error **errp)
+static uint8_t chassis_from_bus(PCIBus *bus)
 {
     if (pci_bus_is_root(bus)) {
         return 0;
     } else {
         PCIDevice *bridge = pci_bridge_get_device(bus);
 
-        return object_property_get_uint(OBJECT(bridge), "chassis_nr", errp);
+        return object_property_get_uint(OBJECT(bridge), "chassis_nr",
+                                        &error_abort);
     }
 }
 
 static SpaprDrc *drc_from_dev(SpaprPhbState *phb, PCIDevice *dev)
 {
-    Error *local_err = NULL;
-    uint8_t chassis = chassis_from_bus(pci_get_bus(dev), &local_err);
-
-    if (local_err) {
-        error_report_err(local_err);
-        return NULL;
-    }
+    uint8_t chassis = chassis_from_bus(pci_get_bus(dev));
 
     return drc_from_devfn(phb, chassis, dev->devfn);
 }
 
-static void add_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
+static void add_drcs(SpaprPhbState *phb, PCIBus *bus)
 {
     Object *owner;
     int i;
     uint8_t chassis;
-    Error *local_err = NULL;
 
     if (!phb->dr_enabled) {
         return;
     }
 
-    chassis = chassis_from_bus(bus, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
+    chassis = chassis_from_bus(bus);
 
     if (pci_bus_is_root(bus)) {
         owner = OBJECT(phb);
@@ -1256,21 +1246,16 @@  static void add_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
     }
 }
 
-static void remove_drcs(SpaprPhbState *phb, PCIBus *bus, Error **errp)
+static void remove_drcs(SpaprPhbState *phb, PCIBus *bus)
 {
     int i;
     uint8_t chassis;
-    Error *local_err = NULL;
 
     if (!phb->dr_enabled) {
         return;
     }
 
-    chassis = chassis_from_bus(bus, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
+    chassis = chassis_from_bus(bus);
 
     for (i = PCI_SLOT_MAX * PCI_FUNC_MAX - 1; i >= 0; i--) {
         SpaprDrc *drc = drc_from_devfn(phb, chassis, i);
@@ -1488,17 +1473,11 @@  int spapr_pci_dt_populate(SpaprDrc *drc, SpaprMachineState *spapr,
 }
 
 static void spapr_pci_bridge_plug(SpaprPhbState *phb,
-                                  PCIBridge *bridge,
-                                  Error **errp)
+                                  PCIBridge *bridge)
 {
-    Error *local_err = NULL;
     PCIBus *bus = pci_bridge_get_sec_bus(bridge);
 
-    add_drcs(phb, bus, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
+    add_drcs(phb, bus);
 }
 
 static void spapr_pci_plug(HotplugHandler *plug_handler,
@@ -1529,11 +1508,7 @@  static void spapr_pci_plug(HotplugHandler *plug_handler,
     g_assert(drc);
 
     if (pc->is_bridge) {
-        spapr_pci_bridge_plug(phb, PCI_BRIDGE(plugged_dev), &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
-            return;
-        }
+        spapr_pci_bridge_plug(phb, PCI_BRIDGE(plugged_dev));
     }
 
     /* Following the QEMU convention used for PCIe multifunction
@@ -1560,12 +1535,7 @@  static void spapr_pci_plug(HotplugHandler *plug_handler,
         spapr_drc_reset(drc);
     } else if (PCI_FUNC(pdev->devfn) == 0) {
         int i;
-        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev), &local_err);
-
-        if (local_err) {
-            error_propagate(errp, local_err);
-            return;
-        }
+        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev));
 
         for (i = 0; i < 8; i++) {
             SpaprDrc *func_drc;
@@ -1587,17 +1557,11 @@  out:
 }
 
 static void spapr_pci_bridge_unplug(SpaprPhbState *phb,
-                                    PCIBridge *bridge,
-                                    Error **errp)
+                                    PCIBridge *bridge)
 {
-    Error *local_err = NULL;
     PCIBus *bus = pci_bridge_get_sec_bus(bridge);
 
-    remove_drcs(phb, bus, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
+    remove_drcs(phb, bus);
 }
 
 static void spapr_pci_unplug(HotplugHandler *plug_handler,
@@ -1619,11 +1583,7 @@  static void spapr_pci_unplug(HotplugHandler *plug_handler,
     pci_device_reset(PCI_DEVICE(plugged_dev));
 
     if (pc->is_bridge) {
-        Error *local_err = NULL;
-        spapr_pci_bridge_unplug(phb, PCI_BRIDGE(plugged_dev), &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
-        }
+        spapr_pci_bridge_unplug(phb, PCI_BRIDGE(plugged_dev));
         return;
     }
 
@@ -1654,13 +1614,7 @@  static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
         SpaprDrcClass *func_drck;
         SpaprDREntitySense state;
         int i;
-        Error *local_err = NULL;
-        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev), &local_err);
-
-        if (local_err) {
-            error_propagate(errp, local_err);
-            return;
-        }
+        uint8_t chassis = chassis_from_bus(pci_get_bus(pdev));
 
         if (pc->is_bridge) {
             error_setg(errp, "PCI: Hot unplug of PCI bridges not supported");
@@ -1741,7 +1695,7 @@  static void spapr_phb_unrealize(DeviceState *dev)
         }
     }
 
-    remove_drcs(sphb, phb->bus, &error_abort);
+    remove_drcs(sphb, phb->bus);
 
     for (i = PCI_NUM_PINS - 1; i >= 0; i--) {
         if (sphb->lsi_table[i].irq) {
@@ -1980,11 +1934,7 @@  static void spapr_phb_realize(DeviceState *dev, Error **errp)
     }
 
     /* allocate connectors for child PCI devices */
-    add_drcs(sphb, phb->bus, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        goto unrealize;
-    }
+    add_drcs(sphb, phb->bus);
 
     /* DMA setup */
     for (i = 0; i < windows_supported; ++i) {