[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 12/14] block/qcow2: read_cache_sizes: return status value
From: |
Greg Kurz |
Subject: |
Re: [PATCH 12/14] block/qcow2: read_cache_sizes: return status value |
Date: |
Fri, 11 Sep 2020 11:40:11 +0200 |
On Wed, 9 Sep 2020 21:59:28 +0300
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> wrote:
> It's better to return status together with setting errp. It allows to
> reduce error propagation.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
Reviewed-by: Greg Kurz <groug@kaod.org>
> block/qcow2.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index c2cd9434cc..31dd28d19e 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -869,7 +869,7 @@ static void qcow2_attach_aio_context(BlockDriverState *bs,
> cache_clean_timer_init(bs, new_context);
> }
>
> -static void read_cache_sizes(BlockDriverState *bs, QemuOpts *opts,
> +static bool read_cache_sizes(BlockDriverState *bs, QemuOpts *opts,
> uint64_t *l2_cache_size,
> uint64_t *l2_cache_entry_size,
> uint64_t *refcount_cache_size, Error **errp)
> @@ -907,16 +907,16 @@ static void read_cache_sizes(BlockDriverState *bs,
> QemuOpts *opts,
> error_setg(errp, QCOW2_OPT_CACHE_SIZE ", "
> QCOW2_OPT_L2_CACHE_SIZE
> " and " QCOW2_OPT_REFCOUNT_CACHE_SIZE " may not be
> set "
> "at the same time");
> - return;
> + return false;
> } else if (l2_cache_size_set &&
> (l2_cache_max_setting > combined_cache_size)) {
> error_setg(errp, QCOW2_OPT_L2_CACHE_SIZE " may not exceed "
> QCOW2_OPT_CACHE_SIZE);
> - return;
> + return false;
> } else if (*refcount_cache_size > combined_cache_size) {
> error_setg(errp, QCOW2_OPT_REFCOUNT_CACHE_SIZE " may not exceed "
> QCOW2_OPT_CACHE_SIZE);
> - return;
> + return false;
> }
>
> if (l2_cache_size_set) {
> @@ -955,8 +955,10 @@ static void read_cache_sizes(BlockDriverState *bs,
> QemuOpts *opts,
> error_setg(errp, "L2 cache entry size must be a power of two "
> "between %d and the cluster size (%d)",
> 1 << MIN_CLUSTER_BITS, s->cluster_size);
> - return;
> + return false;
> }
> +
> + return true;
> }
>
> typedef struct Qcow2ReopenState {
> @@ -983,7 +985,6 @@ static int qcow2_update_options_prepare(BlockDriverState
> *bs,
> int i;
> const char *encryptfmt;
> QDict *encryptopts = NULL;
> - Error *local_err = NULL;
> int ret;
>
> qdict_extract_subqdict(options, &encryptopts, "encrypt.");
> @@ -996,10 +997,8 @@ static int qcow2_update_options_prepare(BlockDriverState
> *bs,
> }
>
> /* get L2 table/refcount block cache size from command line options */
> - read_cache_sizes(bs, opts, &l2_cache_size, &l2_cache_entry_size,
> - &refcount_cache_size, &local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> + if (!read_cache_sizes(bs, opts, &l2_cache_size, &l2_cache_entry_size,
> + &refcount_cache_size, errp)) {
> ret = -EINVAL;
> goto fail;
> }
- Re: [PATCH 11/14] block/qcow2-bitmap: return startus from qcow2_store_persistent_dirty_bitmaps, (continued)
Re: [PATCH 11/14] block/qcow2-bitmap: return startus from qcow2_store_persistent_dirty_bitmaps, Alberto Garcia, 2020/09/17
[PATCH 06/14] block/mirror: drop extra error propagation in commit_active_start(), Vladimir Sementsov-Ogievskiy, 2020/09/09
[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
[PATCH 14/14] block/qed: bdrv_qed_do_open: deal with errp, Vladimir Sementsov-Ogievskiy, 2020/09/09