Re: [Qemu-devel] [RFC PATCH 27/56] block/dirty-bitmap: Clean up signed vs. unsigned dirty counts

2017-08-09 Thread Markus Armbruster 
Eric Blake  writes:

> On 08/07/2017 09:45 AM, Markus Armbruster wrote:
>> hbitmap_count() returns uint64_t.
>> 
>> Clean up test-hbitmap.c to check its value with g_assert_cmpuint()
>> instead of g_assert_cmpint().
>> 
>> bdrv_get_dirty_count() and bdrv_get_meta_dirty_count() return its
>> value converted to int64_t.  Clean them up to return it unadulterated.
>> 
>> This moves the implicit conversion to some callers, so clean them up,
>> too.
>> 
>> mirror_run() assigns the value of bdrv_get_meta_dirty_count() to a
>> local int64_t variable.  Change it to uint64_t.  Signedness still gets
>> mixed up in the computation of s->common.len, but messing with that is
>> more than I can handle right now.
>> 
>> get_remaining_dirty() tallies bdrv_get_dirty_count() values in
>> int64_t.  Its caller block_save_pending() converts it back to
>> uint64_t.  Change get_remaining_dirty() to uint64_t.
>> 
>> Signed-off-by: Markus Armbruster 
>> ---
>>  block/dirty-bitmap.c |  4 ++--
>>  block/mirror.c   |  4 ++--
>>  block/trace-events   |  8 
>>  include/block/dirty-bitmap.h |  4 ++--
>>  migration/block.c|  4 ++--
>>  tests/test-hbitmap.c | 16 +---
>>  6 files changed, 21 insertions(+), 19 deletions(-)
>
> I don't know how much this will conflict with my pending work for
> byte-based block status, but I suspect it may be easier for your RFC to
> go in after my cleanups (I think you'll still have things to fix).

I fully expect to rebase on your work.

Re: [Qemu-devel] [RFC PATCH 27/56] block/dirty-bitmap: Clean up signed vs. unsigned dirty counts

2017-08-08 Thread Eric Blake 
On 08/07/2017 09:45 AM, Markus Armbruster wrote:
> hbitmap_count() returns uint64_t.
> 
> Clean up test-hbitmap.c to check its value with g_assert_cmpuint()
> instead of g_assert_cmpint().
> 
> bdrv_get_dirty_count() and bdrv_get_meta_dirty_count() return its
> value converted to int64_t.  Clean them up to return it unadulterated.
> 
> This moves the implicit conversion to some callers, so clean them up,
> too.
> 
> mirror_run() assigns the value of bdrv_get_meta_dirty_count() to a
> local int64_t variable.  Change it to uint64_t.  Signedness still gets
> mixed up in the computation of s->common.len, but messing with that is
> more than I can handle right now.
> 
> get_remaining_dirty() tallies bdrv_get_dirty_count() values in
> int64_t.  Its caller block_save_pending() converts it back to
> uint64_t.  Change get_remaining_dirty() to uint64_t.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  block/dirty-bitmap.c |  4 ++--
>  block/mirror.c   |  4 ++--
>  block/trace-events   |  8 
>  include/block/dirty-bitmap.h |  4 ++--
>  migration/block.c|  4 ++--
>  tests/test-hbitmap.c | 16 +---
>  6 files changed, 21 insertions(+), 19 deletions(-)

I don't know how much this will conflict with my pending work for
byte-based block status, but I suspect it may be easier for your RFC to
go in after my cleanups (I think you'll still have things to fix).

-- 
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-devel] [RFC PATCH 27/56] block/dirty-bitmap: Clean up signed vs. unsigned dirty counts

2017-08-07 Thread John Snow 


On 08/07/2017 10:45 AM, Markus Armbruster wrote:
> hbitmap_count() returns uint64_t.
> 
> Clean up test-hbitmap.c to check its value with g_assert_cmpuint()
> instead of g_assert_cmpint().
> 
> bdrv_get_dirty_count() and bdrv_get_meta_dirty_count() return its
> value converted to int64_t.  Clean them up to return it unadulterated.
> 
> This moves the implicit conversion to some callers, so clean them up,
> too.
> 
> mirror_run() assigns the value of bdrv_get_meta_dirty_count() to a
> local int64_t variable.  Change it to uint64_t.  Signedness still gets
> mixed up in the computation of s->common.len, but messing with that is
> more than I can handle right now.
> 
> get_remaining_dirty() tallies bdrv_get_dirty_count() values in
> int64_t.  Its caller block_save_pending() converts it back to
> uint64_t.  Change get_remaining_dirty() to uint64_t.
> 
> Signed-off-by: Markus Armbruster 

So these functions can't fail, so there's no reason to keep the int64_t
type around, OK.

Reviewed-by: John Snow

[Qemu-devel] [RFC PATCH 27/56] block/dirty-bitmap: Clean up signed vs. unsigned dirty counts

2017-08-07 Thread Markus Armbruster 
hbitmap_count() returns uint64_t.

Clean up test-hbitmap.c to check its value with g_assert_cmpuint()
instead of g_assert_cmpint().

bdrv_get_dirty_count() and bdrv_get_meta_dirty_count() return its
value converted to int64_t.  Clean them up to return it unadulterated.

This moves the implicit conversion to some callers, so clean them up,
too.

mirror_run() assigns the value of bdrv_get_meta_dirty_count() to a
local int64_t variable.  Change it to uint64_t.  Signedness still gets
mixed up in the computation of s->common.len, but messing with that is
more than I can handle right now.

get_remaining_dirty() tallies bdrv_get_dirty_count() values in
int64_t.  Its caller block_save_pending() converts it back to
uint64_t.  Change get_remaining_dirty() to uint64_t.

Signed-off-by: Markus Armbruster 
---
 block/dirty-bitmap.c |  4 ++--
 block/mirror.c   |  4 ++--
 block/trace-events   |  8 
 include/block/dirty-bitmap.h |  4 ++--
 migration/block.c|  4 ++--
 tests/test-hbitmap.c | 16 +---
 6 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index 30462d4..5b1699c 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -681,12 +681,12 @@ void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *iter, 
int64_t sector_num)
 hbitmap_iter_init(>hbi, iter->hbi.hb, sector_num);
 }
 
