Re: [Qemu-block] [Qemu-devel] [PATCH v4 03/17] qcow2: Ensure bitmap serialization is aligned

[John Snow]

On 07/03/2017 11:10 AM, Eric Blake wrote:
> When subdividing a bitmap serialization, the code in hbitmap.c
> enforces that start/count parameters are aligned (except that
> count can end early at end-of-bitmap).  We exposed this required
> alignment through bdrv_dirty_bitmap_serialization_align(), but
> forgot to actually check that we comply with it.
> 
> Fortunately, qcow2 is never dividing bitmap serialization smaller
> than one cluster (which is a minimum of 512 bytes); so we are
> always compliant with the serialization alignment (which insists
> that we partition at least 64 bits per chunk) because we are doing
> at least 4k bits per chunk.
> 
> Still, it's safer to add an assertion (for the unlikely case that
> we'd ever support a cluster smaller than 512 bytes, or if the
> hbitmap implementation changes what it considers to be aligned),
> rather than leaving bdrv_dirty_bitmap_serialization_align()
> without a caller.
> 
> Signed-off-by: Eric Blake <address@hidden>

Reviewed-by: John Snow <address@hidden>

> 
> ---
> v4: new patch
> ---
>  block/qcow2-bitmap.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
> index 8448bec..75ee238 100644
> --- a/block/qcow2-bitmap.c
> +++ b/block/qcow2-bitmap.c
> @@ -274,10 +274,13 @@ static int free_bitmap_clusters(BlockDriverState *bs, 
> Qcow2BitmapTable *tb)
>  static uint64_t sectors_covered_by_bitmap_cluster(const BDRVQcow2State *s,
>                                                    const BdrvDirtyBitmap 
> *bitmap)
>  {
> -    uint32_t sector_granularity =
> +    uint64_t sector_granularity =
>              bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
> +    uint64_t sbc = sector_granularity * (s->cluster_size << 3);
> 
> -    return (uint64_t)sector_granularity * (s->cluster_size << 3);
> +    assert(QEMU_IS_ALIGNED(sbc,
> +                           bdrv_dirty_bitmap_serialization_align(bitmap)));
> +    return sbc;
>  }
> 
>  /* load_bitmap_data
>