librbd: restore diff-iterate include_parent functionality in fast-diff mode

Commit 4429ed4f3f4c ("librbd: switch diff iterate API to use new snaps
list dispatch methods") removed the recursive execute() call.  The new
list_snaps method does indeed handle parent diffs internally but it is
not used in fast-diff mode.  Nothing changed there -- we still need to
load the parent object map, calculate parent object_diff_state, etc.

Fixes: https://tracker.ceph.com/issues/53787
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>

diff --git a/src/librbd/api/DiffIterate.cc b/src/librbd/api/DiffIterate.cc
index 8b0fa836f81d902100d347764289f2c13786c174..e015944444d7aef87eab40ac64f905b1bf6b04af 100644 (file)
--- a/src/librbd/api/DiffIterate.cc
+++ b/src/librbd/api/DiffIterate.cc
@@ -37,7 +37,6 @@ struct DiffContext {
   bool include_parent;
   uint64_t from_snap_id;
   uint64_t end_snap_id;
-  interval_set<uint64_t> parent_diff;
   OrderedThrottle throttle;
 
   template <typename I>
@@ -150,6 +149,15 @@ private:
   }
 };
 
+int simple_diff_cb(uint64_t off, size_t len, int exists, void *arg) {
+  // it's possible for a discard to create a hole in the parent image -- ignore
+  if (exists) {
+    interval_set<uint64_t> *diff = static_cast<interval_set<uint64_t> *>(arg);
+    diff->insert(off, len);
+  }
+  return 0;
+}
+
 } // anonymous namespace
 
 template <typename I>
