On 04/12/2017 01:49 PM, Eric Blake wrote: > Half the callers were already scaling bytes to sectors; the other > half can eventually be simplified to use byte iteration. Both > callers were already using the result as a bool, so make that > explicit. Making the change also makes it easier for a future > dirty-bitmap patch to offload scaling over to the internal hbitmap. > > Signed-off-by: Eric Blake <ebl...@redhat.com> > --- > include/block/dirty-bitmap.h | 4 ++-- > block/dirty-bitmap.c | 8 ++++---- > block/mirror.c | 3 +-- > migration/block.c | 3 +-- > 4 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h > index efcec60..b8434e5 100644 > --- a/include/block/dirty-bitmap.h > +++ b/include/block/dirty-bitmap.h > @@ -34,8 +34,8 @@ bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap); > bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap); > const char *bdrv_dirty_bitmap_name(const BdrvDirtyBitmap *bitmap); > DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap); > -int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, > - int64_t sector); > +bool bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, > + int64_t offset); > void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap, > int64_t cur_sector, int64_t nr_sectors); > void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap, > diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c > index e3c2e34..c8100d2 100644 > --- a/block/dirty-bitmap.c > +++ b/block/dirty-bitmap.c > @@ -332,13 +332,13 @@ BlockDirtyInfoList > *bdrv_query_dirty_bitmaps(BlockDriverState *bs) > return list; > } > > -int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, > - int64_t sector) > +bool bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, > + int64_t offset) > { > if (bitmap) { > - return hbitmap_get(bitmap->bitmap, sector); > + return hbitmap_get(bitmap->bitmap, offset >> BDRV_SECTOR_BITS); > } else { > - return 0; > + return false; > } > } > > diff --git a/block/mirror.c b/block/mirror.c > index 1b98a77..1e2e655 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -359,8 +359,7 @@ static uint64_t coroutine_fn > mirror_iteration(MirrorBlockJob *s) > int64_t next_offset = offset + nb_chunks * s->granularity; > int64_t next_chunk = next_offset / s->granularity; > if (next_offset >= s->bdev_length || > - !bdrv_get_dirty(source, s->dirty_bitmap, > - next_offset >> BDRV_SECTOR_BITS)) { > + !bdrv_get_dirty(source, s->dirty_bitmap, next_offset)) { > break; > } > if (test_bit(next_chunk, s->in_flight_bitmap)) { > diff --git a/migration/block.c b/migration/block.c > index 3daa5c7..9e21aeb 100644 > --- a/migration/block.c > +++ b/migration/block.c > @@ -537,8 +537,7 @@ static int mig_save_device_dirty(QEMUFile *f, > BlkMigDevState *bmds, > } else { > blk_mig_unlock(); > } > - if (bdrv_get_dirty(bs, bmds->dirty_bitmap, sector)) { > - > + if (bdrv_get_dirty(bs, bmds->dirty_bitmap, sector * > BDRV_SECTOR_SIZE)) {
This one is a little weirder now though, isn't it? We're asking for the dirty status of a single byte, technically. In practice, the scaling factor will always cover the entire sector, but it reads a lot jankier now. > if (total_sectors - sector < BDRV_SECTORS_PER_DIRTY_CHUNK) { > nr_sectors = total_sectors - sector; > } else { > Oh well, it was always janky... Reviewed-by: John Snow <js...@redhat.com>