On Wed, 9 Sep 2020 21:59:25 +0300
Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> wrote:
> Don't use error propagation in qcow2_get_specific_info(). For this
> refactor qcow2_get_bitmap_info_list, its current interface rather
... interface is rather
> weird.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> ---
Reviewed-by: Greg Kurz <gr...@kaod.org>
> block/qcow2.h | 4 ++--
> block/qcow2-bitmap.c | 27 +++++++++++++--------------
> block/qcow2.c | 10 +++-------
> 3 files changed, 18 insertions(+), 23 deletions(-)
>
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 065ec3df0b..ac6a2d3e2a 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -967,8 +967,8 @@ int qcow2_check_bitmaps_refcounts(BlockDriverState *bs,
> BdrvCheckResult *res,
> void **refcount_table,
> int64_t *refcount_table_size);
> bool qcow2_load_dirty_bitmaps(BlockDriverState *bs, Error **errp);
> -Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *bs,
> - Error **errp);
> +bool qcow2_get_bitmap_info_list(BlockDriverState *bs,
> + Qcow2BitmapInfoList **info_list, Error
> **errp);
> int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp);
> int qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp);
> void qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs,
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 8c34b2aef7..9b14c0791f 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -1090,30 +1090,29 @@ static Qcow2BitmapInfoFlagsList
> *get_bitmap_info_flags(uint32_t flags)
> /*
> * qcow2_get_bitmap_info_list()
> * Returns a list of QCOW2 bitmap details.
> - * In case of no bitmaps, the function returns NULL and
> - * the @errp parameter is not set.
> - * When bitmap information can not be obtained, the function returns
> - * NULL and the @errp parameter is set.
> + * On success return true with bm_list set (probably to NULL, if no bitmaps),
> + * on failure return false with errp set.
> */
> -Qcow2BitmapInfoList *qcow2_get_bitmap_info_list(BlockDriverState *bs,
> - Error **errp)
> +bool qcow2_get_bitmap_info_list(BlockDriverState *bs,
> + Qcow2BitmapInfoList **info_list, Error
> **errp)
> {
> BDRVQcow2State *s = bs->opaque;
> Qcow2BitmapList *bm_list;
> Qcow2Bitmap *bm;
> - Qcow2BitmapInfoList *list = NULL;
> - Qcow2BitmapInfoList **plist = &list;
>
> if (s->nb_bitmaps == 0) {
> - return NULL;
> + *info_list = NULL;
> + return true;
> }
>
> bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
> s->bitmap_directory_size, errp);
> - if (bm_list == NULL) {
> - return NULL;
> + if (!bm_list) {
> + return false;
> }
>
> + *info_list = NULL;
> +
> QSIMPLEQ_FOREACH(bm, bm_list, entry) {
> Qcow2BitmapInfo *info = g_new0(Qcow2BitmapInfo, 1);
> Qcow2BitmapInfoList *obj = g_new0(Qcow2BitmapInfoList, 1);
> @@ -1121,13 +1120,13 @@ Qcow2BitmapInfoList
> *qcow2_get_bitmap_info_list(BlockDriverState *bs,
> info->name = g_strdup(bm->name);
> info->flags = get_bitmap_info_flags(bm->flags & ~BME_RESERVED_FLAGS);
> obj->value = info;
> - *plist = obj;
> - plist = &obj->next;
> + *info_list = obj;
> + info_list = &obj->next;
> }
>
> bitmap_list_free(bm_list);
>
> - return list;
> + return true;
> }
>
> int qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp)
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 10175fa399..eb7c82120c 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -5056,12 +5056,10 @@ static ImageInfoSpecific
> *qcow2_get_specific_info(BlockDriverState *bs,
> BDRVQcow2State *s = bs->opaque;
> ImageInfoSpecific *spec_info;
> QCryptoBlockInfo *encrypt_info = NULL;
> - Error *local_err = NULL;
>
> if (s->crypto != NULL) {
> - encrypt_info = qcrypto_block_get_info(s->crypto, &local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> + encrypt_info = qcrypto_block_get_info(s->crypto, errp);
> + if (!encrypt_info) {
> return NULL;
> }
> }
> @@ -5078,9 +5076,7 @@ static ImageInfoSpecific
> *qcow2_get_specific_info(BlockDriverState *bs,
> };
> } else if (s->qcow_version == 3) {
> Qcow2BitmapInfoList *bitmaps;
> - bitmaps = qcow2_get_bitmap_info_list(bs, &local_err);
> - if (local_err) {
> - error_propagate(errp, local_err);
> + if (!qcow2_get_bitmap_info_list(bs, &bitmaps, errp)) {
> qapi_free_ImageInfoSpecific(spec_info);
> qapi_free_QCryptoBlockInfo(encrypt_info);
> return NULL;
