Re: [PATCH v4 00/31] error: auto propagated local_err

[Greg Kurz <groug@kaod.org>]

On Tue, 8 Oct 2019 08:41:08 +0000
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:

> 08.10.2019 10:30, Markus Armbruster wrote:
> > Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
> > 
> >> Hi all!
> >>
> >> Here is a proposal of auto propagation for local_err, to not call
> >> error_propagate on every exit point, when we deal with local_err.
> >>
> >> There are also two issues with errp:
> >>
> >> 1. error_fatal & error_append_hint/error_prepend: user can't see this
> >> additional info, because exit() happens in error_setg earlier than info
> >> is added. [Reported by Greg Kurz]
> > 
> > How is this series related to Greg's "[PATCH 00/17] Fix usage of
> > error_append_hint()"?  Do we need both?
> 
> These series is a substitution for Greg's. Still, there are problems with
> automation, which Greg pointed in 21/31, and I don't know what to do next.
> 
> May be, just continue to review patches and fix them by hand. May be try to
> improve automation...
> 

The feeling I have after working on my series is that the lines that deal
with errors are mixed up with the functional code in a variety of ways.
That makes it very difficult if not impossible to come with code patterns
suitable for a 100% automated solution IMHO.

My questioning is more around the semantics of error_fatal actually. What
does passing &error_fatal gives us over passing &local_err and calling
error_report_err()+exit(), apart from breaking error_append_hint() and
error_prepend() ?

> > 
> >> 2. error_abort & error_propagate: when we wrap
> >> error_abort by local_err+error_propagate, resulting coredump will
> >> refer to error_propagate and not to the place where error happened.
> >> (the macro itself don't fix the issue, but it allows to [3.] drop all
> >> local_err+error_propagate pattern, which will definitely fix the issue)
> >> [Reported by Kevin Wolf]
> >>
> >> Still, applying new macro to all errp-functions is a huge task, which is
> >> impossible to solve in one series.
> >>
> >> So, here is a minimum: solve only [1.], by adding new macro to all
> >> errp-functions which wants to call error_append_hint.
> >>
> >> v4;
> >> 02: - check errp to be not NULL
> >>      - drop Eric's r-b
> >> 03: add Eric's r-b
> >> 04: - rename macro to ERRP_AUTO_PROPAGATE [Kevin]
> >>      - improve comment and commit msg, mention
> >>        error_prepend
> >> 05: - handle error_prepend too
> >>      - use new macro name
> >>      - drop empty line at the end
> >>
> >> commit message for auto-generated commits updated,
> >> commits regenerated.
> >>
> >> I'll use cc-cmd to cc appropriate recipients per patch, still
> >> cover-letter together with 04-06 patches should be interesting for
> >> all:
> > [...]
> > 
> > Big series; let me guess its structure.
> > 
> >> Vladimir Sementsov-Ogievskiy (31):
> >>    errp: rename errp to errp_in where it is IN-argument
> >>    hw/core/loader-fit: fix freeing errp in fit_load_fdt
> >>    net/net: fix local variable shadowing in net_client_init
> > 
> > Preparations.
> > 
> >>    error: auto propagated local_err
> > 
> > The new idea.
> > 
> >>    scripts: add script to fix error_append_hint/error_prepend usage
> >>    python: add commit-per-subsystem.py
> > 
> > Scripts to help you apply it.
> > 
> >>    s390: Fix error_append_hint/error_prepend usage
> >>    ARM TCG CPUs: Fix error_append_hint/error_prepend usage
> >>    PowerPC TCG CPUs: Fix error_append_hint/error_prepend usage
> >>    arm: Fix error_append_hint/error_prepend usage
> >>    SmartFusion2: Fix error_append_hint/error_prepend usage
> >>    ASPEED BMCs: Fix error_append_hint/error_prepend usage
> >>    Boston: Fix error_append_hint/error_prepend usage
> >>    PowerNV (Non-Virtualized): Fix error_append_hint/error_prepend usage
> >>    PCI: Fix error_append_hint/error_prepend usage
> >>    SCSI: Fix error_append_hint/error_prepend usage
> >>    USB: Fix error_append_hint/error_prepend usage
> >>    VFIO: Fix error_append_hint/error_prepend usage
> >>    vhost: Fix error_append_hint/error_prepend usage
> >>    virtio: Fix error_append_hint/error_prepend usage
> >>    virtio-9p: Fix error_append_hint/error_prepend usage
> >>    XIVE: Fix error_append_hint/error_prepend usage
> >>    block: Fix error_append_hint/error_prepend usage
> >>    chardev: Fix error_append_hint/error_prepend usage
> >>    cmdline: Fix error_append_hint/error_prepend usage
> >>    QOM: Fix error_append_hint/error_prepend usage
> >>    Migration: Fix error_append_hint/error_prepend usage
> >>    Sockets: Fix error_append_hint/error_prepend usage
> >>    nbd: Fix error_append_hint/error_prepend usage
> >>    PVRDMA: Fix error_append_hint/error_prepend usage
> >>    ivshmem: Fix error_append_hint/error_prepend usage
> > 
> > Applying it.
> > 
> > Correct?
> > 
> 
> Yes
> 
>