On 01.11.2019 17:25 Vladimir Sementsov-Ogievskiy wrote:
> 01.11.2019 10:37, Tuguoyi wrote:
> > There are two issues in In check_constraints_on_bitmap(),
> > 1) The sanity check on the granularity will cause uint64_t integer
> > left-shift overflow when cluster_size is 2M and the granularity is
> > BIGGER than 32K.
> > 2) The way to calculate image size that the maximum bitmap supported
> > can map to is a bit incorrect.
> > This patch fix it by add a helper function to calculate the number of
> > bytes needed by a normal bitmap in image and compare it to the maximum
> > bitmap bytes supported by qemu.
> >
> > Fixes: 5f72826e7fc62167cf3a
> > Signed-off-by: Guoyi Tu <tu.gu...@h3c.com>
>
> You forget my
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
Sorry for that, it's my first time to submit patch to qemu, and should I send
another patch or not ?
> (I don't see changes except add "Fixes: " to commit msg and put
> get_bitmap_bytes_needed definition header into one line.)
Yes, Only minor changes are made, including removing 'inline' keyword.
Thanks for your patience in reviewing
> > ---
> > block/qcow2-bitmap.c | 14 +++++++++++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index
> > 98294a7..ef9ef62 100644
> > --- a/block/qcow2-bitmap.c
> > +++ b/block/qcow2-bitmap.c
> > @@ -142,6 +142,13 @@ static int check_table_entry(uint64_t entry, int
> cluster_size)
> > return 0;
> > }
> >
> > +static int64_t get_bitmap_bytes_needed(int64_t len, uint32_t
> > +granularity) {
> > + int64_t num_bits = DIV_ROUND_UP(len, granularity);
> > +
> > + return DIV_ROUND_UP(num_bits, 8); }
> > +
> > static int check_constraints_on_bitmap(BlockDriverState *bs,
> > const char *name,
> > uint32_t granularity, @@
> > -150,6 +157,7 @@ static int check_constraints_on_bitmap(BlockDriverState
> *bs,
> > BDRVQcow2State *s = bs->opaque;
> > int granularity_bits = ctz32(granularity);
> > int64_t len = bdrv_getlength(bs);
> > + int64_t bitmap_bytes;
> >
> > assert(granularity > 0);
> > assert((granularity & (granularity - 1)) == 0); @@ -171,9 +179,9
> > @@ static int check_constraints_on_bitmap(BlockDriverState *bs,
> > return -EINVAL;
> > }
> >
> > - if ((len > (uint64_t)BME_MAX_PHYS_SIZE << granularity_bits) ||
> > - (len > (uint64_t)BME_MAX_TABLE_SIZE * s->cluster_size <<
> > - granularity_bits))
> > + bitmap_bytes = get_bitmap_bytes_needed(len, granularity);
> > + if ((bitmap_bytes > (uint64_t)BME_MAX_PHYS_SIZE) ||
> > + (bitmap_bytes > (uint64_t)BME_MAX_TABLE_SIZE *
> > + s->cluster_size))
> > {
> > error_setg(errp, "Too much space will be occupied by the
> bitmap. "
> > "Use larger granularity");
> >
>
>
> --
> Best regards,
> Vladimir
