From: Igor Fedotov <ifedotov@suse.com>
Date: Thu, 28 Jun 2018 14:05:59 +0000 (+0300)
Subject: os/bluestore_tool: fix multiple extents handling in BlueFS::log_dump
X-Git-Tag: v14.0.1~598^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=3befd56ec7363ca2ee6d2e791aa20a625c70c90a;p=ceph.git

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>
---

diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index 29e6217a5cc8..77b2c5c07681 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 5e147a8cc5c0..4abacb672b83 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 b756812ed230..ccdc0e403fef 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)