[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 13/14] block/qcow2: qcow2_do_open: deal with errp
From: |
Alberto Garcia |
Subject: |
Re: [PATCH 13/14] block/qcow2: qcow2_do_open: deal with errp |
Date: |
Thu, 17 Sep 2020 18:23:01 +0200 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Wed 09 Sep 2020 08:59:29 PM CEST, Vladimir Sementsov-Ogievskiy
<vsementsov@virtuozzo.com> wrote:
> 1. Drop extra error propagation
>
> 2. Set errp always on failure. Generic bdrv_open_driver supports driver
> functions which can return negative value and forget to set errp.
> That's a strange thing.. Let's improve qcow2_do_open to not behave this
> way. This allows to simplify code in qcow2_co_invalidate_cache().
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
> block/qcow2.c | 16 +++++++---------
> 1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 31dd28d19e..cc4e7dd461 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1292,6 +1292,7 @@ static int validate_compression_type(BDRVQcow2State *s,
> Error **errp)
> static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
> int flags, Error **errp)
> {
> + ERRP_GUARD();
Why is this necessary?
> BDRVQcow2State *s = bs->opaque;
> unsigned int len, i;
> int ret = 0;
> @@ -1426,6 +1427,8 @@ static int coroutine_fn qcow2_do_open(BlockDriverState
> *bs, QDict *options,
> report_unsupported_feature(errp, feature_table,
> s->incompatible_features &
> ~QCOW2_INCOMPAT_MASK);
> + error_setg(errp,
> + "qcow2 header contains unknown
> incompatible_feature bits");
I think that this is a mistake because the previous call to
report_unsupported_feature() already calls error_setg();
> @@ -2709,11 +2712,11 @@ static void qcow2_close(BlockDriverState *bs)
> static void coroutine_fn qcow2_co_invalidate_cache(BlockDriverState *bs,
> Error **errp)
> {
> + ERRP_GUARD();
Again, why is this necessary?
Berto
- [PATCH 08/14] blockjob: return status from block_job_set_speed(), (continued)
- [PATCH 08/14] blockjob: return status from block_job_set_speed(), Vladimir Sementsov-Ogievskiy, 2020/09/09
- [PATCH 12/14] block/qcow2: read_cache_sizes: return status value, Vladimir Sementsov-Ogievskiy, 2020/09/09
- [PATCH 09/14] block/qcow2: qcow2_get_specific_info(): drop error propagation, Vladimir Sementsov-Ogievskiy, 2020/09/09
- [PATCH 13/14] block/qcow2: qcow2_do_open: deal with errp, Vladimir Sementsov-Ogievskiy, 2020/09/09
- Re: [PATCH 13/14] block/qcow2: qcow2_do_open: deal with errp,
Alberto Garcia <=
- [PATCH 14/14] block/qed: bdrv_qed_do_open: deal with errp, Vladimir Sementsov-Ogievskiy, 2020/09/09