librbd: diffs to clone's first snapshot should include parent diffs

If the clone has a backing object created after the snapshot that
overwrites an extent of the parent, the parent diffs within that
extent are not included in the result.

Fixes: http://tracker.ceph.com/issues/18068
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/DiffIterate.cc b/src/librbd/DiffIterate.cc
index 9b0a3ac3daa86f8765406ef5cda287e623023c62..c0165c72774a77a94dbbdb26db920d5268da42aa 100644 (file)
--- a/src/librbd/DiffIterate.cc
+++ b/src/librbd/DiffIterate.cc
@@ -131,6 +131,9 @@ private:
     ldout(cct, 20) << "  diff " << diff << " end_exists=" << end_exists
                    << dendl;
     if (diff.empty()) {
+      if (m_diff_context.from_snap_id == 0 && !end_exists) {
+        compute_parent_overlap(diffs);
+      }
       return;
     } else if (m_diff_context.whole_object) {
       // provide the full object extents to the callback

diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc
index 0dea0f4922f9f3eb58f7ef59689e0feb7ded82fb..df65318cf24a4b77d9d352fd0bd983d25572a1e5 100644 (file)
--- a/src/test/librbd/test_internal.cc
+++ b/src/test/librbd/test_internal.cc
@@ -850,3 +850,55 @@ TEST_F(TestInternal, RemoveById) {
   librbd::NoOpProgressContext remove_no_op;
   ASSERT_EQ(0, librbd::remove(m_ioctx, "", image_id, remove_no_op));
 }
+
+static int iterate_cb(uint64_t off, size_t len, int exists, void *arg)
+{
+  interval_set<uint64_t> *diff = static_cast<interval_set<uint64_t> *>(arg);
+  diff->insert(off, len);
+  return 0;
+}
+
+TEST_F(TestInternal, DiffIterateCloneOverwrite) {
+  REQUIRE_FEATURE(RBD_FEATURE_LAYERING);
+
+  librbd::RBD rbd;
+  librbd::Image image;
+  uint64_t size = 20 << 20;
+  int order = 0;
+
+  ASSERT_EQ(0, rbd.open(m_ioctx, image, m_image_name.c_str(), NULL));
+
+  bufferlist bl;
+  bl.append(std::string(4096, '1'));
+  ASSERT_EQ(4096, image.write(0, 4096, bl));
+
+  interval_set<uint64_t> one;
+  ASSERT_EQ(0, image.diff_iterate2(NULL, 0, size, false, false, iterate_cb,
+                                   (void *)&one));
+  ASSERT_EQ(0, image.snap_create("one"));
+  ASSERT_EQ(0, image.snap_protect("one"));
+
+  std::string clone_name = this->get_temp_image_name();
+  ASSERT_EQ(0, rbd.clone(m_ioctx, m_image_name.c_str(), "one", m_ioctx,
+                         clone_name.c_str(), RBD_FEATURE_LAYERING, &order));
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(clone_name, &ictx));
+  ASSERT_EQ(0, snap_create(*ictx, "one"));
+  ASSERT_EQ(0, ictx->operations->snap_protect("one"));
+
+  // Simulate a client that doesn't support deep flatten (old librbd / krbd)
+  // which will copy up the full object from the parent
+  std::string oid = ictx->object_prefix + ".0000000000000000";
+  librados::IoCtx io_ctx;
+  io_ctx.dup(m_ioctx);
+  io_ctx.selfmanaged_snap_set_write_ctx(ictx->snapc.seq, ictx->snaps);
+  ASSERT_EQ(0, io_ctx.write(oid, bl, 4096, 4096));
+
+  interval_set<uint64_t> diff;
+  ASSERT_EQ(0, librbd::snap_set(ictx, "one"));
+  ASSERT_EQ(0, librbd::diff_iterate(ictx, nullptr, 0, size, true, false,
+                                    iterate_cb, (void *)&diff));
+  ASSERT_EQ(one, diff);
+}
+