Merge pull request ceph#60731 from joscollin/wip-B68954-check-headers-journal-recovery

cephfs-journal-tool: check the headers in dump file after journal recovery

Reviewed-by: Dhairya Parmar <[email protected]>
Reviewed-by: Rishabh Dave <[email protected]>

Author: rishabh-d-dave

qa/tasks/cephfs/test_journal_repair.py

@@ -450,3 +450,28 @@ def test_journal_import_from_invalid_dump_file(self):
             raise RuntimeError("Expected journal import to fail")
         finally:
             self.mount_a.run_shell(["sudo", "rm", fname], omit_sudo=False)
+
+    def test_header_check_after_journal_recovery(self):
+        """
+        That the 'journal import' recognizes invalid headers post journal recovery and errors out.
+        """
+        # Create an invalid dump file which doesn't have 'object_size' header
+        fname = tempfile.NamedTemporaryFile(delete=False).name
+        self.mount_a.run_shell(["sudo", "sh", "-c", f'printf "Ceph mds0 journal dump\n\
+        start offset 4194304 (0x400000)\nlength 940 (0x3ac)\nwrite_pos 4194304 (0x400000)\n\
+        format 1\ntrimmed_pos 4194304 (0x400000)\nstripe_unit 4194304 (0x400000)\nstripe_count 1 (0x1)\n\
+        fsid 41334b86-2666-4269-a8ea-313bc073564c\n" > {fname}'], omit_sudo=False)
+        self.fs.fail()
+        try:
+            self.fs.journal_tool(["journal", "import", fname, "--force"], 0)
+        except CommandFailedError as e:
+            self.fs.set_joinable()
+            self.fs.wait_for_daemons()
+            if e.exitstatus != 234:
+                raise RuntimeError(f"Unexpected journal import error: {str(e)}")
+        else:
+            self.fs.set_joinable()
+            self.fs.wait_for_daemons()
+            raise RuntimeError("Expected journal import to fail")
+        finally:
+            self.mount_a.run_shell(["sudo", "rm", fname], omit_sudo=False)

src/tools/cephfs/Dumper.cc

@@ -293,24 +293,41 @@ int Dumper::undump(const char *dump_file, bool force)
   }
 
   if (recovered == 0) {
+    //Check if the headers are available in the dump file
+    if (!strstr(buf, "stripe_unit")) {
+      derr  << "Invalid header, no 'stripe_unit' embedded" << dendl;
+      ::close(fd);
+      return -EINVAL;
+    } else if (!strstr(buf, "stripe_count")) {
+      derr  << "Invalid header, no 'stripe_count' embedded" << dendl;
+      ::close(fd);
+      return -EINVAL;
+    } else if (!strstr(buf, "object_size")) {
+      derr  << "Invalid header, no 'object_size' embedded" << dendl;
+      ::close(fd);
+      return -EINVAL;
+    }
     auto&& last_committed = journaler.get_last_committed();
     stripe_unit = last_committed.layout.stripe_unit;
     stripe_count = last_committed.layout.stripe_count;
     object_size = last_committed.layout.object_size;
   } else {
     // try to get layout from dump file header, if failed set layout to default
-    if (strstr(buf, "stripe_unit")) {
-      sscanf(strstr(buf, "stripe_unit"), "stripe_unit %lu", &stripe_unit);
+    char *p_stripe_unit = strstr(buf, "stripe_unit");
+    if (p_stripe_unit) {
+      sscanf(p_stripe_unit, "stripe_unit %lu", &stripe_unit);
     } else {
       stripe_unit = file_layout_t::get_default().stripe_unit;
     }
-    if (strstr(buf, "stripe_count")) {
-      sscanf(strstr(buf, "stripe_count"), "stripe_count %lu", &stripe_count);
+    char *p_stripe_count = strstr(buf, "stripe_count");
+    if (p_stripe_count) {
+      sscanf(p_stripe_count, "stripe_count %lu", &stripe_count);
     } else {
       stripe_count = file_layout_t::get_default().stripe_count;
     }
-    if (strstr(buf, "object_size")) {
-      sscanf(strstr(buf, "object_size"), "object_size %lu", &object_size);
+    char *p_object_size = strstr(buf, "object_size");
+    if (p_object_size) {
+      sscanf(p_object_size, "object_size %lu", &object_size);
     } else {
       object_size = file_layout_t::get_default().object_size;
     }