[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 08/13] block/qcow2: qcow2_get_specific_info(): drop error
From: |
Alberto Garcia |
Subject: |
Re: [PATCH v2 08/13] block/qcow2: qcow2_get_specific_info(): drop error propagation |
Date: |
Fri, 18 Sep 2020 17:04:20 +0200 |
User-agent: |
Notmuch/0.18.2 (http://notmuchmail.org) Emacs/24.4.1 (i586-pc-linux-gnu) |
On Thu 17 Sep 2020 09:55:14 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> Don't use error propagation in qcow2_get_specific_info(). For this
> refactor qcow2_get_bitmap_info_list, its current interface is rather
> weird.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> - * 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.
I still find the 'probably' thing odd :-) I think the API documentation
should describe what the function does and under which conditions, not
the probability of the outcomes.
> QSIMPLEQ_FOREACH(bm, bm_list, entry) {
> Qcow2BitmapInfo *info = g_new0(Qcow2BitmapInfo, 1);
> Qcow2BitmapInfoList *obj = g_new0(Qcow2BitmapInfoList, 1);
> @@ -1124,13 +1123,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;
> }
You were right with this, I got confused by the fact that you are
modifying the pointer provided by the user.
Reviewed-by: Alberto Garcia <berto@igalia.com>
Berto
- [PATCH v2 03/13] block: check return value of bdrv_open_child and drop error propagation, (continued)
- [PATCH v2 03/13] block: check return value of bdrv_open_child and drop error propagation, Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 04/13] blockdev: fix drive_backup_prepare() missed error, Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 05/13] block: drop extra error propagation for bdrv_set_backing_hd, Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 02/13] block: use return status of bdrv_append(), Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 10/13] block/qcow2-bitmap: return status from qcow2_store_persistent_dirty_bitmaps, Vladimir Sementsov-Ogievskiy, 2020/09/17
- [PATCH v2 06/13] block/mirror: drop extra error propagation in commit_active_start(), Vladimir Sementsov-Ogievskiy, 2020/09/17
- [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
- Re: [PATCH v2 08/13] block/qcow2: qcow2_get_specific_info(): drop error propagation,
Alberto Garcia <=
- [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