On Tue, Oct 03, 2017 at 08:43:44PM -0500, Eric Blake wrote: > Handle a 0-length block status request up front, with a uniform > return value claiming the area is not allocated. > > Most callers don't pass a length of 0 to bdrv_get_block_status() > and friends; but it definitely happens with a 0-length read when > copy-on-read is enabled. While we could audit all callers to > ensure that they never make a 0-length request, and then assert > that fact, it was just as easy to fix things to always report > success (as long as the callers are careful to not go into an > infinite loop). However, we had inconsistent behavior on whether > the status is reported as allocated or defers to the backing > layer, depending on what callbacks the driver implements, and > possibly wasting quite a few CPU cycles to get to that answer. > Consistently reporting unallocated up front doesn't really hurt > anything, and makes it easier both for callers (0-length requests > now have well-defined behavior) and for drivers (drivers don't > have to deal with 0-length requests). > > Signed-off-by: Eric Blake <ebl...@redhat.com> > > --- > v2: new patch > --- > block/io.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/block/io.c b/block/io.c > index e0f904583f..1f5baac41d 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -1773,9 +1773,9 @@ static int64_t coroutine_fn > bdrv_co_get_block_status(BlockDriverState *bs, > return total_sectors; > } > > - if (sector_num >= total_sectors) { > + if (sector_num >= total_sectors || !nb_sectors) { > *pnum = 0; > - return BDRV_BLOCK_EOF; > + return sector_num >= total_sectors ? BDRV_BLOCK_EOF : 0; > }
Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> If you respin, please consider using a separate if statement to make the code clearer: if (!nb_sectors) { *pnum = 0; return 0; } if (sector_num >= total_sectors) { *pnum = 0; return BDRV_BLOCK_EOF; }