crimson/os/seastore: ensure data integrity with deep copy in omap_get_value

Previously, omap_get_value could return a bufferlist pointing to
memory without guaranteed lifetime. This patch introduces LogNode::copy_t
to distinguish between DEEP and SHALLOW copies.

- Default get_value to DEEP copy for external safety.
- Use SHALLOW copy in internal paths (e.g., remove_kv) to maintain performance.
- Refactor LogManager::omap_get_value to simplify coroutine flow.

Signed-off-by: Myoungwon Oh <ohmyoungwon@gmail.com>

diff --git a/src/crimson/os/seastore/omap_manager/log/log_manager.cc b/src/crimson/os/seastore/omap_manager/log/log_manager.cc
index 58c87054ec542bfc96ccabe7faafcedd1a5358c0..430fc250d1847ffebef3c897d8d803231428570b 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/log/log_manager.cc
+++ b/src/crimson/os/seastore/omap_manager/log/log_manager.cc
@@ -310,7 +310,7 @@ LogManager::_log_set_key(omap_root_t &log_root,
   // This means the first entry of the new LogNode is not _fastinfo
   if (!is_ow_key(key) && can_ow) {
     // remove _fastinfo in old LogNode
-    auto e = co_await tail->get_value(key);
+    auto e = co_await tail->get_value(key, LogNode::copy_t::SHALLOW);
     if (e != std::nullopt) {
       auto mut = tm.get_mutable_extent(t, tail)->template cast<LogNode>();
       mut->remove_entry(get_ow_key());
@@ -396,14 +396,11 @@ LogManager::omap_get_value(
   LOG_PREFIX(LogManager::omap_get_value);
   DEBUGT("key={}", t, key);
   assert(log_root.get_type() == omap_type_t::LOG);
-  std::optional<bufferlist> ret;
   if (!is_dup_log_key(key)) {
-    ret = co_await find_kv(t, log_root.addr, key);
-  } else {
-    ret = co_await find_kv(t, 
-      co_await get_dup_addr_from_root(t, log_root.addr), key);
+    co_return co_await find_kv(t, log_root.addr, key);
   }
-  co_return ret;
+  co_return co_await find_kv(t, 
+    co_await get_dup_addr_from_root(t, log_root.addr), key);
 }
 
 LogManager::omap_list_ret
@@ -586,7 +583,7 @@ LogManager::remove_kv(Transaction &t, laddr_t dst, const std::string &key, LogNo
     co_return;
   }
 
-  auto e = co_await extent->get_value(key);
+  auto e = co_await extent->get_value(key, LogNode::copy_t::SHALLOW);
   if (e == std::nullopt) {
     if(extent->get_prev_addr() == L_ADDR_NULL) {
       co_return;

diff --git a/src/crimson/os/seastore/omap_manager/log/log_node.cc b/src/crimson/os/seastore/omap_manager/log/log_node.cc
index 4e6c75dfe1e9799949ef091290a3d8fed3a2f9fd..c8c01b7af9a8a4512f5bc16fad6aaa20adecd784 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/log/log_node.cc
+++ b/src/crimson/os/seastore/omap_manager/log/log_node.cc
@@ -203,14 +203,18 @@ void LogNode::list(const std::optional<std::string> &first,
   });
 }
 
-LogNode::get_value_ret LogNode::get_value(const std::string &key)
+LogNode::get_value_ret LogNode::get_value(const std::string &key, copy_t c)
 {
   bufferlist bl;
   bool found = false;
   for_each_live_entry([&](const auto& ent, uint32_t index) -> bool {
     const auto k = ent.get_key();
     if (k == key) {
-      bl = ent.get_val();
+      if (c == copy_t::SHALLOW) {
+       bl = ent.get_val_shallow();
+      } else {
+       bl = ent.get_val();
+      }
       found = true;
       /* If key is time-series log,
        * duplicate does not exist. In this case, return latest one */

diff --git a/src/crimson/os/seastore/omap_manager/log/log_node.h b/src/crimson/os/seastore/omap_manager/log/log_node.h
index b265707bbd0341a2bfa2d4b57de44ac1ad9f0cd8..5ce65be0c519b33f8ffb49bb1636fda4179070c9 100644 (file)
--- a/src/crimson/os/seastore/omap_manager/log/log_node.h
+++ b/src/crimson/os/seastore/omap_manager/log/log_node.h
@@ -459,11 +459,19 @@ public:
     }
 
     ceph::bufferlist get_val() const {
+      auto node_key = get_node_key();
+      ceph::bufferlist bl;
+      bl.append(get_node_val_ptr() + node_key.key_len,
+       node_key.val_len);
+      return bl;
+    }
+
+    ceph::bufferlist get_val_shallow() const {
       auto node_key = get_node_key();
       ceph::bufferlist bl;
       ceph::bufferptr bptr(
        get_node_val_ptr() + node_key.key_len,
-       get_node_key().val_len);
+       node_key.val_len);
       bl.append(bptr);
       return bl;
     }
@@ -822,8 +830,12 @@ struct LogNode
 
   void set_init_vars();
 
+  enum class copy_t : uint8_t {
+    SHALLOW,
+    DEEP,
+  };
   using get_value_ret = OMapManager::omap_get_value_ret;
-  get_value_ret get_value(const std::string &key);
+  get_value_ret get_value(const std::string &key, copy_t c = copy_t::DEEP);
 
   void set_dup_tail_addr(laddr_t laddr);