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 29e6217a5cc8eaf8569bfac44269fb06cf791beb..77b2c5c07681de82e9ff0adc1b58e4e34165ef6d 100644 (file)
--- 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 5e147a8cc5c05ea943e61d13faa2a2326c31e101..4abacb672b8308a2128f14942f3fe46f9b8ff5be 100644 (file)
--- 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 b756812ed230de9412ea5e157a5c63f8e8e29054..ccdc0e403fef2d201a3c1c6b5f18bccc57809ec1 100644 (file)
--- 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)