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

[Max Reitz]

On 2013-10-15 04:23, Fam Zheng wrote:
> There is errp passed in, so no need for local_err and error_propagate.
> Also drop the backing_filename which is set but unused since 34b5d2c.

I approve of dropping the backing_filename code, but I don't know if I 
like removing the error_propagate.

I personally like explicit error_propagate calls in every function 
receiving an error code from an underlying function and propagating that 
as its own. There are two reasons for this:

The first reason is pragmatic: There are lots of places in qemu where 
this is done exactly that way. In fact, I'm responsible for some myself. 
"Fixing" all of them is basically impossible and also unreasonable, 
since the worst error_propagate can do is blow up the code size. 
However, this is not the reason I'd object a patch doing something 
different (here: dropping the unused backing_filename code) and dropping 
"redundant" error_propagate calls along the way.

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.

Max