From: Jason Dillaman <dillaman@redhat.com>
Date: Mon, 14 Sep 2020 18:26:51 +0000 (-0400)
Subject: librbd: don't attempt to read from object snapshots known to not exist
X-Git-Tag: v16.1.0~961^2~2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=7758782380f722063a9da6c13d5a3a1c64bef910;p=ceph.git

librbd: don't attempt to read from object snapshots known to not exist

If there is an issue with the list-snaps command that results in a forced
read of the full object, don't attempt to read from any snapshots that
are known to not exist. This can occur if cache-tiering does not have the
full snapshot context or if there is a bug in the list-snaps op
implementation.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---

diff --git a/src/librbd/deep_copy/ObjectCopyRequest.cc b/src/librbd/deep_copy/ObjectCopyRequest.cc
index 3602a54f74d..569bb7e75a8 100644
--- a/src/librbd/deep_copy/ObjectCopyRequest.cc
+++ b/src/librbd/deep_copy/ObjectCopyRequest.cc
@@ -105,7 +105,9 @@ void ObjectCopyRequest<I>::handle_list_snaps(int r) {
 
   ldout(m_cct, 20) << "snapshot_delta=" << m_snapshot_delta << dendl;
 
+  compute_dst_object_may_exist();
   compute_read_ops();
+
   send_read();
 }
 
@@ -206,6 +208,7 @@ void ObjectCopyRequest<I>::send_write_object() {
     ceph_assert(dst_may_exist_it != m_dst_object_may_exist.end());
     if (!dst_may_exist_it->second && !write_ops.empty()) {
       // if the object cannot exist, the only valid op is to remove it
+      ldout(m_cct, 20) << "object DNE: src_snap_seq=" << src_snap_seq << dendl;
       ceph_assert(write_ops.size() == 1U);
       ceph_assert(write_ops.begin()->type == WRITE_OP_TYPE_REMOVE);
     }
@@ -419,6 +422,24 @@ void ObjectCopyRequest<I>::compute_read_ops() {
   // read from our parent
   for (auto& [key, image_intervals] : m_snapshot_delta) {
     io::WriteReadSnapIds write_read_snap_ids{key};
+
+    // advance the src write snap id to the first valid snap id
+    if (write_read_snap_ids != io::INITIAL_WRITE_READ_SNAP_IDS) {
+      // don't attempt to read from snapshots that shouldn't exist in
+      // case the OSD fails to give a correct snap list
+      auto snap_map_it = m_snap_map.find(write_read_snap_ids.first);
+      ceph_assert(snap_map_it != m_snap_map.end());
+      auto dst_snap_seq = snap_map_it->second.front();
+
+      auto dst_may_exist_it = m_dst_object_may_exist.find(dst_snap_seq);
+      ceph_assert(dst_may_exist_it != m_dst_object_may_exist.end());
+      if (!dst_may_exist_it->second) {
+        ldout(m_cct, 20) << "DNE snapshot: " << write_read_snap_ids.first
+                         << dendl;
+        continue;
+      }
+    }
+
     for (auto& image_interval : image_intervals) {
       auto state = image_interval.get_val().state;
       switch (state) {
@@ -547,8 +568,6 @@ void ObjectCopyRequest<I>::merge_write_ops() {
 
 template <typename I>
 void ObjectCopyRequest<I>::compute_zero_ops() {
-  compute_dst_object_may_exist();
-
   ldout(m_cct, 20) << dendl;
 
   m_src_image_ctx->image_lock.lock_shared();
@@ -739,6 +758,9 @@ void ObjectCopyRequest<I>::compute_dst_object_may_exist() {
     m_dst_object_may_exist[snap_id] =
       (m_dst_object_number < m_dst_image_ctx->get_object_count(snap_id));
   }
+
+  ldout(m_cct, 20) << "dst_object_may_exist=" << m_dst_object_may_exist
+                   << dendl;
 }
 
 } // namespace deep_copy
diff --git a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc
index 582875c81ee..72c194f87bb 100644
--- a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc
+++ b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc
@@ -542,6 +542,7 @@ TEST_F(TestMockDeepCopyObjectCopyRequest, ReadError) {
   mock_dst_image_ctx.object_map = &mock_object_map;
 
   expect_test_features(mock_dst_image_ctx);
+  expect_get_object_count(mock_dst_image_ctx);
 
   C_SaferCond ctx;
   MockObjectCopyRequest *request = create_request(mock_src_image_ctx,