[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/9] qapi: Add optional field "name" to block di
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH 2/9] qapi: Add optional field "name" to block dirty bitmap |
Date: |
Fri, 17 Jan 2014 18:22:50 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Mon, Jan 13, 2014 at 06:39:41PM +0800, Fam Zheng wrote:
> diff --git a/block.c b/block.c
> index 6ad0368..16ef61b 100644
> --- a/block.c
> +++ b/block.c
> @@ -52,6 +52,7 @@
> struct BdrvDirtyBitmap {
> HBitmap *bitmap;
> int refcnt;
> + char name[1024];
Is there a reason for a fixed-size buffer? g_strdup() is nicer than
arbitrary limits, especially since the lifecycle of BdrvDirtyBitmap is
well-defined and you just need to add a g_free().
> +BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
> + int granularity,
> + const char *name,
> + Error **errp)
> {
> int64_t bitmap_size;
> BdrvDirtyBitmap *bitmap;
>
> assert((granularity & (granularity - 1)) == 0);
>
> + if (name && bdrv_find_dirty_bitmap(bs, name)) {
> + if (errp) {
> + error_setg(errp, "Bitmap already exists: %s", name);
> + }
error_setg() already checks that errp != NULL, you can drop the if
statement.
> diff --git a/block/mirror.c b/block/mirror.c
> index 2932bab..cc0665b 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -598,7 +598,7 @@ static void mirror_start_job(BlockDriverState *bs,
> BlockDriverState *target,
> s->granularity = granularity;
> s->buf_size = MAX(buf_size, granularity);
>
> - s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity);
> + s->dirty_bitmap = bdrv_create_dirty_bitmap(bs, granularity, NULL, errp);
No error return means the coroutine will be started even if creating the
dirty bitmap fails. I didn't check what happens but this definitely
makes the reader wonder if the error will be handled cleanly.
> diff --git a/qapi-schema.json b/qapi-schema.json
> index b9b051c..e91143a 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -920,6 +920,8 @@
> #
> # Block dirty bitmap information.
> #
> +# @name: the name of dirty bitmap
Since 2.0
- [Qemu-devel] [PATCH 0/9] QMP: Introduce incremental drive-backup with in-memory dirty bitmap, Fam Zheng, 2014/01/13
- [Qemu-devel] [PATCH 1/9] block: Introduce reference count for dirty bitmaps, Fam Zheng, 2014/01/13
- [Qemu-devel] [PATCH 3/9] block: Add bdrv_dirty_bitmap_make_anon, Fam Zheng, 2014/01/13
- [Qemu-devel] [PATCH 2/9] qapi: Add optional field "name" to block dirty bitmap, Fam Zheng, 2014/01/13
- Re: [Qemu-devel] [PATCH 2/9] qapi: Add optional field "name" to block dirty bitmap,
Stefan Hajnoczi <=
- [Qemu-devel] [PATCH 4/9] qmp: Add dirty-bitmap-add and dirty-bitmap-remove, Fam Zheng, 2014/01/13
- [Qemu-devel] [PATCH 5/9] block: Handle error of bdrv_getlength in bdrv_create_dirty_bitmap, Fam Zheng, 2014/01/13
- [Qemu-devel] [PATCH 7/9] block: Add support of "dirty-bitmap" sync mode, Fam Zheng, 2014/01/13
- [Qemu-devel] [PATCH 6/9] block: Introduce bdrv_dirty_bitmap_granularity(), Fam Zheng, 2014/01/13
- [Qemu-devel] [PATCH 8/9] qmp: Add dirty-bitmap-disable command, Fam Zheng, 2014/01/13
- [Qemu-devel] [PATCH 9/9] qapi: Add transaction support to dirty-bitmap-{add, disable}, Fam Zheng, 2014/01/13
- Re: [Qemu-devel] [PATCH 0/9] QMP: Introduce incremental drive-backup with in-memory dirty bitmap, Stefan Hajnoczi, 2014/01/17