From 3befd56ec7363ca2ee6d2e791aa20a625c70c90a Mon Sep 17 00:00:00 2001
From: Igor Fedotov <ifedotov@suse.com>
Date: Thu, 28 Jun 2018 17:05:59 +0300
Subject: [PATCH] os/bluestore_tool: fix multiple extents handling in
 BlueFS::log_dump

Without the fix the op crashed when trying to read from the second
extent of the log file which was absent in superblock but actually existed.
Regular replay properly retrieves it - which wasn't the case
for log_dump due to 'noop' mode of operation.

Signed-off-by: Igor Fedotov <ifedotov@suse.com>
---
 src/os/bluestore/BlueFS.cc         | 24 +++++++++---------------
 src/os/bluestore/BlueFS.h          |  5 +----
 src/os/bluestore/bluestore_tool.cc | 30 +++++++++++++-----------------
 3 files changed, 23 insertions(+), 36 deletions(-)

diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index 29e6217a5cc..77b2c5c0768 100644
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -570,12 +570,15 @@ int BlueFS::_replay(bool noop, bool to_stdout)
   log_seq = 0;
 
   FileRef log_file;
-  if (noop) {
-    log_file = new File;
+  log_file = _get_file(1);
+  if (!noop) {
+    log_file->fnode = super.log_fnode;
   } else {
-    log_file = _get_file(1);
+    // do not use fnode from superblock in 'noop' mode - log_file's one should
+    // be fine and up-to-date
+    assert(log_file->fnode.ino == 1);
+    assert(log_file->fnode.extents.size() != 0);
   }
-  log_file->fnode = super.log_fnode;
   dout(10) << __func__ << " log_fnode " << super.log_fnode << dendl;
   if (unlikely(to_stdout)) {
     std::cout << " log_fnode " << super.log_fnode << std::endl;
@@ -950,19 +953,10 @@ int BlueFS::_replay(bool noop, bool to_stdout)
   return 0;
 }
 
-int BlueFS::log_dump(
-  CephContext *cct,
-  const string& path,
-  const vector<string>& devs)
+int BlueFS::log_dump()
 {
-  int r = _open_super();
-  if (r < 0) {
-    derr << __func__ << " failed to open super: " << cpp_strerror(r) << dendl;
-    return r;
-  }
-
   // only dump log file's content
-  r = _replay(true, true);
+  int r = _replay(true, true);
   if (r < 0) {
     derr << __func__ << " failed to replay log: " << cpp_strerror(r) << dendl;
     return r;
diff --git a/src/os/bluestore/BlueFS.h b/src/os/bluestore/BlueFS.h
index 5e147a8cc5c..4abacb672b8 100644
--- a/src/os/bluestore/BlueFS.h
+++ b/src/os/bluestore/BlueFS.h
@@ -340,10 +340,7 @@ public:
   int mount();
   void umount();
   
-  int log_dump(
-    CephContext *cct,
-    const string& path,
-    const vector<string>& devs);
+  int log_dump();
 
   void collect_metadata(map<string,string> *pm, unsigned skip_bdev_id);
   void get_devices(set<string> *ls);
diff --git a/src/os/bluestore/bluestore_tool.cc b/src/os/bluestore/bluestore_tool.cc
index b756812ed23..ccdc0e403fe 100644
--- a/src/os/bluestore/bluestore_tool.cc
+++ b/src/os/bluestore/bluestore_tool.cc
@@ -134,43 +134,39 @@ void add_devices(
   }
 }
 
-void log_dump(
+BlueFS *open_bluefs(
   CephContext *cct,
   const string& path,
   const vector<string>& devs)
 {
   validate_path(cct, path, true);
   BlueFS *fs = new BlueFS(cct);
-  
+
   add_devices(fs, cct, devs);
 
-  int r = fs->log_dump(cct, path, devs);
+  int r = fs->mount();
   if (r < 0) {
-    cerr << "log_dump failed" << ": "
-         << cpp_strerror(r) << std::endl;
+    cerr << "unable to mount bluefs: " << cpp_strerror(r)
+	 << std::endl;
     exit(EXIT_FAILURE);
   }
-
-  delete fs;
+  return fs;
 }
 
-BlueFS *open_bluefs(
+void log_dump(
   CephContext *cct,
   const string& path,
   const vector<string>& devs)
 {
-  validate_path(cct, path, true);
-  BlueFS *fs = new BlueFS(cct);
-
-  add_devices(fs, cct, devs);
-
-  int r = fs->mount();
+  BlueFS* fs = open_bluefs(cct, path, devs);
+  int r = fs->log_dump();
   if (r < 0) {
-    cerr << "unable to mount bluefs: " << cpp_strerror(r)
-	 << std::endl;
+    cerr << "log_dump failed" << ": "
+         << cpp_strerror(r) << std::endl;
     exit(EXIT_FAILURE);
   }
-  return fs;
+
+  delete fs;
 }
 
 void inferring_bluefs_devices(vector<string>& devs, std::string& path)
-- 
2.47.3