crimson/os/seastore: hide the complexity of extent indirection

Make it easier for TM::read_pin() users to consume extent without
worrying about the indirections.

This basically reverts 9cdcd06caadef6cdca64a974c6525ba300c1e289

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

8 files changed, 162 insertions, 114 deletions

diff --git a/src/crimson/os/seastore/collection_manager/flat_collection_manager.cc b/src/crimson/os/seastore/collection_manager/flat_collection_manager.cc
index c32dc66619a..866b5bf350c 100644
--- a/src/crimson/os/seastore/collection_manager/flat_collection_manager.cc
+++ b/src/crimson/os/seastore/collection_manager/flat_collection_manager.cc
@@ -51,8 +51,11 @@ FlatCollectionManager::get_coll_root(const coll_root_t &coll_root, Transaction &
     cc.t,
     coll_root.get_location(),
     coll_root.get_size()
-  ).si_then([](auto&& e) {
-    return get_root_iertr::make_ready_future<CollectionNodeRef>(std::move(e));
+  ).si_then([](auto maybe_indirect_extent) {
+    assert(!maybe_indirect_extent.is_indirect());
+    assert(!maybe_indirect_extent.is_clone);
+    return get_root_iertr::make_ready_future<CollectionNodeRef>(
+        std::move(maybe_indirect_extent.extent));
   });
 }
 
