Re: [PATCH v2 12/13] block/qcow2: simplify qcow2_co_invalidate_cache()

[Vladimir Sementsov-Ogievskiy]

18.09.2020 18:51, Alberto Garcia wrote:
> On Fri 18 Sep 2020 05:30:06 PM CEST, Greg Kurz wrote:
> > > qcow2_do_open correctly sets errp on each failure path. So, we can
> > > simplify code in qcow2_co_invalidate_cache() and drop explicit error
> > > propagation. We should use ERRP_GUARD() (accordingly to comment in
> > > include/qapi/error.h) together with error_append() call which we add to
> > > avoid problems with error_fatal.
> >
> > The wording gives the impression that we add error_append() to avoid problems
> > with error_fatal which is certainly not true. Also it isn't _append() but
> > _prepend() :)
> >
> > What about ?
> >
> > "Add ERRP_GUARD() as mandated by the documentation in include/qapi/error.h
> >   to avoid problems with the error_prepend() call if errp is
> >   &error_fatal."

OK for me.

> I had to go to the individual error functions to see what "it doesn't
> work with &error_fatal" actually means.
>
> So in a case like qcow2_do_open() which has:
>
>     error_setg(errp, ...)
>     error_append_hint(errp, ...)
>
> As far as I can see this works just fine without ERRP_GUARD() and with
> error_fatal, the difference is that if we don't use the guard then the
> process exists during error_setg(), and if we use the guard it exists
> during the implicit error_propagate() call triggered by its destruction
> at the end of the function. In this latter case the printed error
> message would include the hint.

Yes the only problem is that without ERRP_GUARD we lose the hint in case of 
error_fatal.

--
Best regards,
Vladimir