From 04ae69fdd112918509c01001a324ef7c09a6ca5e Mon Sep 17 00:00:00 2001
From: chunmei-liu <chunmei.liu@intel.com>
Date: Mon, 7 Jun 2021 16:48:20 -0700
Subject: [PATCH] crimson/seastore: fix OTree read invalid extent

Signed-off-by: chunmei-liu <chunmei.liu@intel.com>
---
 src/crimson/os/seastore/cache.h               | 14 +++---
 .../seastore/test_transaction_manager.cc      | 45 ++++++++++++++++---
 2 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 0c10f2c4e86e..01b59c91c467 100644
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -181,12 +181,14 @@ public:
       auto ret = TCachedExtentRef<T>(static_cast<T*>(&*iter));
       return ret->wait_io(
       ).then([ret=std::move(ret)]() mutable -> get_extent_ret<T> {
-	if (!ret->is_retired()) {
-	  return get_extent_ret<T>(
-	    get_extent_ertr::ready_future_marker{},
-	    std::move(ret));
-	} else {
-	  return crimson::ct_error::eagain::make();
+        if (ret->is_valid()) {
+          return get_extent_ret<T>(
+            get_extent_ertr::ready_future_marker{},
+            std::move(ret));
+        } else if (ret->is_retired()) {
+          ceph_abort_msg("impossible retired extent");
+        } else {
+          return crimson::ct_error::eagain::make();
 	}
       });
     } else {
diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc
index 1a0d65e5e802..b48edeaf5ee4 100644
--- a/src/test/crimson/seastore/test_transaction_manager.cc
+++ b/src/test/crimson/seastore/test_transaction_manager.cc
@@ -430,6 +430,33 @@ struct transaction_manager_test_t :
     return ext;
   }
 
+  TestBlockRef try_get_extent(
+    test_transaction_t &t,
+    laddr_t addr,
+    extent_len_t len) {
+    ceph_assert(test_mappings.contains(addr, t.mapping_delta));
+    ceph_assert(test_mappings.get(addr, t.mapping_delta).desc.len == len);
+
+    using ertr = TransactionManager::read_extent_ertr;
+    using ret = ertr::future<TestBlockRef>;
+    auto ext = tm->read_extent<TestBlock>(
+      *t.t, addr, len
+    ).safe_then([](auto ext) -> ret {
+      return ertr::make_ready_future<TestBlockRef>(ext);
+    }).handle_error(
+      [](const crimson::ct_error::eagain &e) {
+	return seastar::make_ready_future<TestBlockRef>();
+      },
+      crimson::ct_error::assert_all{
+	"get_extent got invalid error"
+      }
+    ).get0();
+    if (ext) {
+      EXPECT_EQ(addr, ext->get_laddr());
+    }
+    return ext;
+  }
+
   test_block_mutator_t mutator;
   TestBlockRef mutate_extent(
     test_transaction_t &t,
@@ -897,13 +924,17 @@ TEST_F(transaction_manager_test_t, random_writes_concurrent)
       boost::make_counting_iterator(0u),
       boost::make_counting_iterator(WRITE_STREAMS),
       [&](auto) {
-	return seastar::async([&] {
-	  while (writes < 300) {
-	    auto t = create_transaction();
-	    auto ext = get_extent(
-	      t,
-	      get_random_laddr(BSIZE, TOTAL),
-	      BSIZE);
+        return seastar::async([&] {
+          while (writes < 300) {
+            auto t = create_transaction();
+            auto ext = try_get_extent(
+              t,
+              get_random_laddr(BSIZE, TOTAL),
+              BSIZE);
+            if (!ext){
+              failures++;
+              continue;
+            }
 	    auto mut = mutate_extent(t, ext);
 	    auto success = try_submit_transaction(std::move(t));
 	    writes += success;
-- 
2.47.3