On 04/17/2017 08:33 PM, Eric Blake wrote: > We are gradually moving away from sector-based interfaces, towards > byte-based. Update the file protocol driver accordingly. > > Signed-off-by: Eric Blake <ebl...@redhat.com> > --- > block/file-posix.c | 47 +++++++++++++++++++++++------------------------ > 1 file changed, 23 insertions(+), 24 deletions(-)
This patch makes qemu-iotests 109 fail, due to an assertion in 6/31 that
the nested call to bdrv_block_status() [thanks to BDRV_BLOCK_RAW] is
still sector-aligned. The culprit:
> +static int64_t coroutine_fn raw_co_block_status(BlockDriverState *bs,
> + int64_t offset,
> + int64_t bytes, int64_t *pnum,
> + BlockDriverState **file)
> {
> - off_t start, data = 0, hole = 0;
> + off_t data = 0, hole = 0;
> int64_t total_size;
> int ret;
>
> @@ -1856,39 +1856,38 @@ static int64_t coroutine_fn
> raw_co_get_block_status(BlockDriverState *bs,
> return ret;
> }
>
> - start = sector_num * BDRV_SECTOR_SIZE;
> total_size = bdrv_getlength(bs);
total_size is always sector-aligned (bdrv_getlength() purposefully
widens the input file size, 560 bytes in the case of an empty qcow image
in test 109, out to a sector boundary)...
> if (total_size < 0) {
> return total_size;
> - } else if (start >= total_size) {
> + } else if (offset >= total_size) {
> *pnum = 0;
> return 0;
> - } else if (start + nb_sectors * BDRV_SECTOR_SIZE > total_size) {
> - nb_sectors = DIV_ROUND_UP(total_size - start, BDRV_SECTOR_SIZE);
> + } else if (offset + bytes > total_size) {
> + bytes = total_size - offset;
> }
>
> - ret = find_allocation(bs, start, &data, &hole);
> + ret = find_allocation(bs, offset, &data, &hole);
...while find_allocation still obeys byte-granularity limits of the
underlying file system, and therefore reports a hole starting at offset
560 (the only time a hole starts at a non-sector boundary); pre-patch
that didn't matter (because we rounded before returning sectors through
*pnum), but post-patch, returning 560 instead of 1024 triggers problems.
I'm still debating whether the best fix is to tweak the file-posix
limits to just round up to sector boundaries at EOF, or to make the
block layer itself special-case EOF in case other protocols also have
byte-granularity support. Any advice you have while reviewing is
appreciated, but it does mean I'll need a v2 of the series one way or
another.
And I didn't spot it during my earlier testing because I was skipping
109 as failing prior to my patches even started testing it - but now
that Jeff and Fam have both posted patches for two separate problems in
test 109 (the need to munge "len", and failure to do proper cleanup), I
no longer have that excuse.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
