From c4f1b42810339b5b45b48f74c3ff0ae661a8807a Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Mon, 12 Dec 2016 11:53:00 -0500
Subject: [PATCH] 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.

Hammer-specific implementation due to librbd refactoring during the
Infernalis release.

Fixes: http://tracker.ceph.com/issues/18111
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---
 src/librbd/internal.cc           | 29 +++++++++---------
 src/test/librbd/test_internal.cc | 50 ++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 15 deletions(-)

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 51912a278672d..62f606eb34dfb 100644
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -3065,8 +3065,20 @@ reprotect_and_return_err:
 
 	librados::snap_set_t snap_set;
 	int r = head_ctx.list_snaps(p->first.name, &snap_set);
-	if (r == -ENOENT) {
-	  if (from_snap_id == 0 && !parent_diff.empty()) {
+	if (r < 0 && r != -ENOENT) {
+	  return r;
+	}
+
+	// calc diff from from_snap_id -> to_snap_id
+	interval_set<uint64_t> diff;
+	bool end_exists = false;
+	if (!snap_set.clones.empty()) {
+	  calc_snap_set_diff(ictx->cct, snap_set, from_snap_id, end_snap_id,
+			     &diff, &end_exists);
+	  ldout(ictx->cct, 20) << "  diff " << diff << " end_exists=" << end_exists << dendl;
+	}
+	if (diff.empty()) {
+	  if (from_snap_id == 0 && !parent_diff.empty() && !end_exists) {
 	    // report parent diff instead
 	    for (vector<ObjectExtent>::iterator q = p->second.begin(); q != p->second.end(); ++q) {
 	      for (vector<pair<uint64_t,uint64_t> >::iterator r = q->buffer_extents.begin();
@@ -3084,19 +3096,6 @@ reprotect_and_return_err:
 	  }
 	  continue;
 	}
-	if (r < 0)
-	  return r;
-
-	// calc diff from from_snap_id -> to_snap_id
-	interval_set<uint64_t> diff;
-	bool end_exists;
-	calc_snap_set_diff(ictx->cct, snap_set,
-			   from_snap_id,
-			   end_snap_id,
-			   &diff, &end_exists);
-	ldout(ictx->cct, 20) << "  diff " << diff << " end_exists=" << end_exists << dendl;
-	if (diff.empty())
-	  continue;
 
 	for (vector<ObjectExtent>::iterator q = p->second.begin(); q != p->second.end(); ++q) {
 	  ldout(ictx->cct, 20) << "diff_iterate object " << p->first
diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc
index 4aef7ae66f6fe..2da290d23ca23 100644
--- a/src/test/librbd/test_internal.cc
+++ b/src/test/librbd/test_internal.cc
@@ -390,3 +390,53 @@ TEST_F(TestInternal, ShrinkFlushesCache) {
   ASSERT_EQ(0, cond_ctx.wait());
   c->put();
 }
+
+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_iterate(NULL, 0, size, 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, librbd::snap_create(ictx, "one"));
+  ASSERT_EQ(0, librbd::snap_protect(ictx, "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, NULL, 0, size, iterate_cb,
+				    (void *)&diff));
+  ASSERT_EQ(one, diff);
+}
-- 
2.39.5