Re: [PATCH v2 2/9] block: inline bdrv_unallocated_blocks_are_zero()

[Vladimir Sementsov-Ogievskiy]

07.05.2020 17:08, Eric Blake wrote:
> On 5/7/20 3:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> > The function has the only user: bdrv_co_block_status(). Inline it to
>
> s/the only/only one/
>
> > simplify reviewing of the following patches, which will finally drop
> > unallocated_blocks_are_zero field too.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > ---
> >   include/block/block.h |  1 -
> >   block.c               | 15 ---------------
> >   block/io.c            | 11 ++++++++---
> >   3 files changed, 8 insertions(+), 19 deletions(-)
>
> > +++ b/block/io.c
> > @@ -2386,15 +2386,20 @@ static int coroutine_fn 
> > bdrv_co_block_status(BlockDriverState *bs,
> >       if (ret & (BDRV_BLOCK_DATA | BDRV_BLOCK_ZERO)) {
> >           ret |= BDRV_BLOCK_ALLOCATED;
> >       } else if (want_zero) {
> > -        if (bdrv_unallocated_blocks_are_zero(bs)) {
> > -            ret |= BDRV_BLOCK_ZERO;
> > -        } else if (bs->backing) {
> > +        if (bs->backing) {
> >               BlockDriverState *bs2 = bs->backing->bs;
> >               int64_t size2 = bdrv_getlength(bs2);
> >               if (size2 >= 0 && offset >= size2) {
> >                   ret |= BDRV_BLOCK_ZERO;
> >               }
> > +        } else {
> > +            BlockDriverInfo bdi;
> > +            int ret2 = bdrv_get_info(bs, &bdi);
> > +
> > +            if (ret2 == 0 && bdi.unallocated_blocks_are_zero) {
>
> Could perhaps condense to:
>
> else {
>      BlockDriverInfo bdi;
>
>      if (bdrv_get_info(bs, &bd) == 0 &&
>          bdi.unallocated_blocks_are_zero) {
>
> but that's cosmetic.

I'll keep it as is, as it is to be removed in 09 anyway.

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


--
Best regards,
Vladimir