[PATCH v3 19/34] spapr: Add return value to spapr_irq_check()

David Gibson posted 34 patches 6 years, 4 months ago
There is a newer version of this series
[PATCH v3 19/34] spapr: Add return value to spapr_irq_check()
Posted by David Gibson 6 years, 4 months ago
Explicitly return success or failure, rather than just relying on the
Error ** parameter.  This makes handling it less verbose in the caller.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr_irq.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
index 3ac67ba0c7..0413fbd0a3 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -529,7 +529,7 @@ SpaprIrq spapr_irq_dual = {
 };
 
 
-static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
+static int spapr_irq_check(SpaprMachineState *spapr, Error **errp)
 {
     MachineState *machine = MACHINE(spapr);
 
@@ -545,7 +545,7 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
          */
         if (spapr->irq == &spapr_irq_dual) {
             spapr->irq = &spapr_irq_xics;
-            return;
+            return 0;
         }
 
         /*
@@ -565,7 +565,7 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
          */
         if (spapr->irq == &spapr_irq_xive) {
             error_setg(errp, "XIVE-only machines require a POWER9 CPU");
-            return;
+            return -1;
         }
     }
 
@@ -579,8 +579,10 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
         machine_kernel_irqchip_required(machine) &&
         xics_kvm_has_broken_disconnect(spapr)) {
         error_setg(errp, "KVM is too old to support ic-mode=dual,kernel-irqchip=on");
-        return;
+        return -1;
     }
+
+    return 0;
 }
 
 /*
@@ -589,7 +591,6 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
 void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
 {
     MachineState *machine = MACHINE(spapr);
-    Error *local_err = NULL;
 
     if (machine_kernel_irqchip_split(machine)) {
         error_setg(errp, "kernel_irqchip split mode not supported on pseries");
@@ -602,9 +603,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
         return;
     }
 
-    spapr_irq_check(spapr, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    if (spapr_irq_check(spapr, errp) < 0) {
         return;
     }
 
-- 
2.21.0


Re: [PATCH v3 19/34] spapr: Add return value to spapr_irq_check()
Posted by Cédric Le Goater 6 years, 4 months ago
On 02/10/2019 04:51, David Gibson wrote:
> Explicitly return success or failure, rather than just relying on the
> Error ** parameter.  This makes handling it less verbose in the caller.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>


Reviewed-by: Cédric Le Goater <clg@kaod.org>
> ---
>  hw/ppc/spapr_irq.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 3ac67ba0c7..0413fbd0a3 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -529,7 +529,7 @@ SpaprIrq spapr_irq_dual = {
>  };
>  
>  
> -static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
> +static int spapr_irq_check(SpaprMachineState *spapr, Error **errp)
>  {
>      MachineState *machine = MACHINE(spapr);
>  
> @@ -545,7 +545,7 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
>           */
>          if (spapr->irq == &spapr_irq_dual) {
>              spapr->irq = &spapr_irq_xics;
> -            return;
> +            return 0;
>          }
>  
>          /*
> @@ -565,7 +565,7 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
>           */
>          if (spapr->irq == &spapr_irq_xive) {
>              error_setg(errp, "XIVE-only machines require a POWER9 CPU");
> -            return;
> +            return -1;
>          }
>      }
>  
> @@ -579,8 +579,10 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
>          machine_kernel_irqchip_required(machine) &&
>          xics_kvm_has_broken_disconnect(spapr)) {
>          error_setg(errp, "KVM is too old to support ic-mode=dual,kernel-irqchip=on");
> -        return;
> +        return -1;
>      }
> +
> +    return 0;
>  }
>  
>  /*
> @@ -589,7 +591,6 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
>  void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
>  {
>      MachineState *machine = MACHINE(spapr);
> -    Error *local_err = NULL;
>  
>      if (machine_kernel_irqchip_split(machine)) {
>          error_setg(errp, "kernel_irqchip split mode not supported on pseries");
> @@ -602,9 +603,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
>          return;
>      }
>  
> -    spapr_irq_check(spapr, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> +    if (spapr_irq_check(spapr, errp) < 0) {
>          return;
>      }
>  
> 


Re: [PATCH v3 19/34] spapr: Add return value to spapr_irq_check()
Posted by Greg Kurz 6 years, 4 months ago
On Wed,  2 Oct 2019 12:51:53 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> Explicitly return success or failure, rather than just relying on the
> Error ** parameter.  This makes handling it less verbose in the caller.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

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

>  hw/ppc/spapr_irq.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 3ac67ba0c7..0413fbd0a3 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -529,7 +529,7 @@ SpaprIrq spapr_irq_dual = {
>  };
>  
>  
> -static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
> +static int spapr_irq_check(SpaprMachineState *spapr, Error **errp)
>  {
>      MachineState *machine = MACHINE(spapr);
>  
> @@ -545,7 +545,7 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
>           */
>          if (spapr->irq == &spapr_irq_dual) {
>              spapr->irq = &spapr_irq_xics;
> -            return;
> +            return 0;
>          }
>  
>          /*
> @@ -565,7 +565,7 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
>           */
>          if (spapr->irq == &spapr_irq_xive) {
>              error_setg(errp, "XIVE-only machines require a POWER9 CPU");
> -            return;
> +            return -1;
>          }
>      }
>  
> @@ -579,8 +579,10 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
>          machine_kernel_irqchip_required(machine) &&
>          xics_kvm_has_broken_disconnect(spapr)) {
>          error_setg(errp, "KVM is too old to support ic-mode=dual,kernel-irqchip=on");
> -        return;
> +        return -1;
>      }
> +
> +    return 0;
>  }
>  
>  /*
> @@ -589,7 +591,6 @@ static void spapr_irq_check(SpaprMachineState *spapr, Error **errp)
>  void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
>  {
>      MachineState *machine = MACHINE(spapr);
> -    Error *local_err = NULL;
>  
>      if (machine_kernel_irqchip_split(machine)) {
>          error_setg(errp, "kernel_irqchip split mode not supported on pseries");
> @@ -602,9 +603,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp)
>          return;
>      }
>  
> -    spapr_irq_check(spapr, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> +    if (spapr_irq_check(spapr, errp) < 0) {
>          return;
>      }
>