@@ -237,6 +245,7 @@ int DiffIterate<I>::execute() {
   int r;
   bool fast_diff_enabled = false;
   BitVector<2> object_diff_state;
+  interval_set<uint64_t> parent_diff;
   if (m_whole_object) {
     C_SaferCond ctx;
     auto req = object_map::DiffRequest<I>::create(&m_image_ctx, from_snap_id,
@@ -250,6 +259,22 @@ int DiffIterate<I>::execute() {
     } else {
       ldout(cct, 5) << "fast diff enabled" << dendl;
       fast_diff_enabled = true;
+
+      // check parent overlap only if we are comparing to the beginning of time
+      if (m_include_parent && from_snap_id == 0) {
+        std::shared_lock image_locker{m_image_ctx.image_lock};
+        uint64_t overlap = 0;
+        m_image_ctx.get_parent_overlap(m_image_ctx.snap_id, &overlap);
+        if (m_image_ctx.parent && overlap > 0) {
+          ldout(cct, 10) << " first getting parent diff" << dendl;
+          DiffIterate diff_parent(*m_image_ctx.parent, {}, nullptr, 0, overlap,
+                                  true, true, &simple_diff_cb, &parent_diff);
+          r = diff_parent.execute();
+          if (r < 0) {
+            return r;
+          }
+        }
+      }
     }
   }
 
@@ -282,13 +307,13 @@ int DiffIterate<I>::execute() {
         const uint64_t object_no = extents.front().objectno;
         uint8_t diff_state = object_diff_state[object_no];
         if (diff_state == object_map::DIFF_STATE_HOLE &&
-            from_snap_id == 0 && !diff_context.parent_diff.empty()) {
+            from_snap_id == 0 && !parent_diff.empty()) {
           // no data in child object -- report parent diff instead
           for (auto& oe : extents) {
             for (auto& be : oe.buffer_extents) {
               interval_set<uint64_t> o;
               o.insert(off + be.first, be.second);
-              o.intersection_of(diff_context.parent_diff);
+              o.intersection_of(parent_diff);
               ldout(cct, 20) << " reporting parent overlap " << o << dendl;
               for (auto e = o.begin(); e != o.end(); ++e) {
                 r = m_callback(e.get_start(), e.get_len(), true,

diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc
index dd5f492a47c3949ad18e4aa48f277b0ce03d7741..766a977367f3c48ea2ae8128848288156176fe5f 100644 (file)
--- a/src/test/librbd/test_librbd.cc
+++ b/src/test/librbd/test_librbd.cc
@@ -4518,6 +4518,73 @@ TYPED_TEST(DiffIterateTest, DiffIterateRegression6926)
   ASSERT_EQ(static_cast<size_t>(0), extents.size());
 }
 
+TYPED_TEST(DiffIterateTest, DiffIterateParent)
+{
+  REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
+
+  librados::IoCtx ioctx;
+  ASSERT_EQ(0, this->_rados.ioctx_create(this->m_pool_name.c_str(), ioctx));
+
+  {
+    librbd::RBD rbd;
+    librbd::Image image;
+    int order = 22;
+    std::string name = this->get_temp_image_name();
+    ssize_t size = 20 << 20;
+
+    ASSERT_EQ(0, create_image_pp(rbd, ioctx, name.c_str(), size, &order));
+    ASSERT_EQ(0, rbd.open(ioctx, image, name.c_str(), NULL));
+
+    uint64_t features;
+    ASSERT_EQ(0, image.features(&features));
+    uint64_t object_size = 0;
+    if (this->whole_object) {
+      object_size = 1 << order;
+    }
+
+    ceph::bufferlist bl;
+    bl.append(std::string(size, '1'));
+    ASSERT_EQ(size, image.write(0, size, bl));
+    ASSERT_EQ(0, image.snap_create("snap"));
+    ASSERT_EQ(0, image.snap_protect("snap"));
+
+    std::string clone_name = this->get_temp_image_name();
+    ASSERT_EQ(0, rbd.clone(ioctx, name.c_str(), "snap", ioctx,
+                           clone_name.c_str(), features, &order));
+    librbd::Image clone;
+    ASSERT_EQ(0, rbd.open(ioctx, clone, clone_name.c_str(), NULL));
+
+    std::vector<diff_extent> extents;
+    ASSERT_EQ(0, clone.diff_iterate2(NULL, 0, size, true, this->whole_object,
+                                     vector_iterate_cb, &extents));
+    ASSERT_EQ(5u, extents.size());
+    ASSERT_EQ(diff_extent(0, 4194304, true, object_size), extents[0]);
+    ASSERT_EQ(diff_extent(4194304, 4194304, true, object_size), extents[1]);
+    ASSERT_EQ(diff_extent(8388608, 4194304, true, object_size), extents[2]);
+    ASSERT_EQ(diff_extent(12582912, 4194304, true, object_size), extents[3]);
+    ASSERT_EQ(diff_extent(16777216, 4194304, true, object_size), extents[4]);
+    extents.clear();
+
+    ASSERT_EQ(0, clone.resize(size / 2));
+    ASSERT_EQ(0, clone.resize(size));
+    ASSERT_EQ(1, clone.write(size - 1, 1, bl));
+
+    ASSERT_EQ(0, clone.diff_iterate2(NULL, 0, size, true, this->whole_object,
+                                     vector_iterate_cb, &extents));
+    ASSERT_EQ(4u, extents.size());
+    ASSERT_EQ(diff_extent(0, 4194304, true, object_size), extents[0]);
+    ASSERT_EQ(diff_extent(4194304, 4194304, true, object_size), extents[1]);
+    ASSERT_EQ(diff_extent(8388608, 2097152, true, object_size), extents[2]);
+    // hole (parent overlap = 10M) followed by copyup'ed object
+    ASSERT_EQ(diff_extent(16777216, 4194304, true, object_size), extents[3]);
+
+    ASSERT_PASSED(this->validate_object_map, image);
+    ASSERT_PASSED(this->validate_object_map, clone);
+  }
+
+  ioctx.close();
+}
+
 TYPED_TEST(DiffIterateTest, DiffIterateIgnoreParent)
 {
   REQUIRE_FEATURE(RBD_FEATURE_LAYERING);