[v3] spapr: Improve error reporting in spapr_caps.c

[PATCH v3 1/4] spapr: Simplify some warning printing paths in spapr_caps.c

Posted by Greg Kurz 5 years, 8 months ago

We obviously only want to print a warning in these cases, but this is done
in a rather convoluted manner. Just use warn_report() instead.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_caps.c |   28 ++++++----------------------
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index efdc0dbbcfc0..0c2bc8e06e44 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -248,23 +248,18 @@ SpaprCapPossible cap_cfpc_possible = {
 static void cap_safe_cache_apply(SpaprMachineState *spapr, uint8_t val,
                                  Error **errp)
 {
-    Error *local_err = NULL;
     uint8_t kvm_val =  kvmppc_get_cap_safe_cache();
 
     if (tcg_enabled() && val) {
         /* TCG only supports broken, allow other values and print a warning */
-        error_setg(&local_err,
-                   "TCG doesn't support requested feature, cap-cfpc=%s",
-                   cap_cfpc_possible.vals[val]);
+        warn_report("TCG doesn't support requested feature, cap-cfpc=%s",
+                    cap_cfpc_possible.vals[val]);
     } else if (kvm_enabled() && (val > kvm_val)) {
         error_setg(errp,
                    "Requested safe cache capability level not supported by kvm,"
                    " try appending -machine cap-cfpc=%s",
                    cap_cfpc_possible.vals[kvm_val]);
     }
-
-    if (local_err != NULL)
-        warn_report_err(local_err);
 }
 
 SpaprCapPossible cap_sbbc_possible = {
@@ -277,23 +272,18 @@ SpaprCapPossible cap_sbbc_possible = {
 static void cap_safe_bounds_check_apply(SpaprMachineState *spapr, uint8_t val,
                                         Error **errp)
 {
-    Error *local_err = NULL;
     uint8_t kvm_val =  kvmppc_get_cap_safe_bounds_check();
 
     if (tcg_enabled() && val) {
         /* TCG only supports broken, allow other values and print a warning */
-        error_setg(&local_err,
-                   "TCG doesn't support requested feature, cap-sbbc=%s",
-                   cap_sbbc_possible.vals[val]);
+        warn_report("TCG doesn't support requested feature, cap-sbbc=%s",
+                    cap_sbbc_possible.vals[val]);
     } else if (kvm_enabled() && (val > kvm_val)) {
         error_setg(errp,
 "Requested safe bounds check capability level not supported by kvm,"
                    " try appending -machine cap-sbbc=%s",
                    cap_sbbc_possible.vals[kvm_val]);
     }
-
-    if (local_err != NULL)
-        warn_report_err(local_err);
 }
 
 SpaprCapPossible cap_ibs_possible = {
@@ -309,24 +299,18 @@ SpaprCapPossible cap_ibs_possible = {
 static void cap_safe_indirect_branch_apply(SpaprMachineState *spapr,
                                            uint8_t val, Error **errp)
 {
-    Error *local_err = NULL;
     uint8_t kvm_val = kvmppc_get_cap_safe_indirect_branch();
 
     if (tcg_enabled() && val) {
         /* TCG only supports broken, allow other values and print a warning */
-        error_setg(&local_err,
-                   "TCG doesn't support requested feature, cap-ibs=%s",
-                   cap_ibs_possible.vals[val]);
+        warn_report("TCG doesn't support requested feature, cap-ibs=%s",
+                    cap_ibs_possible.vals[val]);
     } else if (kvm_enabled() && (val > kvm_val)) {
         error_setg(errp,
 "Requested safe indirect branch capability level not supported by kvm,"
                    " try appending -machine cap-ibs=%s",
                    cap_ibs_possible.vals[kvm_val]);
     }
-
-    if (local_err != NULL) {
-        warn_report_err(local_err);
-    }
 }
 
 #define VALUE_DESC_TRISTATE     " (broken, workaround, fixed)"

Re: [PATCH v3 1/4] spapr: Simplify some warning printing paths in spapr_caps.c

Posted by Vladimir Sementsov-Ogievskiy 5 years, 8 months ago

11.06.2020 16:40, Greg Kurz wrote:
> We obviously only want to print a warning in these cases, but this is done
> in a rather convoluted manner. Just use warn_report() instead.
> 
> Signed-off-by: Greg Kurz<groug@kaod.org>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

-- 
Best regards,
Vladimir

Re: [PATCH v3 1/4] spapr: Simplify some warning printing paths in spapr_caps.c

Posted by Laurent Vivier 5 years, 8 months ago

On 11/06/2020 15:40, Greg Kurz wrote:
> We obviously only want to print a warning in these cases, but this is done
> in a rather convoluted manner. Just use warn_report() instead.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/ppc/spapr_caps.c |   28 ++++++----------------------
>  1 file changed, 6 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index efdc0dbbcfc0..0c2bc8e06e44 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -248,23 +248,18 @@ SpaprCapPossible cap_cfpc_possible = {
>  static void cap_safe_cache_apply(SpaprMachineState *spapr, uint8_t val,
>                                   Error **errp)
>  {
> -    Error *local_err = NULL;
>      uint8_t kvm_val =  kvmppc_get_cap_safe_cache();
>  
>      if (tcg_enabled() && val) {
>          /* TCG only supports broken, allow other values and print a warning */
> -        error_setg(&local_err,
> -                   "TCG doesn't support requested feature, cap-cfpc=%s",
> -                   cap_cfpc_possible.vals[val]);
> +        warn_report("TCG doesn't support requested feature, cap-cfpc=%s",
> +                    cap_cfpc_possible.vals[val]);
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          error_setg(errp,
>                     "Requested safe cache capability level not supported by kvm,"
>                     " try appending -machine cap-cfpc=%s",
>                     cap_cfpc_possible.vals[kvm_val]);
>      }
> -
> -    if (local_err != NULL)
> -        warn_report_err(local_err);
>  }
>  
>  SpaprCapPossible cap_sbbc_possible = {
> @@ -277,23 +272,18 @@ SpaprCapPossible cap_sbbc_possible = {
>  static void cap_safe_bounds_check_apply(SpaprMachineState *spapr, uint8_t val,
>                                          Error **errp)
>  {
> -    Error *local_err = NULL;
>      uint8_t kvm_val =  kvmppc_get_cap_safe_bounds_check();
>  
>      if (tcg_enabled() && val) {
>          /* TCG only supports broken, allow other values and print a warning */
> -        error_setg(&local_err,
> -                   "TCG doesn't support requested feature, cap-sbbc=%s",
> -                   cap_sbbc_possible.vals[val]);
> +        warn_report("TCG doesn't support requested feature, cap-sbbc=%s",
> +                    cap_sbbc_possible.vals[val]);
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          error_setg(errp,
>  "Requested safe bounds check capability level not supported by kvm,"
>                     " try appending -machine cap-sbbc=%s",
>                     cap_sbbc_possible.vals[kvm_val]);
>      }
> -
> -    if (local_err != NULL)
> -        warn_report_err(local_err);
>  }
>  
>  SpaprCapPossible cap_ibs_possible = {
> @@ -309,24 +299,18 @@ SpaprCapPossible cap_ibs_possible = {
>  static void cap_safe_indirect_branch_apply(SpaprMachineState *spapr,
>                                             uint8_t val, Error **errp)
>  {
> -    Error *local_err = NULL;
>      uint8_t kvm_val = kvmppc_get_cap_safe_indirect_branch();
>  
>      if (tcg_enabled() && val) {
>          /* TCG only supports broken, allow other values and print a warning */
> -        error_setg(&local_err,
> -                   "TCG doesn't support requested feature, cap-ibs=%s",
> -                   cap_ibs_possible.vals[val]);
> +        warn_report("TCG doesn't support requested feature, cap-ibs=%s",
> +                    cap_ibs_possible.vals[val]);
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          error_setg(errp,
>  "Requested safe indirect branch capability level not supported by kvm,"
>                     " try appending -machine cap-ibs=%s",
>                     cap_ibs_possible.vals[kvm_val]);
>      }
> -
> -    if (local_err != NULL) {
> -        warn_report_err(local_err);
> -    }
>  }
>  
>  #define VALUE_DESC_TRISTATE     " (broken, workaround, fixed)"
> 
> 

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Re: [PATCH v3 1/4] spapr: Simplify some warning printing paths in spapr_caps.c

Posted by David Gibson 5 years, 8 months ago

On Thu, Jun 11, 2020 at 03:40:11PM +0200, Greg Kurz wrote:
> We obviously only want to print a warning in these cases, but this is done
> in a rather convoluted manner. Just use warn_report() instead.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-5.1, thanks.

> ---
>  hw/ppc/spapr_caps.c |   28 ++++++----------------------
>  1 file changed, 6 insertions(+), 22 deletions(-)
> 
> diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
> index efdc0dbbcfc0..0c2bc8e06e44 100644
> --- a/hw/ppc/spapr_caps.c
> +++ b/hw/ppc/spapr_caps.c
> @@ -248,23 +248,18 @@ SpaprCapPossible cap_cfpc_possible = {
>  static void cap_safe_cache_apply(SpaprMachineState *spapr, uint8_t val,
>                                   Error **errp)
>  {
> -    Error *local_err = NULL;
>      uint8_t kvm_val =  kvmppc_get_cap_safe_cache();
>  
>      if (tcg_enabled() && val) {
>          /* TCG only supports broken, allow other values and print a warning */
> -        error_setg(&local_err,
> -                   "TCG doesn't support requested feature, cap-cfpc=%s",
> -                   cap_cfpc_possible.vals[val]);
> +        warn_report("TCG doesn't support requested feature, cap-cfpc=%s",
> +                    cap_cfpc_possible.vals[val]);
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          error_setg(errp,
>                     "Requested safe cache capability level not supported by kvm,"
>                     " try appending -machine cap-cfpc=%s",
>                     cap_cfpc_possible.vals[kvm_val]);
>      }
> -
> -    if (local_err != NULL)
> -        warn_report_err(local_err);
>  }
>  
>  SpaprCapPossible cap_sbbc_possible = {
> @@ -277,23 +272,18 @@ SpaprCapPossible cap_sbbc_possible = {
>  static void cap_safe_bounds_check_apply(SpaprMachineState *spapr, uint8_t val,
>                                          Error **errp)
>  {
> -    Error *local_err = NULL;
>      uint8_t kvm_val =  kvmppc_get_cap_safe_bounds_check();
>  
>      if (tcg_enabled() && val) {
>          /* TCG only supports broken, allow other values and print a warning */
> -        error_setg(&local_err,
> -                   "TCG doesn't support requested feature, cap-sbbc=%s",
> -                   cap_sbbc_possible.vals[val]);
> +        warn_report("TCG doesn't support requested feature, cap-sbbc=%s",
> +                    cap_sbbc_possible.vals[val]);
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          error_setg(errp,
>  "Requested safe bounds check capability level not supported by kvm,"
>                     " try appending -machine cap-sbbc=%s",
>                     cap_sbbc_possible.vals[kvm_val]);
>      }
> -
> -    if (local_err != NULL)
> -        warn_report_err(local_err);
>  }
>  
>  SpaprCapPossible cap_ibs_possible = {
> @@ -309,24 +299,18 @@ SpaprCapPossible cap_ibs_possible = {
>  static void cap_safe_indirect_branch_apply(SpaprMachineState *spapr,
>                                             uint8_t val, Error **errp)
>  {
> -    Error *local_err = NULL;
>      uint8_t kvm_val = kvmppc_get_cap_safe_indirect_branch();
>  
>      if (tcg_enabled() && val) {
>          /* TCG only supports broken, allow other values and print a warning */
> -        error_setg(&local_err,
> -                   "TCG doesn't support requested feature, cap-ibs=%s",
> -                   cap_ibs_possible.vals[val]);
> +        warn_report("TCG doesn't support requested feature, cap-ibs=%s",
> +                    cap_ibs_possible.vals[val]);
>      } else if (kvm_enabled() && (val > kvm_val)) {
>          error_setg(errp,
>  "Requested safe indirect branch capability level not supported by kvm,"
>                     " try appending -machine cap-ibs=%s",
>                     cap_ibs_possible.vals[kvm_val]);
>      }
> -
> -    if (local_err != NULL) {
> -        warn_report_err(local_err);
> -    }
>  }
>  
>  #define VALUE_DESC_TRISTATE     " (broken, workaround, fixed)"
> 
> 

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

[PATCH v3 1/4] spapr: Simplify some warning printing paths in spapr_caps.c
[PATCH v3 2/4] error: auto propagated local_err
[PATCH v3 3/4] spapr: Use error_append_hint() in spapr_caps.c
[PATCH v3 4/4] spapr: Forbid nested KVM-HV in pre-power9 compat mode