Re: [Qemu-devel] [PATCH] block/gluster: glfs_lseek() workaround

[Eric Blake]

On 05/23/2017 12:27 PM, Jeff Cody wrote:
> On current released versions of glusterfs, glfs_lseek() will sometimes
> return invalid values for SEEK_DATA or SEEK_HOLE.  For SEEK_DATA and
> SEEK_HOLE, the returned value should be >= the passed offset, or < 0 in
> the case of error:
> 

> However, occasionally glfs_lseek() for SEEK_HOLE/DATA will return a
> value less than the passed offset, yet greater than zero.

Ouch.

> 
> Although this is being fixed in gluster, we still should work around it
> in QEMU, given that multiple released versions of gluster behave this
> way.

Fair enough.

> 
> This patch treats the return case of (offs < start) the same as if an
> error value other than ENXIO is returned; we will assume we learned
> nothing, and there are no holes in the file.

Yes, holes are merely an optimization, so treating bugs by the
pessimistic fallback of no holes rather than aborting is reasonable.


> +++ b/block/gluster.c
> @@ -1275,7 +1275,14 @@ static int find_allocation(BlockDriverState *bs, off_t 
> start,
>      if (offs < 0) {
>          return -errno;          /* D3 or D4 */
>      }
> -    assert(offs >= start);
> +
> +    if (offs < start) {
> +        /* This is not a valid return by lseek().  We are safe to just return
> +         * -EIO in this case, and we'll treat it like D4. Unfortunately some
> +         *  versions of libgfapi will return offs < start, so an assert here
> +         *  will unneccesarily abort QEMU. */

s/unneccesarily/unnecessarily/ (twice)

With spelling fix,
Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature