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

[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

[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

[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