diff mbox

[v3,07/10] block: Use bdrv_nb_sectors() where sectors, not bytes are wanted

Message ID 1401473631-10724-8-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster May 30, 2014, 6:13 p.m. UTC
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(-)

Comments

Benoît Canet June 2, 2014, 3:06 p.m. UTC | #1
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
> 
>
Markus Armbruster June 2, 2014, 4:45 p.m. UTC | #2
Benoît Canet <benoit.canet@irqsave.net> writes:

> 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>
[...]
>> 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.

I didn't examine how the bitmap is used.  My patch doesn't change the
value put into bitmap_size.

>>      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 mbox

Patch

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);
     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;