On Thu, 17 Sep 2020 22:55:18 +0300
Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> 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."
With that fixed,
Reviewed-by: Greg Kurz <gr...@kaod.org>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> ---
> block/qcow2.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 2b6ec4b757..cd5f48d3fb 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2702,11 +2702,11 @@ static void qcow2_close(BlockDriverState *bs)
> static void coroutine_fn qcow2_co_invalidate_cache(BlockDriverState *bs,
> Error **errp)
> {
> + ERRP_GUARD();
> BDRVQcow2State *s = bs->opaque;
> int flags = s->flags;
> QCryptoBlock *crypto = NULL;
> QDict *options;
> - Error *local_err = NULL;
> int ret;
>
> /*
> @@ -2724,16 +2724,11 @@ static void coroutine_fn
> qcow2_co_invalidate_cache(BlockDriverState *bs,
>
> flags &= ~BDRV_O_INACTIVE;
> qemu_co_mutex_lock(&s->lock);
> - ret = qcow2_do_open(bs, options, flags, &local_err);
> + ret = qcow2_do_open(bs, options, flags, errp);
> qemu_co_mutex_unlock(&s->lock);
> qobject_unref(options);
> - if (local_err) {
> - error_propagate_prepend(errp, local_err,
> - "Could not reopen qcow2 layer: ");
> - bs->drv = NULL;
> - return;
> - } else if (ret < 0) {
> - error_setg_errno(errp, -ret, "Could not reopen qcow2 layer");
> + if (ret < 0) {
> + error_prepend(errp, "Could not reopen qcow2 layer: ");
> bs->drv = NULL;
> return;
> }