-int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
+uint64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap)
 {
 return hbitmap_count(bitmap->bitmap);
 }
 
-int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap)
+uint64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap)
 {
 return hbitmap_count(bitmap->meta);
 }
diff --git a/block/mirror.c b/block/mirror.c
index c9a6a3c..14c34e9 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -798,8 +798,8 @@ static void coroutine_fn mirror_run(void *opaque)
 assert(!s->dbi);
 s->dbi = bdrv_dirty_iter_new(s->dirty_bitmap, 0);
 for (;;) {
-uint64_t delay_ns = 0;
-int64_t cnt, delta;
+uint64_t cnt, delay_ns = 0;
+int64_t delta;
 bool should_complete;
 
 if (s->ret < 0) {
diff --git a/block/trace-events b/block/trace-events
index 071a8d7..464a11f 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -24,13 +24,13 @@ commit_start(void *bs, void *base, void *top, void *s) "bs 
%p base %p top %p s %
 
 # block/mirror.c
 mirror_start(void *bs, void *s, void *opaque) "bs %p s %p opaque %p"
-mirror_restart_iter(void *s, int64_t cnt) "s %p dirty count %"PRId64
+mirror_restart_iter(void *s, uint64_t cnt) "s %p dirty count %" PRIu64
 mirror_before_flush(void *s) "s %p"
-mirror_before_drain(void *s, int64_t cnt) "s %p dirty count %"PRId64
-mirror_before_sleep(void *s, int64_t cnt, int synced, uint64_t delay_ns) "s %p 
dirty count %"PRId64" synced %d delay %"PRIu64"ns"
+mirror_before_drain(void *s, uint64_t cnt) "s %p dirty count %" PRIu64
+mirror_before_sleep(void *s, uint64_t cnt, int synced, uint64_t delay_ns) "s 
%p dirty count %" PRIu64 " synced %d delay %" PRIu64 "ns"
 mirror_one_iteration(void *s, int64_t offset, uint64_t bytes) "s %p offset %" 
PRId64 " bytes %" PRIu64
 mirror_iteration_done(void *s, int64_t offset, uint64_t bytes, int ret) "s %p 
offset %" PRId64 " bytes %" PRIu64 " ret %d"
-mirror_yield(void *s, int64_t cnt, int buf_free_count, int in_flight) "s %p 
dirty count %"PRId64" free buffers %d in_flight %d"
+mirror_yield(void *s, uint64_t cnt, int buf_free_count, int in_flight) "s %p 
dirty count %" PRIu64 " free buffers %d in_flight %d"
 mirror_yield_in_flight(void *s, int64_t offset, int in_flight) "s %p offset %" 
PRId64 " in_flight %d"
 
 # block/backup.c
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index a79a58d..d7e0f61 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -91,8 +91,8 @@ void bdrv_reset_dirty_bitmap_locked(BdrvDirtyBitmap *bitmap,
 int64_t cur_sector, int64_t nr_sectors);
 int64_t bdrv_dirty_iter_next(BdrvDirtyBitmapIter *iter);
 void bdrv_set_dirty_iter(BdrvDirtyBitmapIter *hbi, int64_t sector_num);
-int64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
-int64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
+uint64_t bdrv_get_dirty_count(BdrvDirtyBitmap *bitmap);
+uint64_t bdrv_get_meta_dirty_count(BdrvDirtyBitmap *bitmap);
 void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
 bool bdrv_dirty_bitmap_readonly(const BdrvDirtyBitmap *bitmap);
 bool bdrv_has_readonly_bitmaps(BlockDriverState *bs);
diff --git a/migration/block.c b/migration/block.c
index 9171f60..59b7551 100644
--- a/migration/block.c
+++ b/migration/block.c
@@ -656,10 +656,10 @@ static int flush_blks(QEMUFile *f)
 
 /* Called with iothread lock taken.  */
 
-static int64_t get_remaining_dirty(void)
+static uint64_t get_remaining_dirty(void)
 {
 BlkMigDevState *bmds;
-int64_t dirty =