[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 12/13] block/qcow2: simplify qcow2_co_invalidate_cache()
From: |
Greg Kurz |
Subject: |
Re: [PATCH v2 12/13] block/qcow2: simplify qcow2_co_invalidate_cache() |
Date: |
Fri, 18 Sep 2020 18:10:00 +0200 |
On Fri, 18 Sep 2020 19:01:34 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@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
- Re: [PATCH v2 08/13] block/qcow2: qcow2_get_specific_info(): drop error propagation, (continued)
[PATCH v2 13/13] block/qed: bdrv_qed_do_open: deal with errp, Vladimir Sementsov-Ogievskiy, 2020/09/17
Re: [PATCH v2 00/13] block: deal with errp: part I, no-reply, 2020/09/17