diff --git a/src/crimson/os/seastore/object_data_handler.cc b/src/crimson/os/seastore/object_data_handler.cc
index 20f86da5d3d..83bb2c08af8 100644
--- a/src/crimson/os/seastore/object_data_handler.cc
+++ b/src/crimson/os/seastore/object_data_handler.cc
@@ -859,15 +859,14 @@ operate_ret operate_left(context_t ctx, LBAMappingRef &pin, const overwrite_plan
         std::nullopt,
         std::nullopt);
     } else {
-      extent_len_t off = pin->get_intermediate_offset();
       return ctx.tm.read_pin<ObjectDataBlock>(
 	ctx.t, pin->duplicate()
-      ).si_then([prepend_len, off](auto left_extent) {
+      ).si_then([prepend_len](auto maybe_indirect_left_extent) {
         return get_iertr::make_ready_future<operate_ret_bare>(
           std::nullopt,
           std::make_optional(bufferptr(
-            left_extent->get_bptr(),
-            off,
+            maybe_indirect_left_extent.get_bptr(),
+            0,
             prepend_len)));
       });
     }
@@ -888,16 +887,15 @@ operate_ret operate_left(context_t ctx, LBAMappingRef &pin, const overwrite_plan
         std::move(left_to_write_extent),
         std::nullopt);
     } else {
-      extent_len_t off = pin->get_intermediate_offset();
       return ctx.tm.read_pin<ObjectDataBlock>(
 	ctx.t, pin->duplicate()
-      ).si_then([prepend_offset=extent_len + off, prepend_len,
+      ).si_then([prepend_offset=extent_len, prepend_len,
                  left_to_write_extent=std::move(left_to_write_extent)]
-                (auto left_extent) mutable {
+                (auto left_maybe_indirect_extent) mutable {
         return get_iertr::make_ready_future<operate_ret_bare>(
           std::move(left_to_write_extent),
           std::make_optional(bufferptr(
-            left_extent->get_bptr(),
+            left_maybe_indirect_extent.get_bptr(),
             prepend_offset,
             prepend_len)));
       });
@@ -944,15 +942,15 @@ operate_ret operate_right(context_t ctx, LBAMappingRef &pin, const overwrite_pla
     } else {
       auto append_offset =
 	overwrite_plan.data_end.get_byte_distance<
-	  extent_len_t>(right_pin_begin)
-	  + pin->get_intermediate_offset();
+	  extent_len_t>(right_pin_begin);
       return ctx.tm.read_pin<ObjectDataBlock>(
 	ctx.t, pin->duplicate()
-      ).si_then([append_offset, append_len](auto right_extent) {
+      ).si_then([append_offset, append_len]
+                (auto right_maybe_indirect_extent) {
         return get_iertr::make_ready_future<operate_ret_bare>(
           std::nullopt,
           std::make_optional(bufferptr(
-            right_extent->get_bptr(),
+            right_maybe_indirect_extent.get_bptr(),
             append_offset,
             append_len)));
       });
@@ -976,17 +974,16 @@ operate_ret operate_right(context_t ctx, LBAMappingRef &pin, const overwrite_pla
     } else {
       auto append_offset =
 	overwrite_plan.data_end.get_byte_distance<
-	  extent_len_t>(right_pin_begin)
-	  + pin->get_intermediate_offset();
+	  extent_len_t>(right_pin_begin);
       return ctx.tm.read_pin<ObjectDataBlock>(
 	ctx.t, pin->duplicate()
       ).si_then([append_offset, append_len,
                  right_to_write_extent=std::move(right_to_write_extent)]
-                (auto right_extent) mutable {
+                (auto maybe_indirect_right_extent) mutable {
         return get_iertr::make_ready_future<operate_ret_bare>(
           std::move(right_to_write_extent),
           std::make_optional(bufferptr(
-            right_extent->get_bptr(),
+            maybe_indirect_right_extent.get_bptr(),
             append_offset,
             append_len)));
       });
@@ -1136,12 +1133,12 @@ ObjectDataHandler::clear_ret ObjectDataHandler::trim_data_reservation(
               ctx.t,
               pin.duplicate()
             ).si_then([ctx, size, pin_offset, append_len, roundup_size,
-                      &pin, &object_data, &to_write](auto extent) {
+                      &pin, &object_data, &to_write](auto maybe_indirect_extent) {
               bufferlist bl;
 	      bl.append(
 	        bufferptr(
-	          extent->get_bptr(),
-		  pin.get_intermediate_offset(),
+	          maybe_indirect_extent.get_bptr(),
+	          0,
 	          size - pin_offset
 	      ));
               bl.append_zero(append_len);
@@ -1501,83 +1498,54 @@ ObjectDataHandler::read_ret ObjectDataHandler::read(
             pins,
             [FNAME, ctx, l_start, l_end,
              &l_current, &ret](auto &pin) -> read_iertr::future<> {
-            auto pin_key = pin->get_key();
-            if (l_current == l_start) {
-              ceph_assert(l_current >= pin_key);
-            } else {
+            auto pin_start = pin->get_key();
+            if (l_current == l_start) { // first pin may skip head
+              ceph_assert(l_current.get_aligned_laddr() >= pin_start);
+            } else { // non-first pin must match start
               assert(l_current > l_start);
-              ceph_assert(l_current == pin_key);
+              ceph_assert(l_current == pin_start);
             }
+
             ceph_assert(l_current < l_end);
             auto pin_len = pin->get_length();
             assert(pin_len > 0);
-            laddr_offset_t l_pin_end = pin_key + pin_len;
-            ceph_assert(l_current < l_pin_end);
-            laddr_offset_t l_current_end = std::min(l_pin_end, l_end);
+            laddr_offset_t pin_end = pin_start + pin_len;
+            assert(l_current < pin_end);
+            laddr_offset_t l_current_end = std::min(pin_end, l_end);
+            extent_len_t read_len =
+              l_current_end.get_byte_distance<extent_len_t>(l_current);
+
             if (pin->get_val().is_zero()) {
               DEBUGT("got {}~{} from zero-pin {}~{}",
                 ctx.t,
                 l_current,
-                l_current_end.get_byte_distance<loffset_t>(l_current),
-                pin_key,
+                read_len,
+                pin_start,
                 pin_len);
-              ret.append_zero(
-		l_current_end.get_byte_distance<
-		  extent_len_t>(l_current));
+              ret.append_zero(read_len);
               l_current = l_current_end;
               return seastar::now();
             }
 
             // non-zero pin
-            bool is_indirect = pin->is_indirect();
-            laddr_t e_key;
-            extent_len_t e_len;
-            extent_len_t e_off;
-            if (is_indirect) {
-              e_key = pin->get_intermediate_base();
-              e_len = pin->get_intermediate_length();
-              e_off = pin->get_intermediate_offset();
-              DEBUGT("reading {}~{} from indirect-pin {}~{}, direct-pin {}~{}(off={})",
-                ctx.t,
-                l_current,
-		l_current_end.get_byte_distance<extent_len_t>(l_current),
-                pin_key,
-                pin_len,
-                e_key,
-                e_len,
-                e_off);
-              assert(e_key <= pin->get_intermediate_key());
-              assert(e_off + pin_len <= e_len);
-            } else {
-              DEBUGT("reading {}~{} from pin {}~{}",
-                ctx.t,
-                l_current,
-		l_current_end.get_byte_distance<
-		   extent_len_t>(l_current),
-                pin_key,
-                pin_len);
-              e_key = pin_key;
-              e_len = pin_len;
-              e_off = 0;
-            }
-            extent_len_t e_current_off = (l_current + e_off)
-		.template get_byte_distance<extent_len_t>(pin_key);
+            DEBUGT("reading {}~{} from pin {}~{}",
+              ctx.t,
+              l_current,
+              read_len,
+              pin_start,
+              pin_len);
+            extent_len_t e_current_off =
+              l_current.template get_byte_distance<extent_len_t>(pin_start);
             return ctx.tm.read_pin<ObjectDataBlock>(
               ctx.t,
               std::move(pin)
             ).si_then([&ret, &l_current, l_current_end,
-#ifndef NDEBUG
-                       e_key, e_len, e_current_off](auto extent) {
-#else
-                       e_current_off](auto extent) {
-#endif
-              assert(e_key == extent->get_laddr());
-              assert(e_len == extent->get_length());
+                       e_current_off, read_len](auto maybe_indirect_extent) {
               ret.append(
                 bufferptr(
-                  extent->get_bptr(),
+                  maybe_indirect_extent.get_bptr(),
                   e_current_off,
-                  l_current_end.get_byte_distance<extent_len_t>(l_current)));
+                  read_len));
               l_current = l_current_end;
               return seastar::now();
             }).handle_error_interruptible(
diff --git a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.cc b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.cc
index 8d06accef1e..df97f394a0d 100644
--- a/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.cc
+++ b/src/crimson/os/seastore/omap_manager/btree/omap_btree_node_impl.cc
@@ -734,23 +734,28 @@ omap_load_extent(omap_context_t oc, laddr_t laddr, depth_t depth)
 {
   ceph_assert(depth > 0);
   if (depth > 1) {
-    return oc.tm.read_extent<OMapInnerNode>(oc.t, laddr,
-      OMAP_INNER_BLOCK_SIZE)
-      .handle_error_interruptible(
+    return oc.tm.read_extent<OMapInnerNode>(
+        oc.t, laddr, OMAP_INNER_BLOCK_SIZE
+    ).handle_error_interruptible(
       omap_load_extent_iertr::pass_further{},
       crimson::ct_error::assert_all{ "Invalid error in omap_load_extent" }
-    ).si_then(
-      [](auto&& e) {
-      return seastar::make_ready_future<OMapNodeRef>(std::move(e));
+    ).si_then([](auto maybe_indirect_extent) {
+      assert(!maybe_indirect_extent.is_indirect());
+      assert(!maybe_indirect_extent.is_clone);
+      return seastar::make_ready_future<OMapNodeRef>(
+          std::move(maybe_indirect_extent.extent));
     });
   } else {
-    return oc.tm.read_extent<OMapLeafNode>(oc.t, laddr, OMAP_LEAF_BLOCK_SIZE
+    return oc.tm.read_extent<OMapLeafNode>(
+        oc.t, laddr, OMAP_LEAF_BLOCK_SIZE
     ).handle_error_interruptible(
       omap_load_extent_iertr::pass_further{},
       crimson::ct_error::assert_all{ "Invalid error in omap_load_extent" }
-    ).si_then(
-      [](auto&& e) {
-      return seastar::make_ready_future<OMapNodeRef>(std::move(e));
+    ).si_then([](auto maybe_indirect_extent) {
+      assert(!maybe_indirect_extent.is_indirect());
+      assert(!maybe_indirect_extent.is_clone);
+      return seastar::make_ready_future<OMapNodeRef>(
+          std::move(maybe_indirect_extent.extent));
     });
   }
 }
diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.h
index 9230051cc50..b5d674ffcaa 100644
--- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.h
+++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.h
@@ -111,10 +111,14 @@ class SeastoreNodeExtentManager final: public TransactionManagerHandle {
       }
     }
     return tm.read_extent<SeastoreNodeExtent>(t, addr
-    ).si_then([addr, &t](auto&& e) -> read_iertr::future<NodeExtentRef> {
+    ).si_then([addr, &t](auto maybe_indirect_extent)
+              -> read_iertr::future<NodeExtentRef> {
+      auto e = maybe_indirect_extent.extent;
       SUBTRACET(seastore_onode,
           "read {}B at {} -- {}",
           t, e->get_length(), e->get_laddr(), *e);
+      assert(!maybe_indirect_extent.is_indirect());
+      assert(!maybe_indirect_extent.is_clone);
       assert(e->get_laddr() == addr);
       std::ignore = addr;
       return read_iertr::make_ready_future<NodeExtentRef>(e);
diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index e81ce553336..b2e500d9d9c 100644
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -137,14 +137,46 @@ public:
   }
 
   /**
+   * maybe_indirect_extent_t
+   *
+   * Contains necessary information in case the extent is loaded from an
+   * indirect pin.
+   */
+  struct indirect_info_t {
+    extent_len_t intermediate_offset = 0;
+    extent_len_t length = 0;
+  };
+  template <typename T>
+  struct maybe_indirect_extent_t {
+    TCachedExtentRef<T> extent;
+    std::optional<indirect_info_t> maybe_indirect_info;
+    bool is_clone = false;
+
+    bool is_indirect() const {
+      return maybe_indirect_info.has_value();
+    }
+
+    ceph::bufferptr get_bptr() const {
+      if (is_indirect()) {
+        return ceph::bufferptr(
+            extent->get_bptr(),
+            maybe_indirect_info->intermediate_offset,
+            maybe_indirect_info->length);
+      } else {
+        return extent->get_bptr();
+      }
+    }
+  };
+
+  /**
    * read_extent
    *
    * Read extent of type T at offset~length
    */
   using read_extent_iertr = get_pin_iertr;
   template <typename T>
-  using read_extent_ret = read_extent_iertr::future<
-    TCachedExtentRef<T>>;
+  using read_extent_ret =
+    read_extent_iertr::future<maybe_indirect_extent_t<T>>;
   template <typename T>
   read_extent_ret<T> read_extent(
     Transaction &t,
@@ -192,10 +224,16 @@ public:
   }
 
   template <typename T>
-  base_iertr::future<TCachedExtentRef<T>> read_pin(
+  base_iertr::future<maybe_indirect_extent_t<T>> read_pin(
     Transaction &t,
     LBAMappingRef pin)
   {
+    bool is_clone = pin->is_clone();
+    std::optional<indirect_info_t> maybe_indirect_info;
+    if (pin->is_indirect()) {
+      maybe_indirect_info = indirect_info_t{
+        pin->get_intermediate_offset(), pin->get_length()};
+    }
     LOG_PREFIX(TransactionManager::read_pin);
     SUBDEBUGT(seastore_tm, "{} {} ...", t, T::TYPE, *pin);
     auto fut = base_iertr::make_ready_future<LBAMappingRef>();
@@ -223,9 +261,16 @@ public:
       } else {
 	return this->pin_to_extent<T>(t, std::move(std::get<0>(ret)));
       }
-    }).si_then([FNAME, &t](TCachedExtentRef<T> ext) {
-      SUBDEBUGT(seastore_tm, "got {}", t, *ext);
-      return ext;
+    }).si_then([FNAME, maybe_indirect_info, is_clone, &t](TCachedExtentRef<T> ext) {
+      if (maybe_indirect_info.has_value()) {
+        SUBDEBUGT(seastore_tm, "got indirect +0x{:x}~0x{:x} is_clone={} {}",
+                  t, maybe_indirect_info->intermediate_offset,
+                  maybe_indirect_info->length, is_clone, *ext);
+      } else {
+        SUBDEBUGT(seastore_tm, "got direct is_clone={} {}",
+                  t, is_clone, *ext);
+      }
+      return maybe_indirect_extent_t<T>{ext, maybe_indirect_info, is_clone};
     });
   }
 
@@ -355,7 +400,8 @@ public:
   }
 
   template <typename T>
-  read_extent_ret<T> get_mutable_extent_by_laddr(
+  get_pin_iertr::future<TCachedExtentRef<T>>
+  get_mutable_extent_by_laddr(
       Transaction &t,
       laddr_t laddr,
       extent_len_t len) {
@@ -367,8 +413,11 @@ public:
       ceph_assert(!pin->is_clone());
       ceph_assert(pin->get_length() == len);
       return this->read_pin<T>(t, std::move(pin));
-    }).si_then([this, &t, FNAME](auto extent) {
-      auto ext = get_mutable_extent(t, extent)->template cast<T>();
+    }).si_then([this, &t, FNAME](auto maybe_indirect_extent) {
+      assert(!maybe_indirect_extent.is_indirect());
+      assert(!maybe_indirect_extent.is_clone);
+      auto ext = get_mutable_extent(
+          t, maybe_indirect_extent.extent)->template cast<T>();
       SUBDEBUGT(seastore_tm, "got mutable {}", t, *ext);
       return read_extent_iertr::make_ready_future<TCachedExtentRef<T>>(
 	std::move(ext));
@@ -431,6 +480,7 @@ public:
       // The according extent might be stable or pending.
       auto fut = base_iertr::now();
       if (!pin->is_indirect()) {
+        ceph_assert(!pin->is_clone());
 	if (!pin->is_parent_viewable()) {
 	  if (pin->is_parent_valid()) {
 	    pin = pin->refresh_with_pending_parent();
@@ -451,7 +501,12 @@ public:
 
 	fut = fut.si_then([this, &t, &pin] {
 	  if (full_extent_integrity_check) {
-	    return read_pin<T>(t, pin->duplicate());
+	    return read_pin<T>(t, pin->duplicate()
+            ).si_then([](auto maybe_indirect_extent) {
+              assert(!maybe_indirect_extent.is_indirect());
+              assert(!maybe_indirect_extent.is_clone);
+              return maybe_indirect_extent.extent;
+            });
 	  } else {
 	    auto ret = get_extent_if_linked<T>(t, pin->duplicate());
 	    if (ret.index() == 1) {
@@ -685,8 +740,11 @@ public:
       t
     ).si_then([&t, this](auto root) {
       return read_extent<RootMetaBlock>(t, root->root.meta);
-    }).si_then([key, &t](auto mblock) {
+    }).si_then([key, &t](auto maybe_indirect_extent) {
       LOG_PREFIX(TransactionManager::read_root_meta);
+      assert(!maybe_indirect_extent.is_indirect());
+      assert(!maybe_indirect_extent.is_clone);
+      auto& mblock = maybe_indirect_extent.extent;
       auto meta = mblock->get_meta();
       auto iter = meta.find(key);
       if (iter == meta.end()) {
@@ -744,7 +802,10 @@ public:
       t
     ).si_then([this, &t](RootBlockRef root) {
       return read_extent<RootMetaBlock>(t, root->root.meta);
-    }).si_then([this, key, value, &t](auto mblock) {
+    }).si_then([this, key, value, &t](auto maybe_indirect_extent) {
+      assert(!maybe_indirect_extent.is_indirect());
+      assert(!maybe_indirect_extent.is_clone);
+      auto& mblock = maybe_indirect_extent.extent;
       mblock = get_mutable_extent(t, mblock
 	)->template cast<RootMetaBlock>();
 
@@ -878,6 +939,8 @@ private:
     extent_types_t type)
   {
     ceph_assert(!pin->parent_modified());
+    assert(!pin->is_indirect());
+    // Note: pin might be a clone
     auto v = pin->get_logical_extent(t);
     // checking the lba child must be atomic with creating
     // and linking the absent child
diff --git a/src/crimson/tools/store_nbd/tm_driver.cc b/src/crimson/tools/store_nbd/tm_driver.cc
index 7af0d996caa..389ecd78afc 100644
--- a/src/crimson/tools/store_nbd/tm_driver.cc
+++ b/src/crimson/tools/store_nbd/tm_driver.cc
@@ -82,11 +82,14 @@ TMDriver::read_extents_ret TMDriver::read_extents(
 	    return tm->read_pin<TestBlock>(
 	      t,
 	      std::move(pin)
-	    ).si_then([&ret](auto ref) mutable {
-	      ret.push_back(std::make_pair(ref->get_laddr(), ref));
+	    ).si_then([&ret](auto maybe_indirect_extent) mutable {
+	      assert(!maybe_indirect_extent.is_indirect());
+	      assert(!maybe_indirect_extent.is_clone);
+	      auto& e = maybe_indirect_extent.extent;
+	      ret.push_back(std::make_pair(e->get_laddr(), e));
 	      logger().debug(
 		"read_extents: got extent {}",
-		*ref);
+		*e);
 	      return seastar::now();
 	    });
 	  }).si_then([&ret] {
diff --git a/src/test/crimson/seastore/test_object_data_handler.cc b/src/test/crimson/seastore/test_object_data_handler.cc
index 5dbc3748e5b..e7374d1d9b8 100644
--- a/src/test/crimson/seastore/test_object_data_handler.cc
+++ b/src/test/crimson/seastore/test_object_data_handler.cc
@@ -264,10 +264,13 @@ struct object_data_handler_test_t:
     auto &layout = onode->get_layout();
     auto odata = layout.object_data.get();
     auto obase = odata.get_reserved_data_base();
-    auto ext = with_trans_intr(t, [&](auto& trans) {
+    auto maybe_indirect_ext = with_trans_intr(t, [&](auto& trans) {
       return tm->read_extent<ObjectDataBlock>(
 	trans, (obase + addr).checked_to_laddr(), len);
     }).unsafe_get();
+    EXPECT_FALSE(maybe_indirect_ext.is_clone);
+    EXPECT_FALSE(maybe_indirect_ext.is_indirect());
+    auto ext = maybe_indirect_ext.extent;
     EXPECT_EQ((obase + addr).checked_to_laddr(), ext->get_laddr());
     return ext;
   }
diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc
index 2d20c5fff94..7ee05f111e7 100644
--- a/src/test/crimson/seastore/test_transaction_manager.cc
+++ b/src/test/crimson/seastore/test_transaction_manager.cc
@@ -506,9 +506,10 @@ struct transaction_manager_test_t :
     ceph_assert(test_mappings.contains(addr, t.mapping_delta));
     ceph_assert(test_mappings.get(addr, t.mapping_delta).desc.len == len);
 
-    auto ext = with_trans_intr(*(t.t), [&](auto& trans) {
+    auto maybe_indirect_ext = with_trans_intr(*(t.t), [&](auto& trans) {
       return tm->read_pin<TestBlock>(trans, std::move(pin));
     }).unsafe_get();
+    auto ext = maybe_indirect_ext.extent;
     EXPECT_EQ(addr, ext->get_laddr());
     return ext;
   }
@@ -520,9 +521,10 @@ struct transaction_manager_test_t :
     ceph_assert(test_mappings.contains(addr, t.mapping_delta));
     ceph_assert(test_mappings.get(addr, t.mapping_delta).desc.len == len);
 
-    auto ext = with_trans_intr(*(t.t), [&](auto& trans) {
+    auto maybe_indirect_ext = with_trans_intr(*(t.t), [&](auto& trans) {
       return tm->read_extent<TestBlock>(trans, addr, len);
     }).unsafe_get();
+    auto ext = maybe_indirect_ext.extent;
     EXPECT_EQ(addr, ext->get_laddr());
     return ext;
   }
@@ -533,11 +535,10 @@ struct transaction_manager_test_t :
     ceph_assert(test_mappings.contains(addr, t.mapping_delta));
 
     using ertr = with_trans_ertr<TransactionManager::read_extent_iertr>;
-    using ret = ertr::future<TestBlockRef>;
     auto ext = with_trans_intr(*(t.t), [&](auto& trans) {
       return tm->read_extent<TestBlock>(trans, addr);
-    }).safe_then([](auto ext) -> ret {
-      return ertr::make_ready_future<TestBlockRef>(ext);
+    }).safe_then([](auto ret) {
+      return ertr::make_ready_future<TestBlockRef>(ret.extent);
     }).handle_error(
       [](const crimson::ct_error::eagain &e) {
 	return seastar::make_ready_future<TestBlockRef>();
@@ -560,11 +561,10 @@ struct transaction_manager_test_t :
     ceph_assert(test_mappings.get(addr, t.mapping_delta).desc.len == len);
 
     using ertr = with_trans_ertr<TransactionManager::read_extent_iertr>;
-    using ret = ertr::future<TestBlockRef>;
     auto ext = with_trans_intr(*(t.t), [&](auto& trans) {
       return tm->read_extent<TestBlock>(trans, addr, len);
-    }).safe_then([](auto ext) -> ret {
-      return ertr::make_ready_future<TestBlockRef>(ext);
+    }).safe_then([](auto ret) {
+      return ertr::make_ready_future<TestBlockRef>(ret.extent);
     }).handle_error(
       [](const crimson::ct_error::eagain &e) {
 	return seastar::make_ready_future<TestBlockRef>();
@@ -583,14 +583,13 @@ struct transaction_manager_test_t :
     test_transaction_t &t,
     LBAMappingRef &&pin) {
     using ertr = with_trans_ertr<TransactionManager::base_iertr>;
-    using ret = ertr::future<TestBlockRef>;
     bool indirect = pin->is_indirect();
     auto addr = pin->get_key();
     auto im_addr = indirect ? pin->get_intermediate_base() : L_ADDR_NULL;
     auto ext = with_trans_intr(*(t.t), [&](auto& trans) {
       return tm->read_pin<TestBlock>(trans, std::move(pin));
-    }).safe_then([](auto ext) -> ret {
-      return ertr::make_ready_future<TestBlockRef>(ext);
+    }).safe_then([](auto ret) {
+      return ertr::make_ready_future<TestBlockRef>(ret.extent);
     }).handle_error(
       [](const crimson::ct_error::eagain &e) {
 	return seastar::make_ready_future<TestBlockRef>();