On 10/05/2017 09:35 AM, Stefan Hajnoczi wrote: > 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> >>
> 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; > } In fact, I think I would prefer to prioritize BDRV_BLOCK_EOF as the first condition, rather than the second, by swapping those two statements (that is, reading beyond EOF should set the EOF bit, regardless of whether nb_sectors was non-zero). -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature