try_seek_hole() doesn't really seek to a hole, it tries to find out whether its argument is in a hole or not, and where the hole or non-hole ends. Rename to find_allocation() and add a proper function comment.
Using arguments passed by reference like local variables is a bad habit. Only assign to them right before return. Avoid nesting of conditionals. When find_allocation() fails, don't make up a range that'll get mapped to nb_sectors, simply set *pnum = nb_sectors directly. Don't repeat BDRV_BLOCK_OFFSET_VALID | start. Drop a pointless assertion, add some meaningful ones. Signed-off-by: Markus Armbruster <arm...@redhat.com> --- block/raw-posix.c | 62 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 2a12a50..ea5b3b8 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -1478,28 +1478,43 @@ out: return result; } -static int try_seek_hole(BlockDriverState *bs, off_t start, off_t *data, - off_t *hole) +/* + * Find allocation range in @bs around offset @start. + * If @start is in a hole, store @start in @hole and the end of the + * hole in @data, and return 0. + * If @start is in a data, store @start to @data, and the end of the + * data to @hole, and return 0. + * If we can't find out, return -errno. + */ +static int find_allocation(BlockDriverState *bs, off_t start, + off_t *data, off_t *hole) { #if defined SEEK_HOLE && defined SEEK_DATA BDRVRawState *s = bs->opaque; + off_t offs; - *hole = lseek(s->fd, start, SEEK_HOLE); - if (*hole == -1) { + offs = lseek(s->fd, start, SEEK_HOLE); + if (offs < 0) { return -errno; } + assert(offs >= start); - if (*hole > start) { + if (offs > start) { + /* in data, next hole at offs */ *data = start; - } else { - /* On a hole. We need another syscall to find its end. */ - *data = lseek(s->fd, start, SEEK_DATA); - if (*data < 0) { - /* no idea where the hole ends, give up (unlikely to happen) */ - return -errno; - } + *hole = offs; + return 0; } + /* in hole, end not yet known */ + offs = lseek(s->fd, start, SEEK_DATA); + if (offs < 0) { + /* no idea where the hole ends, give up (unlikely to happen) */ + return -errno; + } + assert(offs > start); + *hole = start; + *data = offs; return 0; #else return -ENOTSUP; @@ -1543,25 +1558,22 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs, nb_sectors = DIV_ROUND_UP(total_size - start, BDRV_SECTOR_SIZE); } - ret = try_seek_hole(bs, start, &data, &hole); - if (ret < 0) { - /* Assume everything is allocated. */ - data = 0; - hole = start + nb_sectors * BDRV_SECTOR_SIZE; - ret = 0; - } - - assert(ret >= 0); - - if (data <= start) { + ret = BDRV_BLOCK_OFFSET_VALID | start; + if (find_allocation(bs, start, &data, &hole) < 0) { + /* No info available, so pretend there are no holes */ + *pnum = nb_sectors; + ret |= BDRV_BLOCK_DATA; + } else if (data == start) { /* On a data extent, compute sectors to the end of the extent. */ *pnum = MIN(nb_sectors, (hole - start) / BDRV_SECTOR_SIZE); - return ret | BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | start; + ret |= BDRV_BLOCK_DATA; } else { /* On a hole, compute sectors to the beginning of the next extent. */ + assert(hole == start); *pnum = MIN(nb_sectors, (data - start) / BDRV_SECTOR_SIZE); - return ret | BDRV_BLOCK_ZERO | BDRV_BLOCK_OFFSET_VALID | start; + ret |= BDRV_BLOCK_ZERO; } + return ret; } static coroutine_fn BlockAIOCB *raw_aio_discard(BlockDriverState *bs, -- 1.9.3