tools/cephfs: make journal pointer/header failure explicit

iIf the journal pointer and/or header is missing or corrupt, the tool silently
returns with no-op. There is no dentry recovery happening if any of the objects
are missing but it should be explicit to the user.

This still doesn't change returning errnos on failures to maintain status quo
until https://tracker.ceph.com/issues/77913 is addressed

One important change: The condition to scan events is now changed from
header_present to header_valid. The reason for this can be understood by
reading JournalScanner::scan_header -- if the header could not be decoded, it
is set to null, using this in JournalTool::recover_dentries would segfault and
other corruptions include bad magic or inconsistent offsets. A recovery should
not proceed with distorted magic/offset.

Fixes: https://tracker.ceph.com/issues/76422
Signed-off-by: Dhairya Parmar <dparmar@redhat.com>

diff --git a/src/tools/cephfs/JournalScanner.cc b/src/tools/cephfs/JournalScanner.cc
index 03d37b779186f7197f69955fa700e5347321cea0..8e011c82b3542009c232ed2c15c81b0885e27b3a 100644 (file)
--- a/src/tools/cephfs/JournalScanner.cc
+++ b/src/tools/cephfs/JournalScanner.cc
@@ -40,17 +40,26 @@ int JournalScanner::scan(bool const full, EventCallback cb)
     return r;
   }
 
-  if (!is_mdlog || pointer_present) {
+  if (is_mdlog && !pointer_present) {
+    derr << "Aborting journal scan" << dendl;
+    return 0;
+  } else {
     r = scan_header();
     if (r < 0) {
       return r;
     }
   }
 
-  if (full && header_present) {
-    r = scan_events(cb);
-    if (r < 0) {
-      return r;
+  // NOTE:: ensure the caller checks for header validity before scanning events
+  if (full) {
+    if (!header_valid) {
+      derr << "Aborting journal scan" << dendl;
+      return 0;
+    } else {
+      r = scan_events(cb);
+      if (r < 0) {
+        return r;
+      }
     }
   }
 

diff --git a/src/tools/cephfs/JournalTool.cc b/src/tools/cephfs/JournalTool.cc
index 20e14f2b159af1dd3260a3eab0fefa61c6e21256..9711adf3e0baeda29eea407098d1e600d675ef25 100644 (file)
--- a/src/tools/cephfs/JournalTool.cc
+++ b/src/tools/cephfs/JournalTool.cc
@@ -500,15 +500,20 @@ int JournalTool::main_event(std::vector<const char*> &argv)
     std::set<inodeno_t> consumed_inos;
     progress_tracker->set_operation_name("Processing events");
     
-    // decode the journal first to check if the header is valid
+    // decode the journal first to check if the journal pointer and header are valid
     r = js.scan(false);
     if (r < 0) {
       derr << "Failed to scan journal (" << cpp_strerror(r) << ")" << dendl;
       return r;
     }
-    if (!js.header_present || !js.header_valid) {
-      derr << "Journal header is not present or is damaged, cannot recover dentries" << dendl;
-      return -EIO;
+    if (!js.pointer_present) {
+      derr << "Cannot recover dentries: journal pointer is missing" << dendl;
+      return 0;
+    }
+    // if header_valid is false, header_present is also false
+    if (!js.header_valid) {
+      derr << "Cannot recover dentries: journal header is missing or invalid" << dendl;
+      return 0;
     }
 
     // start the progress tracker with the diff b/w write and expire position