Re: [Qemu-devel] [PATCH v3 07/10] block: Use bdrv_nb_sectors() where sectors, not bytes are wanted

["Benoît Canet" <benoit.canet@irqsave.net>]

The Friday 30 May 2014 à 20:13:48 (+0200), Markus Armbruster wrote :
> Instead of bdrv_getlength().
> 
> Aside: a few of these callers don't handle errors.  I didn't
> investigate whether they should.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  block-migration.c | 9 ++++-----
>  block.c           | 3 +--
>  block/qcow2.c     | 2 +-
>  block/vmdk.c      | 5 ++---
>  4 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/block-migration.c b/block-migration.c
> index 1656270..1c7b7be 100644
> --- a/block-migration.c
> +++ b/block-migration.c
> @@ -186,7 +186,7 @@ static int bmds_aio_inflight(BlkMigDevState *bmds, int64_t sector)
>  {
>      int64_t chunk = sector / (int64_t)BDRV_SECTORS_PER_DIRTY_CHUNK;
>  
> -    if ((sector << BDRV_SECTOR_BITS) < bdrv_getlength(bmds->bs)) {
> +    if (sector < bdrv_nb_sectors(bmds->bs)) {
>          return !!(bmds->aio_bitmap[chunk / (sizeof(unsigned long) * 8)] &
>              (1UL << (chunk % (sizeof(unsigned long) * 8))));
>      } else {
> @@ -223,8 +223,7 @@ static void alloc_aio_bitmap(BlkMigDevState *bmds)
>      BlockDriverState *bs = bmds->bs;
>      int64_t bitmap_size;
>  
> -    bitmap_size = (bdrv_getlength(bs) >> BDRV_SECTOR_BITS) +
> -            BDRV_SECTORS_PER_DIRTY_CHUNK * 8 - 1;
> +    bitmap_size = bdrv_nb_sectors(bs) + BDRV_SECTORS_PER_DIRTY_CHUNK * 8 - 1;
>      bitmap_size /= BDRV_SECTORS_PER_DIRTY_CHUNK * 8;
>  
>      bmds->aio_bitmap = g_malloc0(bitmap_size);
> @@ -350,7 +349,7 @@ static void init_blk_migration_it(void *opaque, BlockDriverState *bs)
>      int64_t sectors;
>  
>      if (!bdrv_is_read_only(bs)) {
> -        sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS;
> +        sectors = bdrv_nb_sectors(bs);
>          if (sectors <= 0) {
>              return;
>          }
> @@ -800,7 +799,7 @@ static int block_load(QEMUFile *f, void *opaque, int version_id)
>  
>              if (bs != bs_prev) {
>                  bs_prev = bs;
> -                total_sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS;
> +                total_sectors = bdrv_nb_sectors(bs);
>                  if (total_sectors <= 0) {
>                      error_report("Error getting length of block device %s",
>                                   device_name);
> diff --git a/block.c b/block.c
> index cfeb497..8ebfb79 100644
> --- a/block.c
> +++ b/block.c
> @@ -5258,13 +5258,12 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, int granularity,
>  
>      granularity >>= BDRV_SECTOR_BITS;
>      assert(granularity);
> -    bitmap_size = bdrv_getlength(bs);
> +    bitmap_size = bdrv_nb_sectors(bs);

The name bitmap_size seems to imply the unit is byte.
>      if (bitmap_size < 0) {
>          error_setg_errno(errp, -bitmap_size, "could not get length of device");
>          errno = -bitmap_size;
>          return NULL;
>      }
> -    bitmap_size >>= BDRV_SECTOR_BITS;
>      bitmap = g_malloc0(sizeof(BdrvDirtyBitmap));
>      bitmap->bitmap = hbitmap_alloc(bitmap_size, ffs(granularity) - 1);
>      QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
> diff --git a/block/qcow2.c b/block/qcow2.c
> index a4b97e8..98f624c 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1539,7 +1539,7 @@ static int preallocate(BlockDriverState *bs)
>      int ret;
>      QCowL2Meta *meta;
>  
> -    nb_sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS;
> +    nb_sectors = bdrv_nb_sectors(bs);
>      offset = 0;
>  
>      while (nb_sectors) {
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 480ea37..5c5e73b 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -669,8 +669,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
>      if (le32_to_cpu(header.flags) & VMDK4_FLAG_RGD) {
>          l1_backup_offset = le64_to_cpu(header.rgd_offset) << 9;
>      }
> -    if (bdrv_getlength(file) <
> -            le64_to_cpu(header.grain_offset) * BDRV_SECTOR_SIZE) {
> +    if (bdrv_nb_sectors(file) < le64_to_cpu(header.grain_offset)) {
>          error_setg(errp, "File truncated, expecting at least %" PRId64 " bytes",
>                     (int64_t)(le64_to_cpu(header.grain_offset)
>                               * BDRV_SECTOR_SIZE));
> @@ -2004,7 +2003,7 @@ static int vmdk_check(BlockDriverState *bs, BdrvCheckResult *result,
>      BDRVVmdkState *s = bs->opaque;
>      VmdkExtent *extent = NULL;
>      int64_t sector_num = 0;
> -    int64_t total_sectors = bdrv_getlength(bs) / BDRV_SECTOR_SIZE;
> +    int64_t total_sectors = bdrv_nb_sectors(bs);
>      int ret;
>      uint64_t cluster_offset;
>  
> -- 
> 1.9.3
> 
>