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>

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 51912a278672defc872ad9af3bb85455bc77ad75..62f606eb34dfb1b8d2f2ce0c9675409100172a56 100644 (file)
--- 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 4aef7ae66f6fef0e9d348b51fc077c32f573a7f1..2da290d23ca231079296586c01f67a31220eef42 100644 (file)
--- 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);
+}