Re: [Qemu-block] [PATCH v6 10/18] dirty-bitmap: Change bdrv_get_dirty_count() to report bytes

[Eric Blake]

On 09/08/2017 07:51 AM, Kevin Wolf wrote:
> Am 30.08.2017 um 23:05 hat Eric Blake geschrieben:
>> 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 
>> Reviewed-by: John Snow 
>> Reviewed-by: Juan Quintela 
> 
>> diff --git a/block/mirror.c b/block/mirror.c
>> index af13f5d658..cc47e21814 100644
>> --- a/block/mirror.c
>> +++ b/block/mirror.c
>> @@ -811,11 +811,10 @@ static void coroutine_fn mirror_run(void *opaque)
> 
> There is one more place before this one that needs to be converted,
> mirror_iteration() at line 343:
> 
> trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap) *
>   BDRV_SECTOR_SIZE);

Good catch. (I guess I'm a victim of sitting on a patch for several
months, across several rebases...)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH v6 10/18] dirty-bitmap: Change bdrv_get_dirty_count() to report bytes

[Kevin Wolf]

Am 30.08.2017 um 23:05 hat Eric Blake geschrieben:
> 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 
> Reviewed-by: John Snow 
> Reviewed-by: Juan Quintela 

> diff --git a/block/mirror.c b/block/mirror.c
> index af13f5d658..cc47e21814 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -811,11 +811,10 @@ static void coroutine_fn mirror_run(void *opaque)

There is one more place before this one that needs to be converted,
mirror_iteration() at line 343:

trace_mirror_restart_iter(s, bdrv_get_dirty_count(s->dirty_bitmap) *
  BDRV_SECTOR_SIZE);

Kevin

[Qemu-block] [PATCH v6 10/18] dirty-bitmap: Change bdrv_get_dirty_count() to report bytes

[Eric Blake]

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 
Reviewed-by: John Snow 
Reviewed-by: Juan Quintela 

---
v4: no change
v3: no change, add R-b
v2: no change
---
 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 20f230867d..f983d99def 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -425,7 +425,7 @@ BlockDirtyInfoList 
*bdrv_query_dirty_bitmaps(BlockDriverState *bs)
 QLIST_FOREACH(bm, >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);
@@ -653,7 +653,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 af13f5d658..cc47e21814 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -811,11 +811,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.
@@ -827,8 +826,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) {
@@ -869,7 +867,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);
@@ -888,8 +886,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(>common, QEMU_CLOCK_REALTIME, delay_ns);
 if (block_job_is_cancelled(>common)) {
diff --git a/migration/block.c b/migration/block.c
index 9171f60028..a3512945da 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -667,7 +667,7 @@ static int64_t get_remaining_dirty(void)
 aio_context_release(blk_get_aio_context(bmds->blk));
 }

-return dirty << BDRV_SECTOR_BITS;
+return dirty;
 }


-- 
2.13.5