[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH] block/gluster: glfs_lseek() workaround
From: |
Jeff Cody |
Subject: |
Re: [Qemu-block] [PATCH] block/gluster: glfs_lseek() workaround |
Date: |
Fri, 26 May 2017 15:14:07 -0400 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Tue, May 23, 2017 at 01:27:50PM -0400, 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:
>
> LSEEK(2):
>
> off_t lseek(int fd, off_t offset, int whence);
>
> [...]
>
> SEEK_HOLE
> Adjust the file offset to the next hole in the file greater
> than or equal to offset. If offset points into the middle of
> a hole, then the file offset is set to offset. If there is no
> hole past offset, then the file offset is adjusted to the end
> of the file (i.e., there is an implicit hole at the end of
> any file).
>
> [...]
>
> RETURN VALUE
> Upon successful completion, lseek() returns the resulting
> offset location as measured in bytes from the beginning of the
> file. On error, the value (off_t) -1 is returned and errno is
> set to indicate the error
>
> However, occasionally glfs_lseek() for SEEK_HOLE/DATA will return a
> value less than the passed offset, yet greater than zero.
>
> For instance, here are example values observed from this call:
>
> offs = glfs_lseek(s->fd, start, SEEK_HOLE);
> if (offs < 0) {
> return -errno; /* D1 and (H3 or H4) */
> }
>
> start == 7608336384
> offs == 7607877632
>
> This causes QEMU to abort on the assert test. When this value is
> returned, errno is also 0.
>
> This is a reported and known bug to glusterfs:
> https://bugzilla.redhat.com/show_bug.cgi?id=1425293
>
> 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.
>
> 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.
>
> Signed-off-by: Jeff Cody <address@hidden>
> ---
> block/gluster.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/block/gluster.c b/block/gluster.c
> index 7c76cd0..c147909e 100644
> --- a/block/gluster.c
> +++ 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. */
> + return -EIO;
> + }
>
> if (offs > start) {
> /* D2: in hole, next data at offs */
> @@ -1307,7 +1314,14 @@ static int find_allocation(BlockDriverState *bs, off_t
> start,
> if (offs < 0) {
> return -errno; /* D1 and (H3 or H4) */
> }
> - 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 H4. Unfortunately some
> + * versions of libgfapi will return offs < start, so an assert here
> + * will unneccesarily abort QEMU. */
> + return -EIO;
> + }
>
> if (offs > start) {
> /*
> --
> 2.9.3
>
Thanks,
Applied (with comment fixes) to my block branch:
git://github.com/codyprime/qemu-kvm-jtc block
-Jeff