Re: [PATCH v3 04/25] error: auto propagated local_err

[Kevin Wolf]

Am 01.10.2019 um 10:39 hat Vladimir Sementsov-Ogievskiy geschrieben:
> 30.09.2019 19:39, Kevin Wolf wrote:
> > Am 30.09.2019 um 18:26 hat Vladimir Sementsov-Ogievskiy geschrieben:
> >> 30.09.2019 19:00, Kevin Wolf wrote:
> >>> Am 30.09.2019 um 17:19 hat Vladimir Sementsov-Ogievskiy geschrieben:
> >>>> 30.09.2019 18:12, Kevin Wolf wrote:
> >>>>> Am 24.09.2019 um 22:08 hat Vladimir Sementsov-Ogievskiy geschrieben:
> >>>>>> Here is introduced ERRP_FUNCTION_BEGIN macro, to be used at start of
> >>>>>> functions with errp parameter.
> >>>>>
> >>>>> A bit of bike shedding, but FOO_BEGIN suggests to me that a FOO_END will
> >>>>> follow. Can we find a different name, especially now that we won't use
> >>>>> this macro in every function that uses an errp, so even the "errp
> >>>>> function" part isn't really correct any more?
> >>>>>
> >>>>> How about ERRP_AUTO_PROPAGATE?
> >>>>
> >>>> I have an idea that with this macro we can (optionally) get the whole 
> >>>> call stack
> >>>> of the error and print it to log, so it's good to give it more generic 
> >>>> name, not
> >>>> limited to propagation..
> >>>
> >>> Hm, what's the context for this feature?
> >>>
> >>> The obvious one where you want to have a stack trace is &error_abort,
> >>> but that one crashes, so you get it automatically. If it's just a normal
> >>> error (like a QAPI option contains an invalid value and some function
> >>> down the call chain checks it), why would anyone want to know what the
> >>> call chain in the QEMU code was?
> >>>
> >>
> >> When I have bug from testers, call stack would be a lot more descriptive, 
> >> than just
> >> an error message.
> >>
> >> We may add trace point which will print this information, so with disabled 
> >> trace point
> >> - no extra output.
> > 
> > But wouldn't it make much more sense then to optionally add this
> > functionality to any trace point? I really don't see how this is related
> > specifically to user-visible error messages.
> 
> Interesting idea
> 
> > 
> > However, even if we decide that we want to have this in Error objects,
> > wouldn't it make much more sense to use the real C stack trace and save
> > it from the innermost error_set() using backtrace() or compiler
> > built-ins rather than relying on an error_propagate() chain?
> > 
> Hmm, I thought about this.. And in concatenation with the fact that
> we'll have macro not everywhere, backtrace may be better..
> 
> On the other hand, backtrace will not show coroutine entries..

Hm, good point. I wonder if we can easily get a stack trace not starting
at the current point, but from a jmp_buf. Then we could just switch to
the coroutine caller whenever we reach coroutine_trampoline().

But glibc doesn't seem to support this case easily, so that might mean
rewriting all of the stack unwinding inside QEMU... Maybe not then.

> OK, anyway, if we will track some additional information in
> trace-events or in macros or in error_* API functions, it's not bad to
> track some additional information in macro named ERRP_AUTO_PROPAGATE.

Yes, I think tracking the information where we use ERRP_AUTO_PROPAGATE
anyway is okay. I just wouldn't add the macro everywhere just for the
sake of the additional information.

Kevin