Re: [Qemu-devel] [PATCH v2 1/9] qapi: Add optional field "name" to block dirty bitmap
On Thu, 03/13 14:15, Benoît Canet wrote: > The Wednesday 12 Mar 2014 à 14:30:56 (+0800), Fam Zheng wrote : > > This field will be set for user created dirty bitmap. Also pass in an > > error pointer to bdrv_create_dirty_bitmap, so when a name is already > > taken on this BDS, it can report an error message. This is not global > > check, two BDSes can have dirty bitmap with a common name. > > > > Implemented bdrv_find_dirty_bitmap to find a dirty bitmap by name, will > > be used later when other QMP commands want to reference dirty bitmap by > > name. > > > > Add bdrv_dirty_bitmap_make_anon. This unsets the name of dirty bitmap. > > > > Signed-off-by: Fam Zheng > > --- > > block-migration.c | 3 ++- > > block.c | 34 +- > > block/mirror.c| 2 +- > > include/block/block.h | 8 +++- > > qapi-schema.json | 4 +++- > > 5 files changed, 46 insertions(+), 5 deletions(-) > > > > diff --git a/block-migration.c b/block-migration.c > > index 897fdba..e6e016a 100644 > > --- a/block-migration.c > > +++ b/block-migration.c > > @@ -315,7 +315,8 @@ static void set_dirty_tracking(void) > > BlkMigDevState *bmds; > > > > QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) { > > -bmds->dirty_bitmap = bdrv_create_dirty_bitmap(bmds->bs, > > BLOCK_SIZE); > > +bmds->dirty_bitmap = bdrv_create_dirty_bitmap(bmds->bs, BLOCK_SIZE, > > + NULL, NULL); > > } > > } > > > > diff --git a/block.c b/block.c > > index f1ef4b0..ce48fff 100644 > > --- a/block.c > > +++ b/block.c > > @@ -52,6 +52,7 @@ > > > > struct BdrvDirtyBitmap { > > HBitmap *bitmap; > > +char *name; > > Why not making name a regular char array like bs->device_name or > bs->node_name. > Its emptyness could still be tested and it also would remove some memory > management > duties. Version 1 did use an array. This is Stefan's request. > > > QLIST_ENTRY(BdrvDirtyBitmap) list; > > }; > > > > @@ -5048,18 +5049,46 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, > > QEMUIOVector *qiov) > > return true; > > } > > > > -BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int > > granularity) > > +BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, > > +const char *name) > > +{ > > +BdrvDirtyBitmap *bm; > > +QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) { > > +if (!strcmp(name, bm->name)) { > > +return bm; > > +} > > +} > > +return NULL; > > +} > > + > > +void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap > > *bitmap) > > +{ > > +g_free(bitmap->name); > > +bitmap->name = NULL; > > +} > > + > > +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)) { > > +error_setg(errp, "Bitmap already exists: %s", name); > > +return NULL; > > +} > > granularity >>= BDRV_SECTOR_BITS; > > assert(granularity); > > bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS); > > bitmap = g_malloc0(sizeof(BdrvDirtyBitmap)); > > bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1); > > +if (name) { > > +bitmap->name = g_strdup(name); > > +} > > QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list); > > return bitmap; > > } > > @@ -5071,6 +5100,7 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, > > BdrvDirtyBitmap *bitmap) > > if (bm == bitmap) { > > QLIST_REMOVE(bitmap, list); > > hbitmap_free(bitmap->bitmap); > > +g_free(bitmap->name); > > g_free(bitmap); > > return; > > } > > @@ -5089,6 +5119,8 @@ BlockDirtyInfoList > > *bdrv_query_dirty_bitmaps(BlockDriverState *bs) > > info->count = bdrv_get_dirty_count(bs, bm); > > info->granularity = > > ((int64_t) BDRV_SECTOR_SIZE << > > hbitmap_granularity(bm->bitmap)); > > > +info->has_name = bm->name[0] != '\0'; > > +info->name = g_strdup(bm->name); > Could bm->name be == NULl here ? It doesn't matter. g_strdup returns NULL for NULL. > > > entry->value = info; > > *plist = entry; > > plist = &entry->next; > > diff --git a/block/mirror.c b/block/mirror.c > > index dd5ee05..be8b2a1 100644 > > --- a/block/mirror.c > > +++ b/block/mirror.c > > @@ -596,7 +596,7 @@ static void mirror_start_job(BlockDriverState *bs, > > BlockDriverState *target, > > s->granularity = granularity; > > s->buf
Re: [Qemu-devel] [PATCH v2 1/9] qapi: Add optional field "name" to block dirty bitmap
The Wednesday 12 Mar 2014 à 14:30:56 (+0800), Fam Zheng wrote : > This field will be set for user created dirty bitmap. Also pass in an > error pointer to bdrv_create_dirty_bitmap, so when a name is already > taken on this BDS, it can report an error message. This is not global > check, two BDSes can have dirty bitmap with a common name. > > Implemented bdrv_find_dirty_bitmap to find a dirty bitmap by name, will > be used later when other QMP commands want to reference dirty bitmap by > name. > > Add bdrv_dirty_bitmap_make_anon. This unsets the name of dirty bitmap. > > Signed-off-by: Fam Zheng > --- > block-migration.c | 3 ++- > block.c | 34 +- > block/mirror.c| 2 +- > include/block/block.h | 8 +++- > qapi-schema.json | 4 +++- > 5 files changed, 46 insertions(+), 5 deletions(-) > > diff --git a/block-migration.c b/block-migration.c > index 897fdba..e6e016a 100644 > --- a/block-migration.c > +++ b/block-migration.c > @@ -315,7 +315,8 @@ static void set_dirty_tracking(void) > BlkMigDevState *bmds; > > QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) { > -bmds->dirty_bitmap = bdrv_create_dirty_bitmap(bmds->bs, BLOCK_SIZE); > +bmds->dirty_bitmap = bdrv_create_dirty_bitmap(bmds->bs, BLOCK_SIZE, > + NULL, NULL); > } > } > > diff --git a/block.c b/block.c > index f1ef4b0..ce48fff 100644 > --- a/block.c > +++ b/block.c > @@ -52,6 +52,7 @@ > > struct BdrvDirtyBitmap { > HBitmap *bitmap; > +char *name; Why not making name a regular char array like bs->device_name or bs->node_name. Its emptyness could still be tested and it also would remove some memory management duties. > QLIST_ENTRY(BdrvDirtyBitmap) list; > }; > > @@ -5048,18 +5049,46 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, > QEMUIOVector *qiov) > return true; > } > > -BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int > granularity) > +BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, > +const char *name) > +{ > +BdrvDirtyBitmap *bm; > +QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) { > +if (!strcmp(name, bm->name)) { > +return bm; > +} > +} > +return NULL; > +} > + > +void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap > *bitmap) > +{ > +g_free(bitmap->name); > +bitmap->name = NULL; > +} > + > +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)) { > +error_setg(errp, "Bitmap already exists: %s", name); > +return NULL; > +} > granularity >>= BDRV_SECTOR_BITS; > assert(granularity); > bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS); > bitmap = g_malloc0(sizeof(BdrvDirtyBitmap)); > bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1); > +if (name) { > +bitmap->name = g_strdup(name); > +} > QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list); > return bitmap; > } > @@ -5071,6 +5100,7 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, > BdrvDirtyBitmap *bitmap) > if (bm == bitmap) { > QLIST_REMOVE(bitmap, list); > hbitmap_free(bitmap->bitmap); > +g_free(bitmap->name); > g_free(bitmap); > return; > } > @@ -5089,6 +5119,8 @@ BlockDirtyInfoList > *bdrv_query_dirty_bitmaps(BlockDriverState *bs) > info->count = bdrv_get_dirty_count(bs, bm); > info->granularity = > ((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap)); > +info->has_name = bm->name[0] != '\0'; > +info->name = g_strdup(bm->name); Could bm->name be == NULl here ? > entry->value = info; > *plist = entry; > plist = &entry->next; > diff --git a/block/mirror.c b/block/mirror.c > index dd5ee05..be8b2a1 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -596,7 +596,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); > bdrv_set_enable_write_cache(s->target, true); > bdrv_set_on_error(s->target, on_target_error, on_target_error); > bdrv_iostatus_enable(s->target); > diff --git a/include/block/block.h b/include/block/block.h > index 780f48b..a
[Qemu-devel] [PATCH v2 1/9] qapi: Add optional field "name" to block dirty bitmap
This field will be set for user created dirty bitmap. Also pass in an error pointer to bdrv_create_dirty_bitmap, so when a name is already taken on this BDS, it can report an error message. This is not global check, two BDSes can have dirty bitmap with a common name. Implemented bdrv_find_dirty_bitmap to find a dirty bitmap by name, will be used later when other QMP commands want to reference dirty bitmap by name. Add bdrv_dirty_bitmap_make_anon. This unsets the name of dirty bitmap. Signed-off-by: Fam Zheng --- block-migration.c | 3 ++- block.c | 34 +- block/mirror.c| 2 +- include/block/block.h | 8 +++- qapi-schema.json | 4 +++- 5 files changed, 46 insertions(+), 5 deletions(-) diff --git a/block-migration.c b/block-migration.c index 897fdba..e6e016a 100644 --- a/block-migration.c +++ b/block-migration.c @@ -315,7 +315,8 @@ static void set_dirty_tracking(void) BlkMigDevState *bmds; QSIMPLEQ_FOREACH(bmds, &block_mig_state.bmds_list, entry) { -bmds->dirty_bitmap = bdrv_create_dirty_bitmap(bmds->bs, BLOCK_SIZE); +bmds->dirty_bitmap = bdrv_create_dirty_bitmap(bmds->bs, BLOCK_SIZE, + NULL, NULL); } } diff --git a/block.c b/block.c index f1ef4b0..ce48fff 100644 --- a/block.c +++ b/block.c @@ -52,6 +52,7 @@ struct BdrvDirtyBitmap { HBitmap *bitmap; +char *name; QLIST_ENTRY(BdrvDirtyBitmap) list; }; @@ -5048,18 +5049,46 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov) return true; } -BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity) +BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, +const char *name) +{ +BdrvDirtyBitmap *bm; +QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) { +if (!strcmp(name, bm->name)) { +return bm; +} +} +return NULL; +} + +void bdrv_dirty_bitmap_make_anon(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) +{ +g_free(bitmap->name); +bitmap->name = NULL; +} + +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)) { +error_setg(errp, "Bitmap already exists: %s", name); +return NULL; +} granularity >>= BDRV_SECTOR_BITS; assert(granularity); bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS); bitmap = g_malloc0(sizeof(BdrvDirtyBitmap)); bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1); +if (name) { +bitmap->name = g_strdup(name); +} QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list); return bitmap; } @@ -5071,6 +5100,7 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) if (bm == bitmap) { QLIST_REMOVE(bitmap, list); hbitmap_free(bitmap->bitmap); +g_free(bitmap->name); g_free(bitmap); return; } @@ -5089,6 +5119,8 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs) info->count = bdrv_get_dirty_count(bs, bm); info->granularity = ((int64_t) BDRV_SECTOR_SIZE << hbitmap_granularity(bm->bitmap)); +info->has_name = bm->name[0] != '\0'; +info->name = g_strdup(bm->name); entry->value = info; *plist = entry; plist = &entry->next; diff --git a/block/mirror.c b/block/mirror.c index dd5ee05..be8b2a1 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -596,7 +596,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); bdrv_set_enable_write_cache(s->target, true); bdrv_set_on_error(s->target, on_target_error, on_target_error); bdrv_iostatus_enable(s->target); diff --git a/include/block/block.h b/include/block/block.h index 780f48b..aa0c5e4 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -437,7 +437,13 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov); struct HBitmapIter; typedef struct BdrvDirtyBitmap BdrvDirtyBitmap; -BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity); +BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, + int granularity, + const char *name, +