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

Jeff Cody posted 1 patch 5 years, 12 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/f7011b6ebc323b45b46a8f01310249e2e6d77617.1522730165.git.jcody@redhat.com
Test checkpatch passed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test s390x passed
block/file-posix.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH for-2.12] block: handle invalid lseek returns gracefully
Posted by Jeff Cody 5 years, 12 months ago
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
Posted by Eric Blake 5 years, 11 months ago
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

Re: [Qemu-devel] [PATCH for-2.12] block: handle invalid lseek returns gracefully
Posted by Jeff Cody 5 years, 11 months ago
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
Posted by Kevin Wolf 5 years, 11 months ago
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