On Fri, 18 Sep 2020 19:01:34 +0300 Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> wrote:
> 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. > Yeah, so rather: "Add ERRP_GUARD() as mandated by the documentation in include/qapi/error.h so that error_prepend() is actually called even if errp is &error_fatal." Cheers, -- Greg
