[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:48:52 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.8.1 |
11.06.2020 13:44, Vladimir Sementsov-Ogievskiy wrote:
11.06.2020 13:39, Greg Kurz wrote:
On Thu, 11 Jun 2020 13:21:51 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
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:
Not even that... we have one path (KVM) that directly
uses errp and the other path (TCG) that does:
Error *local_err = NULL;
error_setg(&local_err, ...);
if (local_err) {
warn_report_err(local_err);
}
It really looks like we just want to call warn_report().
yes, I also came this)
--- 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?
Just for record: probably, we'll want at lest something like this (not needed
in this series):
diff --git a/include/qapi/error.h b/include/qapi/error.h
index 30140d9bfe..25b5da0733 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -475,4 +475,10 @@ extern Error *error_abort;
*/
extern Error *error_fatal;
+/*
+ * Special error destination to call warn_report_err.
+ * See error_setg() and error_propagate() for details.
+ */
+extern Error *error_warn_report;
+
#endif
diff --git a/util/error.c b/util/error.c
index b6c89d1412..928eb4cc7a 100644
--- a/util/error.c
+++ b/util/error.c
@@ -27,8 +27,9 @@ struct Error
Error *error_abort;
Error *error_fatal;
+Error *error_warn_report;
-static void error_handle_fatal(Error **errp, Error *err)
+static bool error_handle_special(Error **errp, Error *err)
{
if (errp == &error_abort) {
fprintf(stderr, "Unexpected error in %s() at %s:%d:\n",
@@ -43,6 +44,12 @@ static void error_handle_fatal(Error **errp, Error *err)
error_report_err(err);
exit(1);
}
+ if (errp == &error_warn_report) {
+ warn_report_err(err);
+ return true;
+ }
+
+ return false;
}
static void error_setv(Error **errp,
@@ -70,8 +77,9 @@ static void error_setv(Error **errp,
err->line = line;
err->func = func;
- error_handle_fatal(errp, err);
- *errp = err;
+ if (!error_handle_special(errp, err)) {
+ *errp = err;
+ }
errno = saved_errno;
}
@@ -283,7 +291,9 @@ void error_propagate(Error **dst_errp, Error *local_err)
if (!local_err) {
return;
}
- error_handle_fatal(dst_errp, local_err);
+ if (error_handle_special(dst_errp, local_err)) {
+ return;
+ }
if (dst_errp && !*dst_errp) {
*dst_errp = local_err;
} else {
--
Best regards,
Vladimir
- [PATCH v2 1/3] error: auto propagated local_err, (continued)
- [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, 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, Vladimir Sementsov-Ogievskiy, 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, Laurent Vivier, 2020/06/11
[PATCH v2 3/3] spapr: Forbid nested KVM-HV in pre-power9 compat mode, Greg Kurz, 2020/06/11