crimson/osd/object_context_loader: fix with_clone_obc on resolve_oid case

Resolve_oid on a clone object may actually return the head:
```
    // Because oid.snap > ss.seq, we are trying to read from a snapshot
    // taken after the most recent write to this object. Read from head.
```

In this case, with_clone_obc should apply `func` same as with_head_obc would have.

Note: previously, with_clone_obc_only was called on the resolved head object.
While it didn't cause any errors, using the head_obc as clone is wrong.

Signed-off-by: Matan Breizman <mbreizma@redhat.com>
(cherry picked from commit d8aad5576f67c0e255b42c5d26334c87ba705931)

diff --git a/src/crimson/osd/object_context.cc b/src/crimson/osd/object_context.cc
index 1ea701c229c15610fcd05ca4055cf836f4296193..d7c0c60bc4e61763051608950529e0b6799a85d7 100644 (file)
--- a/src/crimson/osd/object_context.cc
+++ b/src/crimson/osd/object_context.cc
@@ -53,6 +53,7 @@ std::optional<hobject_t> resolve_oid(
   if (oid.snap > ss.seq) {
     // Because oid.snap > ss.seq, we are trying to read from a snapshot
     // taken after the most recent write to this object. Read from head.
+    logger().debug("{} returning head", __func__);
     return oid.get_head();
   } else {
     // which clone would it be?

diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc
index b8a6cbceb296c2d0f52a023dbb9062c1a9e9df54..4f0e2aeff0a4b1cb4a659ac168bdab9181e08427 100644 (file)
--- a/src/crimson/osd/object_context_loader.cc
+++ b/src/crimson/osd/object_context_loader.cc
@@ -56,18 +56,24 @@ using crimson::common::local_conf;
   template<RWState::State State>
   ObjectContextLoader::load_obc_iertr::future<>
   ObjectContextLoader::with_clone_obc_only(ObjectContextRef head,
-                                           hobject_t oid,
+                                           hobject_t clone_oid,
                                            with_obc_func_t&& func)
   {
     LOG_PREFIX(ObjectContextLoader::with_clone_obc_only);
-    auto coid = resolve_oid(head->get_head_ss(), oid);
-    if (!coid) {
-      ERRORDPP("clone {} not found", dpp, oid);
+    DEBUGDPP("{}", clone_oid);
+    assert(!clone_oid.is_head());
+    auto resolved_oid = resolve_oid(head->get_head_ss(), clone_oid);
+    if (!resolved_oid) {
+      ERRORDPP("clone {} not found", dpp, clone_oid);
       return load_obc_iertr::future<>{
         crimson::ct_error::enoent::make()
       };
     }
-    auto [clone, existed] = obc_registry.get_cached_obc(*coid);
+    if (resolved_oid->is_head()) {
+      // See resolve_oid
+      return std::move(func)(head, head);
+    }
+    auto [clone, existed] = obc_registry.get_cached_obc(*resolved_oid);
     return clone->template with_lock<State, IOInterruptCondition>(
       [existed=existed, clone=std::move(clone),
        func=std::move(func), head=std::move(head), this]() mutable

diff --git a/src/crimson/osd/object_context_loader.h b/src/crimson/osd/object_context_loader.h
index 0cd50623abc256678a190ccd25609fe3ad0d0b53..eeadb9daa92c4fa9886f763071a75951b31eaa4d 100644 (file)
--- a/src/crimson/osd/object_context_loader.h
+++ b/src/crimson/osd/object_context_loader.h
@@ -43,7 +43,7 @@ public:
   // with an already locked head.
   template<RWState::State State>
   load_obc_iertr::future<> with_clone_obc_only(ObjectContextRef head,
-                                               hobject_t oid,
+                                               hobject_t clone_oid,
                                                with_obc_func_t&& func);
 
   // Use this variant in the case where both the head