958e1b
From 726f113a3fbea10e910a2962fa25b6274314380c Mon Sep 17 00:00:00 2001
09b1b5
From: Max Reitz <mreitz@redhat.com>
09b1b5
Date: Tue, 18 Nov 2014 15:30:19 +0100
958e1b
Subject: [PATCH 40/41] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP
09b1b5
09b1b5
Message-id: <1416324620-16229-7-git-send-email-mreitz@redhat.com>
09b1b5
Patchwork-id: 62441
09b1b5
O-Subject: [RHEL-7.1/7.0.z qemu-kvm PATCH v3 6/7] raw-posix: SEEK_HOLE suffices, get rid of FIEMAP
958e1b
Bugzilla: 1160237
09b1b5
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
09b1b5
RH-Acked-by: Kevin Wolf <kwolf@redhat.com>
09b1b5
RH-Acked-by: Markus Armbruster <armbru@redhat.com>
09b1b5
09b1b5
From: Markus Armbruster <armbru@redhat.com>
09b1b5
09b1b5
Commit 5500316 (May 2012) implemented raw_co_is_allocated() as
09b1b5
follows:
09b1b5
09b1b5
1. If defined(CONFIG_FIEMAP), use the FS_IOC_FIEMAP ioctl
09b1b5
09b1b5
2. Else if defined(SEEK_HOLE) && defined(SEEK_DATA), use lseek()
09b1b5
09b1b5
3. Else pretend there are no holes
09b1b5
09b1b5
Later on, raw_co_is_allocated() was generalized to
09b1b5
raw_co_get_block_status().
09b1b5
09b1b5
Commit 4f11aa8 (May 2014) changed it to try the three methods in order
09b1b5
until success, because "there may be implementations which support
09b1b5
[SEEK_HOLE/SEEK_DATA] but not [FIEMAP] (e.g., NFSv4.2) as well as vice
09b1b5
versa."
09b1b5
09b1b5
Unfortunately, we used FIEMAP incorrectly: we lacked FIEMAP_FLAG_SYNC.
09b1b5
Commit 38c4d0a (Sep 2014) added it.  Because that's a significant
09b1b5
speed hit, the next commit 7c159037 put SEEK_HOLE/SEEK_DATA first.
09b1b5
09b1b5
As you see, the obvious use of FIEMAP is wrong, and the correct use is
09b1b5
slow.  I guess this puts it somewhere between -7 "The obvious use is
09b1b5
wrong" and -10 "It's impossible to get right" on Rusty Russel's Hard
09b1b5
to Misuse scale[*].
09b1b5
09b1b5
"Fortunately", the FIEMAP code is used only when
09b1b5
09b1b5
* SEEK_HOLE/SEEK_DATA aren't defined, but CONFIG_FIEMAP is
09b1b5
09b1b5
  Uncommon.  SEEK_HOLE had no XFS implementation between 2011 (when it
09b1b5
  was introduced for ext4 and btrfs) and 2012.
09b1b5
09b1b5
* SEEK_HOLE/SEEK_DATA and CONFIG_FIEMAP are defined, but lseek() fails
09b1b5
09b1b5
  Unlikely.
09b1b5
09b1b5
Thus, the FIEMAP code executes rarely.  Makes it a nice hidey-hole for
09b1b5
bugs.  Worse, bugs hiding there can theoretically bite even on a host
09b1b5
that has SEEK_HOLE/SEEK_DATA.
09b1b5
09b1b5
I don't want to worry about this crap, not even theoretically.  Get
09b1b5
rid of it.
09b1b5
09b1b5
[*] http://ozlabs.org/~rusty/index.cgi/tech/2008-04-01.html
09b1b5
09b1b5
Signed-off-by: Markus Armbruster <armbru@redhat.com>
09b1b5
Reviewed-by: Max Reitz <mreitz@redhat.com>
09b1b5
Reviewed-by: Eric Blake <eblake@redhat.com>
09b1b5
Signed-off-by: Max Reitz <mreitz@redhat.com>
09b1b5
(cherry picked from commit c4875e5b2216cf5427459e619b10f75083565792)
09b1b5
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
09b1b5
09b1b5
Conflicts:
09b1b5
	block/raw-posix.c
09b1b5
09b1b5
Upstream has a "needs_alignment" field in BDRVRawState, downstream does
09b1b5
not.
09b1b5
09b1b5
Signed-off-by: Max Reitz <mreitz@redhat.com>
09b1b5
---
09b1b5
 block/raw-posix.c | 63 ++++---------------------------------------------------
09b1b5
 1 file changed, 4 insertions(+), 59 deletions(-)
09b1b5
09b1b5
diff --git a/block/raw-posix.c b/block/raw-posix.c
958e1b
index a46e50b..aeb8a97 100644
09b1b5
--- a/block/raw-posix.c
09b1b5
+++ b/block/raw-posix.c
958e1b
@@ -57,9 +57,6 @@
09b1b5
 #include <linux fd.h="">
