Re: [Qemu-devel] [PATCH 07/12] dirty-bitmap: Change bdrv_get_dirty_count() to report bytes

[John Snow <jsnow@redhat.com>]

On 04/12/2017 01:49 PM, Eric Blake wrote:
> Thanks to recent cleanups, all callers were scaling a return value
> of sectors into bytes; do the scaling internally instead.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  block/dirty-bitmap.c |  4 ++--
>  block/mirror.c       | 13 +++++--------
>  migration/block.c    |  2 +-
>  3 files changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
> index 2f9f554..e3c2e34 100644
> --- a/block/dirty-bitmap.c
> +++ b/block/dirty-bitmap.c
> @@ -319,7 +319,7 @@ BlockDirtyInfoList *bdrv_query_dirty_bitmaps(BlockDriverState *bs)
>      QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
>          BlockDirtyInfo *info = g_new0(BlockDirtyInfo, 1);
>          BlockDirtyInfoList *entry = g_new0(BlockDirtyInfoList, 1);
> -        info->count = bdrv_get_dirty_count(bm) << BDRV_SECTOR_BITS;
> +        info->count = bdrv_get_dirty_count(bm);
>          info->granularity = bdrv_dirty_bitmap_granularity(bm);
>          info->has_name = !!bm->name;
>          info->name = g_strdup(bm->name);
> @@ -494,7 +494,7 @@ void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *iter, int64_t offset)
> 
>  int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
>  {
> -    return hbitmap_count(bitmap->bitmap);
> +    return hbitmap_count(bitmap->bitmap) << BDRV_SECTOR_BITS;
>  }
> 
>  int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap)
> diff --git a/block/mirror.c b/block/mirror.c
> index f404ff3..1b98a77 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -794,11 +794,10 @@ static void coroutine_fn mirror_run(void *opaque)
> 
>          cnt = bdrv_get_dirty_count(s->dirty_bitmap);
>          /* s->common.offset contains the number of bytes already processed so
> -         * far, cnt is the number of dirty sectors remaining and
> +         * far, cnt is the number of dirty bytes remaining and
>           * s->bytes_in_flight is the number of bytes currently being
>           * processed; together those are the current total operation length */
> -        s->common.len = s->common.offset + s->bytes_in_flight +
> -            cnt * BDRV_SECTOR_SIZE;
> +        s->common.len = s->common.offset + s->bytes_in_flight + cnt;
> 
>          /* Note that even when no rate limit is applied we need to yield
>           * periodically with no pending I/O so that bdrv_drain_all() returns.
> @@ -810,8 +809,7 @@ static void coroutine_fn mirror_run(void *opaque)
>              s->common.iostatus == BLOCK_DEVICE_IO_STATUS_OK) {
>              if (s->in_flight >= MAX_IN_FLIGHT || s->buf_free_count == 0 ||
>                  (cnt == 0 && s->in_flight > 0)) {
> -                trace_mirror_yield(s, cnt * BDRV_SECTOR_SIZE,
> -                                   s->buf_free_count, s->in_flight);
> +                trace_mirror_yield(s, cnt, s->buf_free_count, s->in_flight);
>                  mirror_wait_for_io(s);
>                  continue;
>              } else if (cnt != 0) {
> @@ -852,7 +850,7 @@ static void coroutine_fn mirror_run(void *opaque)
>               * whether to switch to target check one last time if I/O has
>               * come in the meanwhile, and if not flush the data to disk.
>               */
> -            trace_mirror_before_drain(s, cnt * BDRV_SECTOR_SIZE);
> +            trace_mirror_before_drain(s, cnt);
> 
>              bdrv_drained_begin(bs);
>              cnt = bdrv_get_dirty_count(s->dirty_bitmap);
> @@ -871,8 +869,7 @@ static void coroutine_fn mirror_run(void *opaque)
>          }
> 
>          ret = 0;
> -        trace_mirror_before_sleep(s, cnt * BDRV_SECTOR_SIZE,
> -                                  s->synced, delay_ns);
> +        trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
>          if (!s->synced) {
>              block_job_sleep_ns(&s->common, QEMU_CLOCK_REALTIME, delay_ns);
>              if (block_job_is_cancelled(&s->common)) {
> diff --git a/migration/block.c b/migration/block.c
> index 9a9c214..3daa5c7 100644
> --- a/migration/block.c
> +++ b/migration/block.c
> @@ -672,7 +672,7 @@ static int64_t get_remaining_dirty(void)
>          aio_context_release(blk_get_aio_context(bmds->blk));
>      }
> 
> -    return dirty << BDRV_SECTOR_BITS;
> +    return dirty;
>  }
> 
>  /* Called with iothread lock taken.  */
> 

Reviewed-by: John Snow <jsnow@redhat.com>