diff mbox series

[RFC,v5,030/126] kvm: introduce ERRP_AUTO_PROPAGATE

Message ID 20191011160552.22907-31-vsementsov@virtuozzo.com (mailing list archive)
State New, archived
Headers show
Series error: auto propagated local_err | expand

Commit Message

Vladimir Sementsov-Ogievskiy Oct. 11, 2019, 4:04 p.m. UTC
If we want to add some info to errp (by error_prepend() or
error_append_hint()), we must use the ERRP_AUTO_PROPAGATE macro.
Otherwise, this info will not be added when errp == &fatal_err
(the program will exit prior to the error_append_hint() or
error_prepend() call).  Fix such cases.

If we want to check error after errp-function call, we need to
introduce local_err and than propagate it to errp. Instead, use
ERRP_AUTO_PROPAGATE macro, benefits are:
1. No need of explicit error_propagate call
2. No need of explicit local_err variable: use errp directly
3. ERRP_AUTO_PROPAGATE leaves errp as is if it's not NULL or
   &error_fatel, this means that we don't break error_abort
   (we'll abort on error_set, not on error_propagate)

This commit (together with its neighbors) was generated by

for f in $(git grep -l errp \*.[ch]); do \
    spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
    --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff $f; \
done;

then fix a bit of compilation problems: coccinelle for some reason
leaves several
f() {
    ...
    goto out;
    ...
    out:
}
patterns, with "out:" at function end.

then
./python/commit-per-subsystem.py MAINTAINERS "$(< auto-msg)"

(auto-msg was a file with this commit message)

Still, for backporting it may be more comfortable to use only the first
command and then do one huge commit.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Reported-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 target/ppc/kvm.c          | 8 ++++----
 target/s390x/cpu_models.c | 2 ++
 2 files changed, 6 insertions(+), 4 deletions(-)

Comments

Cornelia Huck Nov. 12, 2019, 1:31 p.m. UTC | #1
On Fri, 11 Oct 2019 19:04:16 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> If we want to add some info to errp (by error_prepend() or
> error_append_hint()), we must use the ERRP_AUTO_PROPAGATE macro.
> Otherwise, this info will not be added when errp == &fatal_err
> (the program will exit prior to the error_append_hint() or
> error_prepend() call).  Fix such cases.
> 
> If we want to check error after errp-function call, we need to
> introduce local_err and than propagate it to errp. Instead, use
> ERRP_AUTO_PROPAGATE macro, benefits are:
> 1. No need of explicit error_propagate call
> 2. No need of explicit local_err variable: use errp directly
> 3. ERRP_AUTO_PROPAGATE leaves errp as is if it's not NULL or
>    &error_fatel, this means that we don't break error_abort
>    (we'll abort on error_set, not on error_propagate)
> 
> This commit (together with its neighbors) was generated by
> 
> for f in $(git grep -l errp \*.[ch]); do \
>     spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
>     --macro-file scripts/cocci-macro-file.h --in-place --no-show-diff $f; \
> done;
> 
> then fix a bit of compilation problems: coccinelle for some reason
> leaves several
> f() {
>     ...
>     goto out;
>     ...
>     out:
> }
> patterns, with "out:" at function end.
> 
> then
> ./python/commit-per-subsystem.py MAINTAINERS "$(< auto-msg)"
> 
> (auto-msg was a file with this commit message)
> 
> Still, for backporting it may be more comfortable to use only the first
> command and then do one huge commit.
> 
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Reported-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  target/ppc/kvm.c          | 8 ++++----
>  target/s390x/cpu_models.c | 2 ++

This grouping seems a bit unintuitive :)

>  2 files changed, 6 insertions(+), 4 deletions(-)
> 

s390x part:
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
diff mbox series

Patch

diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index af6e667bf8..d58a3f5bf5 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -237,6 +237,7 @@  static int kvm_booke206_tlb_init(PowerPCCPU *cpu)
 #if defined(TARGET_PPC64)
 static void kvm_get_smmu_info(struct kvm_ppc_smmu_info *info, Error **errp)
 {
+    ERRP_AUTO_PROPAGATE();
     int ret;
 
     assert(kvm_state != NULL);
@@ -325,18 +326,17 @@  bool kvmppc_hpt_needs_host_contiguous_pages(void)
 
 void kvm_check_mmu(PowerPCCPU *cpu, Error **errp)
 {
+    ERRP_AUTO_PROPAGATE();
     struct kvm_ppc_smmu_info smmu_info;
     int iq, ik, jq, jk;
-    Error *local_err = NULL;
 
     /* For now, we only have anything to check on hash64 MMUs */
     if (!cpu->hash64_opts || !kvm_enabled()) {
         return;
     }
 
-    kvm_get_smmu_info(&smmu_info, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    kvm_get_smmu_info(&smmu_info, errp);
+    if (*errp) {
         return;
     }
 
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 009afc38b9..32f2e5e822 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -840,6 +840,7 @@  static void error_prepend_missing_feat(const char *name, void *opaque)
 static void check_compatibility(const S390CPUModel *max_model,
                                 const S390CPUModel *model, Error **errp)
 {
+    ERRP_AUTO_PROPAGATE();
     S390FeatBitmap missing;
 
     if (model->def->gen > max_model->def->gen) {
@@ -922,6 +923,7 @@  static inline void apply_cpu_model(const S390CPUModel *model, Error **errp)
 
 void s390_realize_cpu_model(CPUState *cs, Error **errp)
 {
+    ERRP_AUTO_PROPAGATE();
     S390CPUClass *xcc = S390_CPU_GET_CLASS(cs);
     S390CPU *cpu = S390_CPU(cs);
     const S390CPUModel *max_model;