Re: [PATCH] migration/block-dirty-bitmap: fix larger granularity bitmaps

[Eric Blake]

On 10/21/20 9:44 AM, Stefan Reiter wrote:
> sectors_per_chunk is a 64 bit integer, but the calculation is done in 32
> bits, leading to an overflow for coarse bitmap granularities.
> 
> If that results in the value 0, it leads to a hang where no progress is
> made but send_bitmap_bits is constantly called with nr_sectors being 0.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
>  migration/block-dirty-bitmap.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
> index 5bef793ac0..5398869e2b 100644
> --- a/migration/block-dirty-bitmap.c
> +++ b/migration/block-dirty-bitmap.c
> @@ -562,8 +562,9 @@ static int add_bitmaps_to_list(DBMSaveState *s, 
> BlockDriverState *bs,
>          dbms->bitmap_alias = g_strdup(bitmap_alias);
>          dbms->bitmap = bitmap;
>          dbms->total_sectors = bdrv_nb_sectors(bs);
> -        dbms->sectors_per_chunk = CHUNK_SIZE * 8 *
> +        dbms->sectors_per_chunk = CHUNK_SIZE * 8lu *

8lu is not necessarily 64-bit; you need llu.  Also, I prefer
capitalizing the type suffix, as in 8LLU.

I can touch that up while queuing through my bitmaps tree, so:

Reviewed-by: Eric Blake <eblake@redhat.com>

>              bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
> +        assert(dbms->sectors_per_chunk != 0);
>          if (bdrv_dirty_bitmap_enabled(bitmap)) {
>              dbms->flags |= DIRTY_BITMAP_MIG_START_FLAG_ENABLED;
>          }
> 

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