09b1b5
 #include <linux fs.h="">
09b1b5
 #endif
09b1b5
-#ifdef CONFIG_FIEMAP
09b1b5
-#include <linux fiemap.h="">
09b1b5
-#endif
09b1b5
 #ifdef CONFIG_FALLOCATE_PUNCH_HOLE
09b1b5
 #include <linux falloc.h="">
09b1b5
 #endif
958e1b
@@ -147,9 +144,6 @@ typedef struct BDRVRawState {
09b1b5
     bool has_discard:1;
09b1b5
     bool has_write_zeroes:1;
09b1b5
     bool discard_zeroes:1;
09b1b5
-#ifdef CONFIG_FIEMAP
09b1b5
-    bool skip_fiemap;
09b1b5
-#endif
09b1b5
 } BDRVRawState;
09b1b5
 
09b1b5
 typedef struct BDRVRawReopenState {
958e1b
@@ -1308,52 +1302,6 @@ out:
09b1b5
     return result;
09b1b5
 }
09b1b5
 
09b1b5
-static int try_fiemap(BlockDriverState *bs, off_t start, off_t *data,
09b1b5
-                      off_t *hole, int nb_sectors)
09b1b5
-{
09b1b5
-#ifdef CONFIG_FIEMAP
09b1b5
-    BDRVRawState *s = bs->opaque;
09b1b5
-    int ret = 0;
09b1b5
-    struct {
09b1b5
-        struct fiemap fm;
09b1b5
-        struct fiemap_extent fe;
09b1b5
-    } f;
09b1b5
-
09b1b5
-    if (s->skip_fiemap) {
09b1b5
-        return -ENOTSUP;
09b1b5
-    }
09b1b5
-
09b1b5
-    f.fm.fm_start = start;
09b1b5
-    f.fm.fm_length = (int64_t)nb_sectors * BDRV_SECTOR_SIZE;
09b1b5
-    f.fm.fm_flags = FIEMAP_FLAG_SYNC;
09b1b5
-    f.fm.fm_extent_count = 1;
09b1b5
-    f.fm.fm_reserved = 0;
09b1b5
-    if (ioctl(s->fd, FS_IOC_FIEMAP, &f) == -1) {
09b1b5
-        s->skip_fiemap = true;
09b1b5
-        return -errno;
09b1b5
-    }
09b1b5
-
09b1b5
-    if (f.fm.fm_mapped_extents == 0) {
09b1b5
-        /* No extents found, data is beyond f.fm.fm_start + f.fm.fm_length.
09b1b5
-         * f.fm.fm_start + f.fm.fm_length must be clamped to the file size!
09b1b5
-         */
09b1b5
-        off_t length = lseek(s->fd, 0, SEEK_END);
09b1b5
-        *hole = f.fm.fm_start;
09b1b5
-        *data = MIN(f.fm.fm_start + f.fm.fm_length, length);
09b1b5
-    } else {
09b1b5
-        *data = f.fe.fe_logical;
09b1b5
-        *hole = f.fe.fe_logical + f.fe.fe_length;
09b1b5
-        if (f.fe.fe_flags & FIEMAP_EXTENT_UNWRITTEN) {
09b1b5
-            ret |= BDRV_BLOCK_ZERO;
09b1b5
-        }
09b1b5
-    }
09b1b5
-
09b1b5
-    return ret;
09b1b5
-#else
09b1b5
-    return -ENOTSUP;
09b1b5
-#endif
09b1b5
-}
09b1b5
-
09b1b5
 static int try_seek_hole(BlockDriverState *bs, off_t start, off_t *data,
09b1b5
                          off_t *hole)
09b1b5
 {
958e1b
@@ -1422,13 +1370,10 @@ static int64_t coroutine_fn raw_co_get_block_status(BlockDriverState *bs,
09b1b5
 
09b1b5
     ret = try_seek_hole(bs, start, &data, &hole);
09b1b5
     if (ret < 0) {
09b1b5
-        ret = try_fiemap(bs, start, &data, &hole, nb_sectors);
09b1b5
-        if (ret < 0) {
09b1b5
-            /* Assume everything is allocated. */
09b1b5
-            data = 0;
09b1b5
-            hole = start + nb_sectors * BDRV_SECTOR_SIZE;
09b1b5
-            ret = 0;
09b1b5
-        }
09b1b5
+        /* Assume everything is allocated. */
09b1b5
+        data = 0;
09b1b5
+        hole = start + nb_sectors * BDRV_SECTOR_SIZE;
09b1b5
+        ret = 0;
09b1b5
     }
09b1b5
 
09b1b5
     assert(ret >= 0);
09b1b5
-- 
09b1b5
1.8.3.1
09b1b5