crimson/onode-staged-tree: validate node header when load

Add logs to detect corruptions when load nodes. assert() is not
informative enough to understand the context.

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
index 859e82e91689f75f13d374059edf6760136e2bb8..6eea71f4c08642a6f501d6a8e92d236e78179fa2 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node.cc
@@ -684,21 +684,40 @@ eagain_future<Ref<Node>> Node::load(
              c.t, addr, expect_is_level_tail);
       ceph_abort("fatal error");
     })
-  ).safe_then([FNAME, c, expect_is_level_tail](auto extent) {
-    auto [node_type, field_type] = extent->get_types();
+  ).safe_then([FNAME, c, addr, expect_is_level_tail](auto extent) {
+    auto header = extent->get_header();
+    auto field_type = header.get_field_type();
+    if (!field_type) {
+      ERRORT("load addr={:x}, is_level_tail={} error, "
+             "got invalid header -- {}",
+             c.t, addr, expect_is_level_tail, extent);
+      ceph_abort("fatal error");
+    }
+    if (header.get_is_level_tail() != expect_is_level_tail) {
+      ERRORT("load addr={:x}, is_level_tail={} error, "
+             "is_level_tail mismatch -- {}",
+             c.t, addr, expect_is_level_tail, extent);
+      ceph_abort("fatal error");
+    }
+
+    auto node_type = header.get_node_type();
     if (node_type == node_type_t::LEAF) {
       if (extent->get_length() != c.vb.get_leaf_node_size()) {
-        ERRORT("leaf length mismatch -- {}", c.t, extent);
+        ERRORT("load addr={:x}, is_level_tail={} error, "
+               "leaf length mismatch -- {}",
+               c.t, addr, expect_is_level_tail, extent);
         ceph_abort("fatal error");
       }
-      auto impl = LeafNodeImpl::load(extent, field_type, expect_is_level_tail);
+      auto impl = LeafNodeImpl::load(extent, *field_type);
       return Ref<Node>(new LeafNode(impl.get(), std::move(impl)));
     } else if (node_type == node_type_t::INTERNAL) {
       if (extent->get_length() != c.vb.get_internal_node_size()) {
-        ERRORT("internal length mismatch -- {}", c.t, extent);
+        ERRORT("load addr={:x}, is_level_tail={} error, "
+               "internal length mismatch -- {}",
+               c.t, addr, expect_is_level_tail, extent);
         ceph_abort("fatal error");
       }
-      auto impl = InternalNodeImpl::load(extent, field_type, expect_is_level_tail);
+      auto impl = InternalNodeImpl::load(extent, *field_type);
       return Ref<Node>(new InternalNode(impl.get(), std::move(impl)));
     } else {
       ceph_abort("impossible path");
@@ -1518,6 +1537,11 @@ eagain_future<Ref<Node>> InternalNode::get_or_track_child(
     ).safe_then([this, position, c, FNAME] (auto child) {
       TRACET("loaded child untracked {}",
              c.t, child->get_name());
+      if (child->level() + 1 != level()) {
+        ERRORT("loaded child {} error from parent {} at pos({}), level mismatch",
+               c.t, child->get_name(), get_name(), position);
+        ceph_abort("fatal error");
+      }
       child->as_child(position, this);
       return child;
     });

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.cc
index e0c83bf524e550f03534e22feec4ff35b2664241..e8f6fbe5e54ac2d87a33fb0949780c770e8f62b1 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.cc
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.cc
@@ -5,21 +5,9 @@
 
 #include "node_extent_manager/dummy.h"
 #include "node_extent_manager/seastore.h"
-#include "stages/node_stage_layout.h"
 
 namespace crimson::os::seastore::onode {
 
-std::pair<node_type_t, field_type_t> NodeExtent::get_types() const
-{
-  const auto header = reinterpret_cast<const node_header_t*>(get_read());
-  auto node_type = header->get_node_type();
-  auto field_type = header->get_field_type();
-  if (!field_type.has_value()) {
-    throw std::runtime_error("load failed: bad field type");
-  }
-  return {node_type, *field_type};
-}
-
 NodeExtentManagerURef NodeExtentManager::create_dummy(bool is_sync)
 {
   if (is_sync) {

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h
index ac3b7179cf8b52958be55f33a80dc089d3a81ebc..924d6ee8770a14af7b9f2d5b73daa7dd9094401c 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h
@@ -8,9 +8,10 @@
 #include "crimson/os/seastore/transaction_manager.h"
 
 #include "fwd.h"
-#include "super.h"
 #include "node_extent_mutable.h"
 #include "node_types.h"
+#include "stages/node_stage_layout.h"
+#include "super.h"
 
 /**
  * node_extent_manager.h
@@ -24,7 +25,9 @@ using crimson::os::seastore::LogicalCachedExtent;
 class NodeExtent : public LogicalCachedExtent {
  public:
   virtual ~NodeExtent() = default;
-  std::pair<node_type_t, field_type_t> get_types() const;
+  const node_header_t& get_header() const {
+    return *reinterpret_cast<const node_header_t*>(get_read());
+  }
   const char* get_read() const {
     return get_bptr().c_str();
   }

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.cc
index 528acb8f131579236896ca4ba56062d15fed4aef..ba6eb63d7cfdfe96ae90d6770e21d3f5c7dd8182 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.cc
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.cc
@@ -62,13 +62,19 @@ void SeastoreNodeExtent::apply_delta(const ceph::bufferlist& bl)
 {
   DEBUG("replay {:#x} ...", get_laddr());
   if (!recorder) {
-    auto [node_type, field_type] = get_types();
-    recorder = create_replay_recorder(node_type, field_type);
+    auto header = get_header();
+    auto field_type = header.get_field_type();
+    if (!field_type.has_value()) {
+      ERROR("replay got invalid node -- {}", *this);
+      ceph_abort("fatal error");
+    }
+    auto node_type = header.get_node_type();
+    recorder = create_replay_recorder(node_type, *field_type);
   } else {
 #ifndef NDEBUG
-    auto [node_type, field_type] = get_types();
-    assert(recorder->node_type() == node_type);
-    assert(recorder->field_type() == field_type);
+    auto header = get_header();
+    assert(recorder->node_type() == header.get_node_type());
+    assert(recorder->field_type() == *header.get_field_type());
 #endif
   }
   auto node = do_get_mutable();

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.cc
index e8fc618da4498c0ecc65f61f3b5f258396980a63..219dbb861374cb4a96f3c1531f67f2e13f6a133f 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.cc
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.cc
@@ -46,32 +46,32 @@ LeafNodeImpl::allocate(
 }
 
 InternalNodeImplURef InternalNodeImpl::load(
-    NodeExtentRef extent, field_type_t type, bool expect_is_level_tail)
+    NodeExtentRef extent, field_type_t type)
 {
   if (type == field_type_t::N0) {
-    return InternalNode0::load(extent, expect_is_level_tail);
+    return InternalNode0::load(extent);
   } else if (type == field_type_t::N1) {
-    return InternalNode1::load(extent, expect_is_level_tail);
+    return InternalNode1::load(extent);
   } else if (type == field_type_t::N2) {
-    return InternalNode2::load(extent, expect_is_level_tail);
+    return InternalNode2::load(extent);
   } else if (type == field_type_t::N3) {
-    return InternalNode3::load(extent, expect_is_level_tail);
+    return InternalNode3::load(extent);
   } else {
     ceph_abort("impossible path");
   }
 }
 
 LeafNodeImplURef LeafNodeImpl::load(
-    NodeExtentRef extent, field_type_t type, bool expect_is_level_tail)
+    NodeExtentRef extent, field_type_t type)
 {
   if (type == field_type_t::N0) {
-    return LeafNode0::load(extent, expect_is_level_tail);
+    return LeafNode0::load(extent);
   } else if (type == field_type_t::N1) {
-    return LeafNode1::load(extent, expect_is_level_tail);
+    return LeafNode1::load(extent);
   } else if (type == field_type_t::N2) {
-    return LeafNode2::load(extent, expect_is_level_tail);
+    return LeafNode2::load(extent);
   } else if (type == field_type_t::N3) {
-    return LeafNode3::load(extent, expect_is_level_tail);
+    return LeafNode3::load(extent);
   } else {
     ceph_abort("impossible path");
   }

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.h
index 9a0ad143e22bafeaa90e87faaa98218cc880e384..94655686faef9403e00560130b2a37c422e8f085 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_impl.h
@@ -181,7 +181,7 @@ class InternalNodeImpl : public NodeImpl {
   };
   static eagain_future<fresh_impl_t> allocate(context_t, field_type_t, bool, level_t);
 
-  static InternalNodeImplURef load(NodeExtentRef, field_type_t, bool);
+  static InternalNodeImplURef load(NodeExtentRef, field_type_t);
 
  protected:
   InternalNodeImpl() = default;
@@ -261,7 +261,7 @@ class LeafNodeImpl : public NodeImpl {
   };
   static eagain_future<fresh_impl_t> allocate(context_t, field_type_t, bool);
 
-  static LeafNodeImplURef load(NodeExtentRef, field_type_t, bool);
+  static LeafNodeImplURef load(NodeExtentRef, field_type_t);
 
  protected:
   LeafNodeImpl() = default;

diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_layout.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_layout.h
index fadb4a509b0cd203d494b079a215834f157263c3..d474e86954c162efcace63e6e1832a9c0251a896 100644 (file)
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_layout.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_layout.h
@@ -60,9 +60,8 @@ class NodeLayoutT final : public InternalNodeImpl, public LeafNodeImpl {
   NodeLayoutT& operator=(NodeLayoutT&&) = delete;
   ~NodeLayoutT() override = default;
 
-  static URef load(NodeExtentRef extent, bool expect_is_level_tail) {
+  static URef load(NodeExtentRef extent) {
     std::unique_ptr<NodeLayoutT> ret(new NodeLayoutT(extent));
-    assert(ret->is_level_tail() == expect_is_level_tail);
     return ret;
   }