Re: [PATCH v5 1/5] block/io: fix bdrv_co_block_status_above

[Vladimir Sementsov-Ogievskiy]

Thanks a lot for reviewing!

18.08.2020 17:15, Alberto Garcia wrote:
> On Wed 10 Jun 2020 02:04:22 PM CEST, Vladimir Sementsov-Ogievskiy wrote:
> > +             * The top layer deferred to this layer, and because this layer is
> > +             * short, any zeroes that we synthesize beyond EOF behave as if 
> > they
> > +             * were allocated at this layer
> >                */
> > +            assert(ret & BDRV_BLOCK_EOF);
> >               *pnum = bytes;
> > +            if (file) {
> > +                *file = p;
> > +            }
> > +            return BDRV_BLOCK_ZERO | BDRV_BLOCK_ALLOCATED;
>
> You don't add BDRV_BLOCK_EOF to the return code here ?

No we shouldn't, as this is the end of backing file when the top layer is 
larger.

> > +        res = bdrv_block_status_above(bs, NULL, offset, bytes, &nr, NULL, 
> > NULL);
> > +        offset += nr;
> > +        bytes -= nr;
> > +    } while (res >= 0 && (res & BDRV_BLOCK_ZERO) && nr && bytes);
>
> About this last "... && nr && bytes", I think 'nr' already implies
> 'bytes', maybe you want to use an assertion instead?

No, on the last iteration, bytes would be 0 and nr is a last chunk. So, if we 
check
only nr, we'll do one extra call of bdrv_block_status_above with bytes=0, I 
don't
want do it.

> Berto


--
Best regards,
Vladimir