[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v16 13/22] qcow2: add persistent dirty bitmaps s
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v16 13/22] qcow2: add persistent dirty bitmaps support |
Date: |
Mon, 27 Feb 2017 15:38:30 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 |
On 25.02.2017 18:07, Vladimir Sementsov-Ogievskiy wrote:
> Store persistent dirty bitmaps in qcow2 image.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
> block.c | 6 +-
> block/qcow2-bitmap.c | 473
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> block/qcow2.c | 9 +
> block/qcow2.h | 1 +
> 4 files changed, 486 insertions(+), 3 deletions(-)
>
> diff --git a/block.c b/block.c
> index a0346c80c6..16cf522219 100644
> --- a/block.c
> +++ b/block.c
> @@ -2322,9 +2322,6 @@ static void bdrv_close(BlockDriverState *bs)
> bdrv_flush(bs);
> bdrv_drain(bs); /* in case flush left pending I/O */
>
> - bdrv_release_named_dirty_bitmaps(bs);
> - assert(QLIST_EMPTY(&bs->dirty_bitmaps));
> -
> if (bs->drv) {
> BdrvChild *child, *next;
>
> @@ -2363,6 +2360,9 @@ static void bdrv_close(BlockDriverState *bs)
> bs->full_open_options = NULL;
> }
>
> + bdrv_release_named_dirty_bitmaps(bs);
> + assert(QLIST_EMPTY(&bs->dirty_bitmaps));
> +
> QLIST_FOREACH_SAFE(ban, &bs->aio_notifiers, list, ban_next) {
> g_free(ban);
> }
Might deserve an own patch, but I don't mind.
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index ba72b7d2ac..e377215d5c 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
[...]
> @@ -127,6 +145,70 @@ static int check_table_entry(uint64_t entry, int
> cluster_size)
> return 0;
> }
>
> +static int check_constraints_on_bitmap(BlockDriverState *bs,
> + const char *name,
> + uint32_t granularity,
> + Error **errp)
> +{
> + BDRVQcow2State *s = bs->opaque;
> + int granularity_bits = ctz32(granularity);
> + int64_t len = bdrv_getlength(bs);
> +
> + 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;
> + }
> +
> + if (granularity_bits > BME_MAX_GRANULARITY_BITS) {
> + error_setg(errp, "Granularity exceeds maximum (%u bytes)",
> + 1 << BME_MAX_GRANULARITY_BITS);
This will overflow because 1 << 31 is not representable in int (and 1 is
an int). The %u saves it by converting it back, but it's still
implementation-defined behavior at most.
I'd prefer a plain 1ull and %ull. That way, this would be save no matter
what value BME_MAX_GRANULARITY_BITS is.
> + return -EINVAL;
> + }
> + if (granularity_bits < BME_MIN_GRANULARITY_BITS) {
> + error_setg(errp, "Granularity is under minimum (%u bytes)",
> + 1 << BME_MIN_GRANULARITY_BITS);
The same applies here, although this does not have the overflow issue.
Rest looks good (to me O:-)).
Max
> + return -EINVAL;
> + }
> +
> + 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;
> +}
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v16 11/22] block: introduce persistent dirty bitmaps, (continued)
- [Qemu-devel] [PATCH v16 17/22] qmp: add autoload parameter to block-dirty-bitmap-add, Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 19/22] iotests: test qcow2 persistent dirty bitmap, Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 02/22] specs/qcow2: do not use wording 'bitmap header', Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 14/22] block: add bdrv_can_store_new_dirty_bitmap, Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 05/22] block: fix bdrv_dirty_bitmap_granularity signature, Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 18/22] qmp: add x-debug-block-dirty-bitmap-sha256, Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 21/22] qcow2: add .bdrv_remove_persistent_dirty_bitmap, Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 01/22] specs/qcow2: fix bitmap granularity qemu-specific note, Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 13/22] qcow2: add persistent dirty bitmaps support, Vladimir Sementsov-Ogievskiy, 2017/02/25
- Re: [Qemu-devel] [PATCH v16 13/22] qcow2: add persistent dirty bitmaps support,
Max Reitz <=
- [Qemu-devel] [PATCH v16 08/22] qcow2: add bitmaps extension, Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 04/22] tests: add hbitmap iter test, Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 12/22] block/dirty-bitmap: add bdrv_dirty_bitmap_next(), Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 03/22] hbitmap: improve dirty iter, Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 15/22] qcow2: add .bdrv_can_store_new_dirty_bitmap, Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 22/22] qmp: block-dirty-bitmap-remove: remove persistent, Vladimir Sementsov-Ogievskiy, 2017/02/25
- [Qemu-devel] [PATCH v16 09/22] qcow2: autoloading dirty bitmaps, Vladimir Sementsov-Ogievskiy, 2017/02/25