Re: [PATCH 06/14] spapr/xive: Rework error handling of kvmppc_xive_[gs]et_queue_config()

[David Gibson]

On Mon, Aug 10, 2020 at 06:54:33PM +0200, Greg Kurz wrote:
> Since kvm_device_access() returns a negative errno on failure, convert
> kvmppc_xive_get_queue_config() and kvmppc_xive_set_queue_config() to
> use it for error checking. This allows to get rid of the local_err
> boilerplate.
> 
> Propagate the return value so that callers may use it as well to check
> failures.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-5.2.

> ---
>  hw/intc/spapr_xive_kvm.c    |   35 ++++++++++++++++-------------------
>  include/hw/ppc/spapr_xive.h |    4 ++--
>  2 files changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
> index 5e088ccbf885..696623f717b7 100644
> --- a/hw/intc/spapr_xive_kvm.c
> +++ b/hw/intc/spapr_xive_kvm.c
> @@ -374,15 +374,15 @@ void kvmppc_xive_source_set_irq(void *opaque, int 
> srcno, int val)
>  /*
>   * sPAPR XIVE interrupt controller (KVM)
>   */
> -void kvmppc_xive_get_queue_config(SpaprXive *xive, uint8_t end_blk,
> -                                  uint32_t end_idx, XiveEND *end,
> -                                  Error **errp)
> +int kvmppc_xive_get_queue_config(SpaprXive *xive, uint8_t end_blk,
> +                                 uint32_t end_idx, XiveEND *end,
> +                                 Error **errp)
>  {
>      struct kvm_ppc_xive_eq kvm_eq = { 0 };
>      uint64_t kvm_eq_idx;
>      uint8_t priority;
>      uint32_t server;
> -    Error *local_err = NULL;
> +    int ret;
>  
>      assert(xive_end_is_valid(end));
>  
> @@ -394,11 +394,10 @@ void kvmppc_xive_get_queue_config(SpaprXive *xive, 
> uint8_t end_blk,
>      kvm_eq_idx |= server << KVM_XIVE_EQ_SERVER_SHIFT &
>          KVM_XIVE_EQ_SERVER_MASK;
>  
> -    kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_EQ_CONFIG, kvm_eq_idx,
> -                      &kvm_eq, false, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> +    ret = kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_EQ_CONFIG, kvm_eq_idx,
> +                            &kvm_eq, false, errp);
> +    if (ret < 0) {
> +        return ret;
>      }
>  
>      /*
> @@ -408,17 +407,18 @@ void kvmppc_xive_get_queue_config(SpaprXive *xive, 
> uint8_t end_blk,
>       */
>      end->w1 = xive_set_field32(END_W1_GENERATION, 0ul, kvm_eq.qtoggle) |
>          xive_set_field32(END_W1_PAGE_OFF, 0ul, kvm_eq.qindex);
> +
> +    return 0;
>  }
>  
> -void kvmppc_xive_set_queue_config(SpaprXive *xive, uint8_t end_blk,
> -                                  uint32_t end_idx, XiveEND *end,
> -                                  Error **errp)
> +int kvmppc_xive_set_queue_config(SpaprXive *xive, uint8_t end_blk,
> +                                 uint32_t end_idx, XiveEND *end,
> +                                 Error **errp)
>  {
>      struct kvm_ppc_xive_eq kvm_eq = { 0 };
>      uint64_t kvm_eq_idx;
>      uint8_t priority;
>      uint32_t server;
> -    Error *local_err = NULL;
>  
>      /*
>       * Build the KVM state from the local END structure.
> @@ -456,12 +456,9 @@ void kvmppc_xive_set_queue_config(SpaprXive *xive, 
> uint8_t end_blk,
>      kvm_eq_idx |= server << KVM_XIVE_EQ_SERVER_SHIFT &
>          KVM_XIVE_EQ_SERVER_MASK;
>  
> -    kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_EQ_CONFIG, kvm_eq_idx,
> -                      &kvm_eq, true, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    return
> +        kvm_device_access(xive->fd, KVM_DEV_XIVE_GRP_EQ_CONFIG, kvm_eq_idx,
> +                          &kvm_eq, true, errp);
>  }
>  
>  void kvmppc_xive_reset(SpaprXive *xive, Error **errp)
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index 93d09d68deb7..d0a08b618f79 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -85,10 +85,10 @@ void kvmppc_xive_set_source_config(SpaprXive *xive, 
> uint32_t lisn, XiveEAS *eas,
>  void kvmppc_xive_sync_source(SpaprXive *xive, uint32_t lisn, Error **errp);
>  uint64_t kvmppc_xive_esb_rw(XiveSource *xsrc, int srcno, uint32_t offset,
>                              uint64_t data, bool write);
> -void kvmppc_xive_set_queue_config(SpaprXive *xive, uint8_t end_blk,
> +int kvmppc_xive_set_queue_config(SpaprXive *xive, uint8_t end_blk,
>                                   uint32_t end_idx, XiveEND *end,
>                                   Error **errp);
> -void kvmppc_xive_get_queue_config(SpaprXive *xive, uint8_t end_blk,
> +int kvmppc_xive_get_queue_config(SpaprXive *xive, uint8_t end_blk,
>                                   uint32_t end_idx, XiveEND *end,
>                                   Error **errp);
>  void kvmppc_xive_synchronize_state(SpaprXive *xive, Error **errp);
> 
> 

-- 
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