cryptospore / rpms / qemu-kvm

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