[Qemu-devel] [PATCH for-2.12] block: handle invalid lseek returns gracefully

[Qemu-devel] [PATCH for-2.12] block: handle invalid lseek returns gracefully

[Jeff Cody]

In commit 223a23c198787328ae75bc65d84edf5fde33c0b6, we implemented a
workaround in the gluster driver to handle invalid values returned for
SEEK_DATA or SEEK_HOLE.

In some instances, these same invalid values can be seen in the posix
file handler as well - for example, it has been reported on FUSE gluster
mounts.

Calling assert() for these invalid values is overly harsh; we can safely
return -EIO and allow this case to be treated as a "learned nothing"
case (e.g., D4 / H4, as commented in the code).

This patch does the same thing that 223a23c198787 did for gluster.c,
except in file-posix.c

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/file-posix.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/block/file-posix.c b/block/file-posix.c
index d7fb772c14..a2f6d8a8c8 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2114,7 +2114,12 @@ 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. */
+        return -EIO;
+    }
 
     if (offs > start) {
         /* D2: in hole, next data at offs */
@@ -2146,7 +2151,12 @@ 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. */
+        return -EIO;
+    }
 
     if (offs > start) {
         /*
-- 
2.13.6

Re: [Qemu-devel] [PATCH for-2.12] block: handle invalid lseek returns gracefully

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1212 bytes --]

On 04/02/2018 11:37 PM, Jeff Cody wrote:
> In commit 223a23c198787328ae75bc65d84edf5fde33c0b6, we implemented a
> workaround in the gluster driver to handle invalid values returned for
> SEEK_DATA or SEEK_HOLE.
> 
> In some instances, these same invalid values can be seen in the posix
> file handler as well - for example, it has been reported on FUSE gluster
> mounts.

Yuck - that should be reported to the FUSE and gluster folks, as it does
not scale to have everyone else work around their bug.  But in the
meantime, working around it here is acceptable.

> 
> Calling assert() for these invalid values is overly harsh; we can safely
> return -EIO and allow this case to be treated as a "learned nothing"
> case (e.g., D4 / H4, as commented in the code).
> 
> This patch does the same thing that 223a23c198787 did for gluster.c,
> except in file-posix.c
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/file-posix.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH for-2.12] block: handle invalid lseek returns gracefully

[Jeff Cody]

On Tue, Apr 03, 2018 at 07:57:14AM -0500, Eric Blake wrote:
> On 04/02/2018 11:37 PM, Jeff Cody wrote:
> > In commit 223a23c198787328ae75bc65d84edf5fde33c0b6, we implemented a
> > workaround in the gluster driver to handle invalid values returned for
> > SEEK_DATA or SEEK_HOLE.
> > 
> > In some instances, these same invalid values can be seen in the posix
> > file handler as well - for example, it has been reported on FUSE gluster
> > mounts.
> 
> Yuck - that should be reported to the FUSE and gluster folks, as it does
> not scale to have everyone else work around their bug.  But in the
> meantime, working around it here is acceptable.
> 

Yes - there is a bug report on gluster still open for the lseek issue, I'll
make sure to add on it that it affects FUSE as well, so that they are aware.

-Jeff

Re: [Qemu-devel] [PATCH for-2.12] block: handle invalid lseek returns gracefully

[Kevin Wolf]

Am 03.04.2018 um 06:37 hat Jeff Cody geschrieben:
> In commit 223a23c198787328ae75bc65d84edf5fde33c0b6, we implemented a
> workaround in the gluster driver to handle invalid values returned for
> SEEK_DATA or SEEK_HOLE.
> 
> In some instances, these same invalid values can be seen in the posix
> file handler as well - for example, it has been reported on FUSE gluster
> mounts.
> 
> Calling assert() for these invalid values is overly harsh; we can safely
> return -EIO and allow this case to be treated as a "learned nothing"
> case (e.g., D4 / H4, as commented in the code).
> 
> This patch does the same thing that 223a23c198787 did for gluster.c,
> except in file-posix.c
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>

Thanks, applied to the block branch.

Kevin