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
