Re: [Qemu-devel] [PATCH v2] mirror: drop local_err in mirror_complete

[Fam Zheng]

On Thu, 10/17 15:00, Kevin Wolf wrote:
> Am 17.10.2013 um 14:49 hat Stefan Hajnoczi geschrieben:
> > On Wed, Oct 16, 2013 at 08:56:49PM +0200, Max Reitz wrote:
> > > On 2013-10-15 04:23, Fam Zheng wrote:
> > > The reason I object it here is that error_propagate *currently* is a
> > > no-op. But this may change in the future: I have already sent an RFC
> > > which extends error_propagate so it can generate an error backtrace
> > > if enabled through configure. If this (or something similar which
> > > extends error_propagate to do more than basically just *errp =
> > > local_err) is merged to/implemented in qemu later on, I believe we
> > > want to call error_propagate in every function that touches an error
> > > variable in order to generate a backtrace with maximum verbosity.
> > 
> > Did you check if glib's backtrace(3) APIs can be used in error_set()
> > instead of rolling our own backtrace?
> > 
> > Also, what is the purpose of the backtrace?  If the problem is that
> > error messages don't identify unique errors, then we should fix that
> > instead of relying on backtraces.  For example, a function that opens
> > two separate files shouldn't just fail with "File not found" since we
> > don't know which of the two files wasn't found.
> 
> Mostly debugging, I guess. Even if you have unique error messages that
> can only come from a single place, finding that single place by looking
> at all called functions from a given QMP command can take quite a bit of
> time. I can see it useful.
> 
> And we don't even have the unique error messages yet, so we shouldn't
> fall in the trap of rejecting an improvement because it's not perfect.

Stacktrace already provides useful information for debugging, so I'm wondering
if it makes sense to add support (a framework) to catch or dump the stacktrace,
which can be used in error_set(), abort() and tracing framework.

Manually calling error_propagate as above sounds a unnecessary reinvention of
this.

Fam