librbd: deep_copy: don't hide parent if zero overlap for snapshot

Fixes: http://tracker.ceph.com/issues/24545
Signed-off-by: Mykola Golub <mgolub@suse.com>

diff --git a/src/librbd/deep_copy/ObjectCopyRequest.cc b/src/librbd/deep_copy/ObjectCopyRequest.cc
index 7fb911d23df847a5f17045c513d0454fccc82b56..5fb70c7f37c8c367fc2bc492bbcabcb66e8abcf9 100644 (file)
--- a/src/librbd/deep_copy/ObjectCopyRequest.cc
+++ b/src/librbd/deep_copy/ObjectCopyRequest.cc
@@ -816,42 +816,46 @@ void ObjectCopyRequest<I>::compute_zero_ops() {
 
   bool fast_diff = m_dst_image_ctx->test_features(RBD_FEATURE_FAST_DIFF);
   uint64_t prev_end_size = 0;
-  bool hide_parent = false;
-  if (m_src_parent_image_ctx != nullptr) {
-    RWLock::RLocker snap_locker(m_dst_image_ctx->snap_lock);
-    RWLock::RLocker parent_locker(m_dst_image_ctx->parent_lock);
-    auto dst_snap_seq = m_snap_map.begin()->second.front();
-    uint64_t parent_overlap = 0;
-    int r = m_dst_image_ctx->get_parent_overlap(dst_snap_seq, &parent_overlap);
-    if (r < 0) {
-      ldout(m_cct, 5) << "failed getting parent overlap for snap_id: "
-                      << dst_snap_seq << ": " << cpp_strerror(r) << dendl;
-    }
-    if (parent_overlap == 0) {
-      ldout(m_cct, 20) << "no parent overlap" << dendl;
-    } else {
-      std::vector<std::pair<uint64_t, uint64_t>> image_extents;
-      Striper::extent_to_file(m_cct, &m_dst_image_ctx->layout,
-                              m_dst_object_number, 0,
-                              m_dst_image_ctx->layout.object_size,
-                              image_extents);
-      uint64_t overlap = m_dst_image_ctx->prune_parent_extents(image_extents,
-                                                               parent_overlap);
-      if (overlap == 0) {
+  bool hide_parent = m_src_parent_image_ctx != nullptr;
+
+  for (auto &it : m_dst_zero_interval) {
+    auto src_snap_seq = it.first;
+    auto &zero_interval = it.second;
+
+    if (hide_parent) {
+      RWLock::RLocker snap_locker(m_dst_image_ctx->snap_lock);
+      RWLock::RLocker parent_locker(m_dst_image_ctx->parent_lock);
+      auto snap_map_it = m_snap_map.find(src_snap_seq);
+      assert(snap_map_it != m_snap_map.end());
+      auto dst_snap_seq = snap_map_it->second.front();
+      uint64_t parent_overlap = 0;
+      int r = m_dst_image_ctx->get_parent_overlap(dst_snap_seq, &parent_overlap);
+      if (r < 0) {
+        ldout(m_cct, 5) << "failed getting parent overlap for snap_id: "
+                        << dst_snap_seq << ": " << cpp_strerror(r) << dendl;
+      }
+      if (parent_overlap == 0) {
         ldout(m_cct, 20) << "no parent overlap" << dendl;
+        hide_parent = false;
       } else {
-        for (auto e : image_extents) {
-          prev_end_size += e.second;
+        std::vector<std::pair<uint64_t, uint64_t>> image_extents;
+        Striper::extent_to_file(m_cct, &m_dst_image_ctx->layout,
+                                m_dst_object_number, 0,
+                                m_dst_image_ctx->layout.object_size,
+                                image_extents);
+        uint64_t overlap = m_dst_image_ctx->prune_parent_extents(image_extents,
+                                                                 parent_overlap);
+        if (overlap == 0) {
+          ldout(m_cct, 20) << "no parent overlap" << dendl;
+          hide_parent = false;
+        } else if (src_snap_seq == m_dst_zero_interval.begin()->first) {
+          for (auto e : image_extents) {
+            prev_end_size += e.second;
+          }
+          assert(prev_end_size <= m_dst_image_ctx->layout.object_size);
         }
-        assert(prev_end_size <= m_dst_image_ctx->layout.object_size);
-        hide_parent = true;
       }
     }
-  }
-
-  for (auto &it : m_dst_zero_interval) {
-    auto src_snap_seq = it.first;
-    auto &zero_interval = it.second;
 
     uint64_t end_size = prev_end_size;
 

diff --git a/src/test/librbd/test_DeepCopy.cc b/src/test/librbd/test_DeepCopy.cc
index 5af04c66dc3456ebfafe900c74c10eff6b7a149e..0f14e855097a03d817e6257a45969583870f0466 100644 (file)
--- a/src/test/librbd/test_DeepCopy.cc
+++ b/src/test/librbd/test_DeepCopy.cc
@@ -36,12 +36,6 @@ struct TestDeepCopy : public TestFixture {
     if (m_src_ictx != nullptr) {
       deep_copy();
       if (m_dst_ictx != nullptr) {
-        if (m_dst_ictx->test_features(RBD_FEATURE_LAYERING)) {
-          bool flags_set;
-          EXPECT_EQ(0, m_dst_ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID,
-                                              &flags_set));
-          EXPECT_FALSE(flags_set);
-        }
         compare();
         close_image(m_dst_ictx);
       }
@@ -91,6 +85,13 @@ struct TestDeepCopy : public TestFixture {
       }
       EXPECT_EQ(src_size, dst_size);
 
+      if (m_dst_ictx->test_features(RBD_FEATURE_LAYERING)) {
+        bool flags_set;
+        EXPECT_EQ(0, m_dst_ictx->test_flags(RBD_FLAG_OBJECT_MAP_INVALID,
+                                            &flags_set));
+        EXPECT_FALSE(flags_set);
+      }
+
       ssize_t read_size = 1 << m_src_ictx->order;
       uint64_t offset = 0;
       while (offset < src_size) {
@@ -260,6 +261,45 @@ struct TestDeepCopy : public TestFixture {
     ASSERT_EQ(0, m_src_ictx->operations->resize(new_size, true, no_op));
   }
 
+  void test_clone_hide_parent() {
+    uint64_t object_size = 1 << m_src_ictx->order;
+    bufferlist bl;
+    bl.append(std::string(100, '1'));
+    ASSERT_EQ(static_cast<ssize_t>(bl.length()),
+              m_src_ictx->io_work_queue->write(object_size, bl.length(),
+                                               bufferlist{bl}, 0));
+    ASSERT_EQ(0, m_src_ictx->io_work_queue->flush());
+
+    ASSERT_EQ(0, snap_create(*m_src_ictx, "snap"));
+    ASSERT_EQ(0, snap_protect(*m_src_ictx, "snap"));
+
+    std::string clone_name = get_temp_image_name();
+    int order = m_src_ictx->order;
+    uint64_t features;
+    ASSERT_EQ(0, librbd::get_features(m_src_ictx, &features));
+    ASSERT_EQ(0, librbd::clone(m_ioctx, m_src_ictx->name.c_str(), "snap",
+                               m_ioctx, clone_name.c_str(), features, &order, 0,
+                               0));
+    close_image(m_src_ictx);
+    ASSERT_EQ(0, open_image(clone_name, &m_src_ictx));
+
+    ASSERT_EQ(0, snap_create(*m_src_ictx, "snap1"));
+
+    ASSERT_EQ(static_cast<ssize_t>(bl.length()),
+              m_src_ictx->io_work_queue->discard(object_size, bl.length(),
+                                                 false));
+    ASSERT_EQ(0, m_src_ictx->io_work_queue->flush());
+
+    ASSERT_EQ(0, snap_create(*m_src_ictx, "snap2"));
+
+    librbd::NoOpProgressContext no_op;
+    ASSERT_EQ(0, m_src_ictx->operations->resize(object_size, true, no_op));
+
+    ASSERT_EQ(0, snap_create(*m_src_ictx, "snap3"));
+
+    ASSERT_EQ(0, m_src_ictx->operations->resize(2 * object_size, true, no_op));
+  }
+
   void test_clone() {
     bufferlist bl;
     bl.append(std::string(((1 << m_src_ictx->order) * 2) + 1, '1'));
@@ -440,6 +480,13 @@ TEST_F(TestDeepCopy, CloneExpand)
   test_clone_expand();
 }
 
+TEST_F(TestDeepCopy, CloneHideParent)
+{
+  REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
+
+  test_clone_hide_parent();
+}
+
 TEST_F(TestDeepCopy, Clone)
 {
   REQUIRE_FEATURE(RBD_FEATURE_LAYERING);