Message ID | 20170703151051.31327-15-eblake@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 07/03/2017 11:10 AM, Eric Blake wrote: > Now that we have adjusted the majority of the calls this function > makes to be byte-based, it is easier to read the code if it makes > passes over the image using bytes rather than sectors. > > Signed-off-by: Eric Blake <eblake@redhat.com> > Reviewed-by: John Snow <jsnow@redhat.com> Vladimir, look good to you? --js
03.07.2017 18:10, Eric Blake wrote: > Now that we have adjusted the majority of the calls this function > makes to be byte-based, it is easier to read the code if it makes > passes over the image using bytes rather than sectors. > > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > v4: new patch > --- > block/qcow2-bitmap.c | 22 ++++++++-------------- > 1 file changed, 8 insertions(+), 14 deletions(-) > > diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c > index 280960a..1de2ab5 100644 > --- a/block/qcow2-bitmap.c > +++ b/block/qcow2-bitmap.c > @@ -291,9 +291,8 @@ static int load_bitmap_data(BlockDriverState *bs, > { > int ret = 0; > BDRVQcow2State *s = bs->opaque; > - uint64_t sector, limit, sbc; > + uint64_t offset, limit; > uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap); > - uint64_t bm_sectors = DIV_ROUND_UP(bm_size, BDRV_SECTOR_SIZE); > uint8_t *buf = NULL; > uint64_t i, tab_size = > size_to_clusters(s, > @@ -305,32 +304,27 @@ static int load_bitmap_data(BlockDriverState *bs, > > buf = g_malloc(s->cluster_size); > limit = bytes_covered_by_bitmap_cluster(s, bitmap); > - sbc = limit >> BDRV_SECTOR_BITS; > - for (i = 0, sector = 0; i < tab_size; ++i, sector += sbc) { > - uint64_t count = MIN(bm_sectors - sector, sbc); > + for (i = 0, offset = 0; i < tab_size; ++i, offset += limit) { > + uint64_t count = MIN(bm_size - offset, limit); > uint64_t entry = bitmap_table[i]; > - uint64_t offset = entry & BME_TABLE_ENTRY_OFFSET_MASK; > + uint64_t data_offset = entry & BME_TABLE_ENTRY_OFFSET_MASK; > > assert(check_table_entry(entry, s->cluster_size) == 0); > > - if (offset == 0) { > + if (data_offset == 0) { > if (entry & BME_TABLE_ENTRY_FLAG_ALL_ONES) { > - bdrv_dirty_bitmap_deserialize_ones(bitmap, > - sector * BDRV_SECTOR_SIZE, > - count * BDRV_SECTOR_SIZE, > + bdrv_dirty_bitmap_deserialize_ones(bitmap, offset, count, The change is that now 3rd parameter may not be aligned by sectors, but looks like it should be ok. Reviewed-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com> > false); > } else { > /* No need to deserialize zeros because the dirty bitmap is > * already cleared */ > } > } else { > - ret = bdrv_pread(bs->file, offset, buf, s->cluster_size); > + ret = bdrv_pread(bs->file, data_offset, buf, s->cluster_size); > if (ret < 0) { > goto finish; > } > - bdrv_dirty_bitmap_deserialize_part(bitmap, buf, > - sector * BDRV_SECTOR_SIZE, > - count * BDRV_SECTOR_SIZE, > + bdrv_dirty_bitmap_deserialize_part(bitmap, buf, offset, count, > false); > } > }
On 07/13/2017 04:16 AM, Vladimir Sementsov-Ogievskiy wrote: > 03.07.2017 18:10, Eric Blake wrote: >> Now that we have adjusted the majority of the calls this function >> makes to be byte-based, it is easier to read the code if it makes >> passes over the image using bytes rather than sectors. >> >> Signed-off-by: Eric Blake <eblake@redhat.com> >> >> --- >> v4: new patch >> --- >> >> - if (offset == 0) { >> + if (data_offset == 0) { >> if (entry & BME_TABLE_ENTRY_FLAG_ALL_ONES) { >> - bdrv_dirty_bitmap_deserialize_ones(bitmap, >> - sector * >> BDRV_SECTOR_SIZE, >> - count * >> BDRV_SECTOR_SIZE, >> + bdrv_dirty_bitmap_deserialize_ones(bitmap, offset, >> count, > > The change is that now 3rd parameter may not be aligned by sectors, but > looks like it should be ok. Correct - the dirty bitmap automatically widens any non-aligned input into granularity. > > Reviewed-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com> > I posted a v5 prior to this email, if you want to add your Reviewed-by on any patches there (I think this patch went unchanged into that revision, but scripts don't always pick up R-b given on a previous version of a patch).
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index 280960a..1de2ab5 100644 --- a/block/qcow2-bitmap.c +++ b/block/qcow2-bitmap.c @@ -291,9 +291,8 @@ static int load_bitmap_data(BlockDriverState *bs, { int ret = 0; BDRVQcow2State *s = bs->opaque; - uint64_t sector, limit, sbc; + uint64_t offset, limit; uint64_t bm_size = bdrv_dirty_bitmap_size(bitmap); - uint64_t bm_sectors = DIV_ROUND_UP(bm_size, BDRV_SECTOR_SIZE); uint8_t *buf = NULL; uint64_t i, tab_size = size_to_clusters(s, @@ -305,32 +304,27 @@ static int load_bitmap_data(BlockDriverState *bs, buf = g_malloc(s->cluster_size); limit = bytes_covered_by_bitmap_cluster(s, bitmap); - sbc = limit >> BDRV_SECTOR_BITS; - for (i = 0, sector = 0; i < tab_size; ++i, sector += sbc) { - uint64_t count = MIN(bm_sectors - sector, sbc); + for (i = 0, offset = 0; i < tab_size; ++i, offset += limit) { + uint64_t count = MIN(bm_size - offset, limit); uint64_t entry = bitmap_table[i]; - uint64_t offset = entry & BME_TABLE_ENTRY_OFFSET_MASK; + uint64_t data_offset = entry & BME_TABLE_ENTRY_OFFSET_MASK; assert(check_table_entry(entry, s->cluster_size) == 0); - if (offset == 0) { + if (data_offset == 0) { if (entry & BME_TABLE_ENTRY_FLAG_ALL_ONES) { - bdrv_dirty_bitmap_deserialize_ones(bitmap, - sector * BDRV_SECTOR_SIZE, - count * BDRV_SECTOR_SIZE, + bdrv_dirty_bitmap_deserialize_ones(bitmap, offset, count, false); } else { /* No need to deserialize zeros because the dirty bitmap is * already cleared */ } } else { - ret = bdrv_pread(bs->file, offset, buf, s->cluster_size); + ret = bdrv_pread(bs->file, data_offset, buf, s->cluster_size); if (ret < 0) { goto finish; } - bdrv_dirty_bitmap_deserialize_part(bitmap, buf, - sector * BDRV_SECTOR_SIZE, - count * BDRV_SECTOR_SIZE, + bdrv_dirty_bitmap_deserialize_part(bitmap, buf, offset, count, false); } }
Now that we have adjusted the majority of the calls this function makes to be byte-based, it is easier to read the code if it makes passes over the image using bytes rather than sectors. Signed-off-by: Eric Blake <eblake@redhat.com> --- v4: new patch --- block/qcow2-bitmap.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-)