[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 17:30:06 +0200 |
On Thu, 17 Sep 2020 22:55:18 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@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 <groug@kaod.org>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@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;
> }
- [PATCH v2 07/13] blockjob: return status from block_job_set_speed(), (continued)
- [PATCH v2 07/13] blockjob: return status from block_job_set_speed(), Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 11/13] block/qcow2: read_cache_sizes: return status value, Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 08/13] block/qcow2: qcow2_get_specific_info(): drop error propagation, Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 09/13] block/qcow2-bitmap: improve qcow2_load_dirty_bitmaps() interface, Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 12/13] block/qcow2: simplify qcow2_co_invalidate_cache(), Vladimir Sementsov-Ogievskiy, 2020/09/17
[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