| Message ID | 20191219100348.24827-11-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,01/10] hbitmap: assert that we don't create bitmap larger than INT64_MAX | expand |
On 19.12.19 11:03, Vladimir Sementsov-Ogievskiy wrote: > store_bitmap_data() loop does bdrv_set_dirty_iter() on each iteration, > which means that we actually don't need iterator itself and we can use > simpler bitmap API. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/qcow2-bitmap.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c > index c6c8ebbe89..015f5d18d2 100644 > --- a/block/qcow2-bitmap.c > +++ b/block/qcow2-bitmap.c [...] > @@ -1360,19 +1361,17 @@ static uint64_t *store_bitmap_data(BlockDriverState *bs, (One line more context:) > if (end >= bm_size) > break; > } Can we drop this now? If so, and with that done: Reviewed-by: Max Reitz <mreitz@redhat.com> > > - bdrv_set_dirty_iter(dbi, end); > + offset = end; > }
20.01.2020 17:18, Max Reitz wrote: > On 19.12.19 11:03, Vladimir Sementsov-Ogievskiy wrote: >> store_bitmap_data() loop does bdrv_set_dirty_iter() on each iteration, >> which means that we actually don't need iterator itself and we can use >> simpler bitmap API. >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> --- >> block/qcow2-bitmap.c | 11 +++++------ >> 1 file changed, 5 insertions(+), 6 deletions(-) >> >> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c >> index c6c8ebbe89..015f5d18d2 100644 >> --- a/block/qcow2-bitmap.c >> +++ b/block/qcow2-bitmap.c > > [...] > >> @@ -1360,19 +1361,17 @@ static uint64_t *store_bitmap_data(BlockDriverState *bs, > > (One line more context:) > >> if (end >= bm_size) >> break; >> } > > Can we drop this now? Yes, we can, as hbitmap_next_dirty returns -1 in this case. > > If so, and with that done: > > Reviewed-by: Max Reitz <mreitz@redhat.com> > >> >> - bdrv_set_dirty_iter(dbi, end); >> + offset = end; >> } >
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index c6c8ebbe89..015f5d18d2 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -1289,7 +1289,6 @@ static uint64_t *store_bitmap_data(BlockDriverState *bs, uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap); const char *bm_name = bdrv_dirty_bitmap_name(bitmap); uint8_t *buf = NULL; - BdrvDirtyBitmapIter *dbi; uint64_t *tb; uint64_t tb_size = size_to_clusters(s, @@ -1308,12 +1307,14 @@ static uint64_t *store_bitmap_data(BlockDriverState *bs, return NULL; } - dbi = bdrv_dirty_iter_new(bitmap); buf = g_malloc(s->cluster_size); limit = bytes_covered_by_bitmap_cluster(s, bitmap); assert(DIV_ROUND_UP(bm_size, limit) == tb_size); - while ((offset = bdrv_dirty_iter_next(dbi)) >= 0) { + offset = 0; + while ((offset = bdrv_dirty_bitmap_next_dirty(bitmap, offset, INT64_MAX)) + >= 0) + { uint64_t cluster = offset / limit; uint64_t end, write_size; int64_t off; @@ -1360,19 +1361,17 @@ static uint64_t *store_bitmap_data(BlockDriverState *bs, break; } - bdrv_set_dirty_iter(dbi, end); + offset = end; } *bitmap_table_size = tb_size; g_free(buf); - bdrv_dirty_iter_free(dbi); return tb; fail: clear_bitmap_table(bs, tb, tb_size); g_free(buf); - bdrv_dirty_iter_free(dbi); g_free(tb); return NULL;
store_bitmap_data() loop does bdrv_set_dirty_iter() on each iteration, which means that we actually don't need iterator itself and we can use simpler bitmap API. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- block/qcow2-bitmap.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)