On Mon, Aug 10, 2020 at 06:53:58PM +0200, Greg Kurz wrote: > The spapr_phb_realize() function has a local_err variable which > is used to: > > 1) check failures of spapr_irq_findone() and spapr_irq_claim() > > 2) prepend extra information to the error message > > Recent work from Markus Armbruster highlighted we get better > code when testing the return value of a function, rather than > setting up all the local_err boiler plate. For similar reasons, > it is now preferred to use ERRP_GUARD() and error_prepend() > rather than error_propagate_prepend(). > > Since spapr_irq_findone() and spapr_irq_claim() return negative > values in case of failure, do both changes. > > This is just cleanup, no functional impact. > > Signed-off-by: Greg Kurz <gr...@kaod.org> > Reviewed-by: Markus Armbruster <arm...@redhat.com> > Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
Applied to ppc-for-5.2.
> ---
> hw/ppc/spapr_pci.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 363cdb3f7b8d..0a418f1e6711 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1796,6 +1796,7 @@ static void spapr_phb_destroy_msi(gpointer opaque)
>
> static void spapr_phb_realize(DeviceState *dev, Error **errp)
> {
> + ERRP_GUARD();
> /* We don't use SPAPR_MACHINE() in order to exit gracefully if the user
> * tries to add a sPAPR PHB to a non-pseries machine.
> */
> @@ -1813,7 +1814,6 @@ static void spapr_phb_realize(DeviceState *dev, Error
> **errp)
> uint64_t msi_window_size = 4096;
> SpaprTceTable *tcet;
> const unsigned windows_supported = spapr_phb_windows_supported(sphb);
> - Error *local_err = NULL;
>
> if (!spapr) {
> error_setg(errp, TYPE_SPAPR_PCI_HOST_BRIDGE " needs a pseries
> machine");
> @@ -1964,13 +1964,12 @@ static void spapr_phb_realize(DeviceState *dev, Error
> **errp)
>
> /* Initialize the LSI table */
> for (i = 0; i < PCI_NUM_PINS; i++) {
> - uint32_t irq = SPAPR_IRQ_PCI_LSI + sphb->index * PCI_NUM_PINS + i;
> + int irq = SPAPR_IRQ_PCI_LSI + sphb->index * PCI_NUM_PINS + i;
>
> if (smc->legacy_irq_allocation) {
> - irq = spapr_irq_findone(spapr, &local_err);
> - if (local_err) {
> - error_propagate_prepend(errp, local_err,
> - "can't allocate LSIs: ");
> + irq = spapr_irq_findone(spapr, errp);
> + if (irq < 0) {
> + error_prepend(errp, "can't allocate LSIs: ");
> /*
> * Older machines will never support PHB hotplug, ie, this
> is an
> * init only path and QEMU will terminate. No need to
> rollback.
> @@ -1979,9 +1978,8 @@ static void spapr_phb_realize(DeviceState *dev, Error
> **errp)
> }
> }
>
> - spapr_irq_claim(spapr, irq, true, &local_err);
> - if (local_err) {
> - error_propagate_prepend(errp, local_err, "can't allocate LSIs:
> ");
> + if (spapr_irq_claim(spapr, irq, true, errp) < 0) {
> + error_prepend(errp, "can't allocate LSIs: ");
> goto unrealize;
> }
>
>
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
