[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c
From: |
Vladimir Sementsov-Ogievskiy |
Subject: |
Re: [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c |
Date: |
Thu, 11 Jun 2020 13:21:51 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.1 |
11.06.2020 13:13, Greg Kurz wrote:
On Thu, 11 Jun 2020 11:50:57 +0200
Laurent Vivier <lvivier@redhat.com> wrote:
On 11/06/2020 11:10, Greg Kurz wrote:
We have a dedicated error API for hints. Use it instead of embedding
the hint in the error message, as recommanded in the "qapi/error.h"
header file.
Since spapr_caps_apply() passes &error_fatal, all functions must
also call the ERRP_AUTO_PROPAGATE() macro for error_append_hint()
to be functional.
While here, add some missing braces around one line statements that
are part of the patch context. Also have cap_fwnmi_apply(), which
already uses error_append_hint() to call ERRP_AUTO_PROPAGATE() as
well.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/ppc/spapr_caps.c | 95 +++++++++++++++++++++++++++++----------------------
1 file changed, 54 insertions(+), 41 deletions(-)
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index efdc0dbbcfc0..2cb7ba8f005a 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
...
@@ -248,6 +249,7 @@ SpaprCapPossible cap_cfpc_possible = {
static void cap_safe_cache_apply(SpaprMachineState *spapr, uint8_t val,
Error **errp)
{
+ ERRP_AUTO_PROPAGATE();
Error *local_err = NULL;
I think you should rename it, something like "local_warn" to not be
confused with the _auto_errp_prop.local_err...
or don't use ERRP_AUTO_PROPAGE(), use the local_err instead and move the
warning inside the braces of the if.
Same comment for cap_safe_bounds_check_apply() and
cap_safe_indirect_branch_apply()
Hmm... local_err isn't useful actually. It looks like we just want
to call warn_report() directly instead of error_setg(&local_err)
and warn_report_err(local_err). I'll post a v3.
something like this I think:
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -250,24 +250,23 @@ static void cap_safe_cache_apply(SpaprMachineState
*spapr, uint8_t val,
Error **errp)
{
ERRP_AUTO_PROPAGATE();
- 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,
+ error_setg(errp,
"TCG doesn't support requested feature, cap-cfpc=%s",
cap_cfpc_possible.vals[val]);
+ if (*errp) {
+ warn_report_err(*errp);
+ *errp = NULL;
+ }
} else if (kvm_enabled() && (val > kvm_val)) {
error_setg(errp,
"Requested safe cache capability level not supported by
KVM");
error_append_hint(errp, "Try appending -machine cap-cfpc=%s\n",
cap_cfpc_possible.vals[kvm_val]);
}
-
- if (local_err != NULL) {
- warn_report_err(local_err);
- }
}
Or, we need to implement warn_report_errp() function, as I proposed in earlier
version of auto-propagation series.
=====
side idea: what if we make Error to be some kind of enum of pointer-to-pointer
and pointer-to-function?
Than, instead of passing pointers to error_abort and error_fatal as special
casing, we'll pass pointers to functions,
which do appropriate handling of error. And we'll be able to pass warn_report
function. Not about this patch set,
but seems interesting, isn't it?
--
Best regards,
Vladimir
- [PATCH v2 0/3] spapr: Improve error reporting in spapr_caps.c, Greg Kurz, 2020/06/11
- [PATCH v2 1/3] error: auto propagated local_err, Greg Kurz, 2020/06/11
- [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c, Greg Kurz, 2020/06/11
- Re: [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c, Vladimir Sementsov-Ogievskiy, 2020/06/11
- Re: [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c, Laurent Vivier, 2020/06/11
- Re: [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c, Greg Kurz, 2020/06/11
- Re: [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c,
Vladimir Sementsov-Ogievskiy <=
- Re: [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c, Vladimir Sementsov-Ogievskiy, 2020/06/11
- Re: [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c, Greg Kurz, 2020/06/11
- Re: [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c, Vladimir Sementsov-Ogievskiy, 2020/06/11
- Re: [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c, Vladimir Sementsov-Ogievskiy, 2020/06/11
- Re: [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c, Vladimir Sementsov-Ogievskiy, 2020/06/11
- Re: [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c, Greg Kurz, 2020/06/11
- Re: [PATCH v2 2/3] spapr: Use error_append_hint() in spapr_caps.c, Laurent Vivier, 2020/06/11
[PATCH v2 3/3] spapr: Forbid nested KVM-HV in pre-power9 compat mode, Greg Kurz, 2020/06/11