[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 25/25] qcow2-bitmap: improve check_
From: |
John Snow |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 25/25] qcow2-bitmap: improve check_constraints_on_bitmap |
Date: |
Tue, 14 Feb 2017 19:35:09 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 |
On 02/14/2017 11:59 AM, Vladimir Sementsov-Ogievskiy wrote:
> Add detailed error messages.
>
yay
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
> block/qcow2-bitmap.c | 63
> ++++++++++++++++++++++++++++++++++------------------
> 1 file changed, 41 insertions(+), 22 deletions(-)
>
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 7ad1f7c..113d48c 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -160,28 +160,49 @@ static int check_table_entry(uint64_t entry, int
> cluster_size)
>
> static int check_constraints_on_bitmap(BlockDriverState *bs,
> const char *name,
> - uint32_t granularity)
> + uint32_t granularity,
> + Error **errp)
> {
> BDRVQcow2State *s = bs->opaque;
> int granularity_bits = ctz32(granularity);
> int64_t len = bdrv_getlength(bs);
> - bool fail;
>
> assert(granularity > 0);
> assert((granularity & (granularity - 1)) == 0);
>
> if (len < 0) {
> + error_setg_errno(errp, -len, "Failed to get size of '%s'",
> + bdrv_get_device_or_node_name(bs));
> return len;
> }
>
> - fail = (granularity_bits > BME_MAX_GRANULARITY_BITS) ||
> - (granularity_bits < BME_MIN_GRANULARITY_BITS) ||
> - (len > (uint64_t)BME_MAX_PHYS_SIZE << granularity_bits) ||
> - (len > (uint64_t)BME_MAX_TABLE_SIZE * s->cluster_size <<
> - granularity_bits) ||
> - (strlen(name) > BME_MAX_NAME_SIZE);
> + if (granularity_bits > BME_MAX_GRANULARITY_BITS) {
> + error_setg(errp, "Granularity exceeds maximum (%u bytes)",
> + 1 << BME_MAX_GRANULARITY_BITS);
> + return -EINVAL;
> + }
> + if (granularity_bits < BME_MIN_GRANULARITY_BITS) {
> + error_setg(errp, "Granularity is under minimum (%u bytes)",
> + 1 << BME_MIN_GRANULARITY_BITS);
> + return -EINVAL;
> + }
>
> - return fail ? -EINVAL : 0;
> + if ((len > (uint64_t)BME_MAX_PHYS_SIZE << granularity_bits) ||
> + (len > (uint64_t)BME_MAX_TABLE_SIZE * s->cluster_size <<
> + granularity_bits))
> + {
> + error_setg(errp, "Too much space will be occupied by the bitmap. "
> + "Use larger granularity");
> + return -EINVAL;
> + }
> +
> + if (strlen(name) > BME_MAX_NAME_SIZE) {
> + error_setg(errp, "Name length exceeds maximum (%u characters)",
> + BME_MAX_NAME_SIZE);
> + return -EINVAL;
> + }
> +
> + return 0;
> }
>
> static void clear_bitmap_table(BlockDriverState *bs, uint64_t *bitmap_table,
> @@ -1142,9 +1163,9 @@ void
> qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, Error **errp)
> continue;
> }
>
> - if (check_constraints_on_bitmap(bs, name, granularity) < 0) {
> - error_setg(errp, "Bitmap '%s' doesn't satisfy the constraints",
> - name);
> + if (check_constraints_on_bitmap(bs, name, granularity, errp) < 0) {
> + error_prepend(errp, "Bitmap '%s' doesn't satisfy the
> constraints: ",
> + name);
> goto fail;
> }
>
> @@ -1230,13 +1251,11 @@ bool
> qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
> Error **errp)
> {
> BDRVQcow2State *s = bs->opaque;
> - const char *reason = NULL;
> bool found;
> Qcow2BitmapList *bm_list;
>
> - if (check_constraints_on_bitmap(bs, name, granularity) != 0) {
> - reason = "it doesn't satisfy the constraints";
> - goto common_errp;
> + if (check_constraints_on_bitmap(bs, name, granularity, errp) != 0) {
> + goto fail;
> }
>
> if (s->nb_bitmaps == 0) {
> @@ -1246,21 +1265,21 @@ bool
> qcow2_can_store_new_dirty_bitmap(BlockDriverState *bs,
> bm_list = bitmap_list_load(bs, s->bitmap_directory_offset,
> s->bitmap_directory_size, errp);
> if (bm_list == NULL) {
> - return false;
> + goto fail;
> }
>
> found = find_bitmap_by_name(bm_list, name);
> bitmap_list_free(bm_list);
> if (found) {
> - reason = "bitmap with the same name is already stored";
> - goto common_errp;
> + error_setg(errp, "Bitmap with the same name is already stored");
> + goto fail;
> }
>
> return true;
>
> -common_errp:
> - error_setg(errp, "Can't make bitmap '%s' persistent in '%s', as %s.",
> - name, bdrv_get_device_or_node_name(bs), reason);
> +fail:
> + error_prepend(errp, "Can't make bitmap '%s' persistent in '%s': ",
> + name, bdrv_get_device_or_node_name(bs));
> return false;
> }
>
>
Sorry I made it less efficient :)
Reviewed-by: John Snow <address@hidden>
- [Qemu-block] [PATCH 05/25] block: fix bdrv_dirty_bitmap_granularity signature, (continued)
- [Qemu-block] [PATCH 05/25] block: fix bdrv_dirty_bitmap_granularity signature, Vladimir Sementsov-Ogievskiy, 2017/02/14
- [Qemu-block] [PATCH 16/25] qmp: add persistent flag to block-dirty-bitmap-add, Vladimir Sementsov-Ogievskiy, 2017/02/14
- [Qemu-block] [PATCH 12/25] block/dirty-bitmap: add bdrv_dirty_bitmap_next(), Vladimir Sementsov-Ogievskiy, 2017/02/14
- [Qemu-block] [PATCH 22/25] block/dirty-bitmap: add bdrv_remove_persistent_dirty_bitmap, Vladimir Sementsov-Ogievskiy, 2017/02/14
- [Qemu-block] [PATCH 18/25] qmp: add x-debug-block-dirty-bitmap-sha256, Vladimir Sementsov-Ogievskiy, 2017/02/14
- [Qemu-block] [PATCH 06/25] block/dirty-bitmap: add deserialize_ones func, Vladimir Sementsov-Ogievskiy, 2017/02/14
- [Qemu-block] [PATCH 25/25] qcow2-bitmap: improve check_constraints_on_bitmap, Vladimir Sementsov-Ogievskiy, 2017/02/14
- Re: [Qemu-block] [Qemu-devel] [PATCH 25/25] qcow2-bitmap: improve check_constraints_on_bitmap,
John Snow <=
- [Qemu-block] [PATCH 24/25] qmp: block-dirty-bitmap-remove: remove persistent, Vladimir Sementsov-Ogievskiy, 2017/02/14
- [Qemu-block] [PATCH 04/25] tests: add hbitmap iter test, Vladimir Sementsov-Ogievskiy, 2017/02/14
- [Qemu-block] [PATCH 02/25] specs/qcow2: do not use wording 'bitmap header', Vladimir Sementsov-Ogievskiy, 2017/02/14
- [Qemu-block] [PATCH 21/25] qcow2-bitmap: refcounts, Vladimir Sementsov-Ogievskiy, 2017/02/14
- [Qemu-block] [PATCH 01/25] specs/qcow2: fix bitmap granularity qemu-specific note, Vladimir Sementsov-Ogievskiy, 2017/02/14
- [Qemu-block] [PATCH 08/25] block: introduce auto-loading bitmaps, Vladimir Sementsov-Ogievskiy, 2017/02/14
- [Qemu-block] [PATCH 23/25] qcow2: add .bdrv_remove_persistent_dirty_bitmap, Vladimir Sementsov-Ogievskiy, 2017/02/14