librados/snap_set_diff: ignore truncates above size at start

Because currently calc_snap_set_diff() only ever appends to the running
diff, an excessive (either too large or completely bogus) zero extent
is reported in cases where an object is first expanded (with a snapshot
taken at that point) and then truncated but still above the size of the
object as of the starting snapshot.

Fixes: https://tracker.ceph.com/issues/63770
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit 5b11fb314abefea390c5663c33e9e1ace6179974)

diff --git a/src/librados/snap_set_diff.cc b/src/librados/snap_set_diff.cc
index 0029bcd647801be2779df94a5677016797fb9a48..42d78eb0392c3c96159cd0ea437c612f08f87ba1 100644 (file)
--- a/src/librados/snap_set_diff.cc
+++ b/src/librados/snap_set_diff.cc
@@ -86,26 +86,36 @@ void calc_snap_set_diff(CephContext *cct, const librados::snap_set_t& snap_set,
       return;
     }
 
-    // start with the max(this size, next size), and subtract off any
-    // overlap
+    // start with the largest possible diff to next, and subtract off
+    // any overlap
     const vector<pair<uint64_t, uint64_t> > *overlap = &r->overlap;
     interval_set<uint64_t> diff_to_next;
-    uint64_t max_size = r->size;
+    uint64_t diff_boundary;
+    uint64_t prev_size = r->size;
     ++r;
     if (r != snap_set.clones.end()) {
-      if (r->size > max_size)
-       max_size = r->size;
-    }
-    if (max_size)
-      diff_to_next.insert(0, max_size);
-    for (vector<pair<uint64_t, uint64_t> >::const_iterator p = overlap->begin();
-        p != overlap->end();
-        ++p) {
-      diff_to_next.erase(p->first, p->second);
+      if (r->size >= prev_size) {
+        diff_boundary = r->size;
+      } else if (prev_size <= start_size) {
+        // truncated range below size at start
+        diff_boundary = prev_size;
+      } else {
+        // truncated range (partially) above size at start -- drop that
+        // part from the running diff
+        diff_boundary = std::max(r->size, start_size);
+        ldout(cct, 20) << "  no more diff beyond " << diff_boundary << dendl;
+        diff->erase(diff_boundary, prev_size - diff_boundary);
+      }
+      if (diff_boundary) {
+        diff_to_next.insert(0, diff_boundary);
+      }
+      for (auto p = overlap->begin(); p != overlap->end(); ++p) {
+        diff_to_next.erase(p->first, p->second);
+      }
+      ldout(cct, 20) << "  diff_to_next " << diff_to_next << dendl;
+      diff->union_of(diff_to_next);
+      ldout(cct, 20) << "  diff now " << *diff << dendl;
     }
-    ldout(cct, 20) << "  diff_to_next " << diff_to_next << dendl;
-    diff->union_of(diff_to_next);
-    ldout(cct, 20) << "  diff now " << *diff << dendl;
   }
 
   if (r != snap_set.clones.end()) {

diff --git a/src/test/librbd/io/test_mock_ObjectRequest.cc b/src/test/librbd/io/test_mock_ObjectRequest.cc
index 91e8827b9515a74fd6ff1770321e33b4aa067698..8357000ca151b65bccb9d28710f883b98c559952 100644 (file)
--- a/src/test/librbd/io/test_mock_ObjectRequest.cc
+++ b/src/test/librbd/io/test_mock_ObjectRequest.cc
@@ -1784,6 +1784,432 @@ TEST_F(TestMockIoObjectRequest, ListSnaps) {
   ASSERT_EQ(expected_snapshot_delta, snapshot_delta);
 }
 
+TEST_F(TestMockIoObjectRequest, ListSnapsGrowFromSizeAtStart) {
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockTestImageCtx mock_image_ctx(*ictx);
+  mock_image_ctx.snaps = {3, 4};
+
+  librados::snap_set_t snap_set;
+  snap_set.seq = 4;
+  librados::clone_info_t clone_info;
+
+  clone_info.cloneid = 3;
+  clone_info.snaps = {3};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 512}};
+  clone_info.size = 512;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = 4;
+  clone_info.snaps = {4};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 2048}};
+  clone_info.size = 2048;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = CEPH_NOSNAP;
+  clone_info.snaps = {};
+  clone_info.overlap = {};
+  clone_info.size = 3072;
+  snap_set.clones.push_back(clone_info);
+
+  expect_list_snaps(mock_image_ctx, snap_set, 0);
+
+  {
+    SnapshotDelta snapshot_delta;
+    C_SaferCond ctx;
+    auto req = MockObjectListSnapsRequest::create(
+      &mock_image_ctx, 0, {{0, mock_image_ctx.layout.object_size}},
+      {4, CEPH_NOSNAP}, 0, {}, &snapshot_delta, &ctx);
+    req->send();
+    ASSERT_EQ(0, ctx.wait());
+
+    SnapshotDelta expected_snapshot_delta;
+    expected_snapshot_delta[{CEPH_NOSNAP,CEPH_NOSNAP}].insert(
+      2048, 1024, {SPARSE_EXTENT_STATE_DATA, 1024});
+    EXPECT_EQ(expected_snapshot_delta, snapshot_delta);
+  }
+
+  expect_list_snaps(mock_image_ctx, snap_set, 0);
+
+  {
+    SnapshotDelta snapshot_delta;
+    C_SaferCond ctx;
+    auto req = MockObjectListSnapsRequest::create(
+      &mock_image_ctx, 0, {{0, mock_image_ctx.layout.object_size}},
+      {3, 4, CEPH_NOSNAP}, 0, {}, &snapshot_delta, &ctx);
+    req->send();
+    ASSERT_EQ(0, ctx.wait());
+
+    SnapshotDelta expected_snapshot_delta;
+    expected_snapshot_delta[{4,4}].insert(
+      512, 1536, {SPARSE_EXTENT_STATE_DATA, 1536});
+    expected_snapshot_delta[{CEPH_NOSNAP,CEPH_NOSNAP}].insert(
+      2048, 1024, {SPARSE_EXTENT_STATE_DATA, 1024});
+    EXPECT_EQ(expected_snapshot_delta, snapshot_delta);
+  }
+}
+
+TEST_F(TestMockIoObjectRequest, ListSnapsTruncateFromSizeAtStart) {
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockTestImageCtx mock_image_ctx(*ictx);
+  mock_image_ctx.snaps = {3, 4};
+
+  librados::snap_set_t snap_set;
+  snap_set.seq = 4;
+  librados::clone_info_t clone_info;
+
+  clone_info.cloneid = 3;
+  clone_info.snaps = {3};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 512}};
+  clone_info.size = 512;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = 4;
+  clone_info.snaps = {4};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 1536}};
+  clone_info.size = 2048;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = CEPH_NOSNAP;
+  clone_info.snaps = {};
+  clone_info.overlap = {};
+  clone_info.size = 1536;
+  snap_set.clones.push_back(clone_info);
+
+  expect_list_snaps(mock_image_ctx, snap_set, 0);
+
+  {
+    SnapshotDelta snapshot_delta;
+    C_SaferCond ctx;
+    auto req = MockObjectListSnapsRequest::create(
+      &mock_image_ctx, 0, {{0, mock_image_ctx.layout.object_size}},
+      {4, CEPH_NOSNAP}, 0, {}, &snapshot_delta, &ctx);
+    req->send();
+    ASSERT_EQ(0, ctx.wait());
+
+    SnapshotDelta expected_snapshot_delta;
+    expected_snapshot_delta[{CEPH_NOSNAP,CEPH_NOSNAP}].insert(
+      1536, 512, {SPARSE_EXTENT_STATE_ZEROED, 512});
+    EXPECT_EQ(expected_snapshot_delta, snapshot_delta);
+  }
+
+  expect_list_snaps(mock_image_ctx, snap_set, 0);
+
+  {
+    SnapshotDelta snapshot_delta;
+    C_SaferCond ctx;
+    auto req = MockObjectListSnapsRequest::create(
+      &mock_image_ctx, 0, {{0, mock_image_ctx.layout.object_size}},
+      {3, 4, CEPH_NOSNAP}, 0, {}, &snapshot_delta, &ctx);
+    req->send();
+    ASSERT_EQ(0, ctx.wait());
+
+    SnapshotDelta expected_snapshot_delta;
+    expected_snapshot_delta[{4,4}].insert(
+      512, 1536, {SPARSE_EXTENT_STATE_DATA, 1536});
+    expected_snapshot_delta[{CEPH_NOSNAP,CEPH_NOSNAP}].insert(
+      1536, 512, {SPARSE_EXTENT_STATE_ZEROED, 512});
+    EXPECT_EQ(expected_snapshot_delta, snapshot_delta);
+  }
+}
+
+TEST_F(TestMockIoObjectRequest, ListSnapsTruncateFromBelowSizeAtStart) {
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockTestImageCtx mock_image_ctx(*ictx);
+  mock_image_ctx.snaps = {3, 4, 5};
+
+  librados::snap_set_t snap_set;
+  snap_set.seq = 5;
+  librados::clone_info_t clone_info;
+
+  clone_info.cloneid = 3;
+  clone_info.snaps = {3};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 512}};
+  clone_info.size = 512;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = 4;
+  clone_info.snaps = {4};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 1536}};
+  clone_info.size = 2048;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = 5;
+  clone_info.snaps = {5};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 1024}};
+  clone_info.size = 1536;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = CEPH_NOSNAP;
+  clone_info.snaps = {};
+  clone_info.overlap = {};
+  clone_info.size = 1024;
+  snap_set.clones.push_back(clone_info);
+
+  expect_list_snaps(mock_image_ctx, snap_set, 0);
+
+  {
+    SnapshotDelta snapshot_delta;
+    C_SaferCond ctx;
+    auto req = MockObjectListSnapsRequest::create(
+      &mock_image_ctx, 0, {{0, mock_image_ctx.layout.object_size}},
+      {4, CEPH_NOSNAP}, 0, {}, &snapshot_delta, &ctx);
+    req->send();
+    ASSERT_EQ(0, ctx.wait());
+
+    SnapshotDelta expected_snapshot_delta;
+    expected_snapshot_delta[{CEPH_NOSNAP,CEPH_NOSNAP}].insert(
+      1024, 1024, {SPARSE_EXTENT_STATE_ZEROED, 1024});
+    EXPECT_EQ(expected_snapshot_delta, snapshot_delta);
+  }
+
+  expect_list_snaps(mock_image_ctx, snap_set, 0);
+
+  {
+    SnapshotDelta snapshot_delta;
+    C_SaferCond ctx;
+    auto req = MockObjectListSnapsRequest::create(
+      &mock_image_ctx, 0, {{0, mock_image_ctx.layout.object_size}},
+      {3, 4, 5, CEPH_NOSNAP}, 0, {}, &snapshot_delta, &ctx);
+    req->send();
+    ASSERT_EQ(0, ctx.wait());
+
+    SnapshotDelta expected_snapshot_delta;
+    expected_snapshot_delta[{4,4}].insert(
+      512, 1536, {SPARSE_EXTENT_STATE_DATA, 1536});
+    expected_snapshot_delta[{5,5}].insert(
+      1536, 512, {SPARSE_EXTENT_STATE_ZEROED, 512});
+    expected_snapshot_delta[{CEPH_NOSNAP,CEPH_NOSNAP}].insert(
+      1024, 512, {SPARSE_EXTENT_STATE_ZEROED, 512});
+    EXPECT_EQ(expected_snapshot_delta, snapshot_delta);
+  }
+}
+
+TEST_F(TestMockIoObjectRequest, ListSnapsTruncateStraddlingSizeAtStart) {
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockTestImageCtx mock_image_ctx(*ictx);
+  mock_image_ctx.snaps = {3, 4, 5};
+
+  librados::snap_set_t snap_set;
+  snap_set.seq = 5;
+  librados::clone_info_t clone_info;
+
+  clone_info.cloneid = 3;
+  clone_info.snaps = {3};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 512}};
+  clone_info.size = 512;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = 4;
+  clone_info.snaps = {4};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 2048}};
+  clone_info.size = 2048;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = 5;
+  clone_info.snaps = {5};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 1536}};
+  clone_info.size = 3072;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = CEPH_NOSNAP;
+  clone_info.snaps = {};
+  clone_info.overlap = {};
+  clone_info.size = 1536;
+  snap_set.clones.push_back(clone_info);
+
+  expect_list_snaps(mock_image_ctx, snap_set, 0);
+
+  {
+    SnapshotDelta snapshot_delta;
+    C_SaferCond ctx;
+    auto req = MockObjectListSnapsRequest::create(
+      &mock_image_ctx, 0, {{0, mock_image_ctx.layout.object_size}},
+      {4, CEPH_NOSNAP}, 0, {}, &snapshot_delta, &ctx);
+    req->send();
+    ASSERT_EQ(0, ctx.wait());
+
+    SnapshotDelta expected_snapshot_delta;
+    expected_snapshot_delta[{CEPH_NOSNAP,CEPH_NOSNAP}].insert(
+      1536, 512, {SPARSE_EXTENT_STATE_ZEROED, 512});
+    EXPECT_EQ(expected_snapshot_delta, snapshot_delta);
+  }
+
+  expect_list_snaps(mock_image_ctx, snap_set, 0);
+
+  {
+    SnapshotDelta snapshot_delta;
+    C_SaferCond ctx;
+    auto req = MockObjectListSnapsRequest::create(
+      &mock_image_ctx, 0, {{0, mock_image_ctx.layout.object_size}},
+      {3, 4, 5, CEPH_NOSNAP}, 0, {}, &snapshot_delta, &ctx);
+    req->send();
+    ASSERT_EQ(0, ctx.wait());
+
+    SnapshotDelta expected_snapshot_delta;
+    expected_snapshot_delta[{4,4}].insert(
+      512, 1536, {SPARSE_EXTENT_STATE_DATA, 1536});
+    expected_snapshot_delta[{5,5}].insert(
+      2048, 1024, {SPARSE_EXTENT_STATE_DATA, 1024});
+    expected_snapshot_delta[{CEPH_NOSNAP,CEPH_NOSNAP}].insert(
+      1536, 1536, {SPARSE_EXTENT_STATE_ZEROED, 1536});
+    EXPECT_EQ(expected_snapshot_delta, snapshot_delta);
+  }
+}
+
+TEST_F(TestMockIoObjectRequest, ListSnapsTruncateToSizeAtStart) {
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockTestImageCtx mock_image_ctx(*ictx);
+  mock_image_ctx.snaps = {3, 4, 5};
+
+  librados::snap_set_t snap_set;
+  snap_set.seq = 5;
+  librados::clone_info_t clone_info;
+
+  clone_info.cloneid = 3;
+  clone_info.snaps = {3};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 512}};
+  clone_info.size = 512;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = 4;
+  clone_info.snaps = {4};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 2048}};
+  clone_info.size = 2048;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = 5;
+  clone_info.snaps = {5};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 2048}};
+  clone_info.size = 3072;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = CEPH_NOSNAP;
+  clone_info.snaps = {};
+  clone_info.overlap = {};
+  clone_info.size = 2048;
+  snap_set.clones.push_back(clone_info);
+
+  expect_list_snaps(mock_image_ctx, snap_set, 0);
+
+  {
+    SnapshotDelta snapshot_delta;
+    C_SaferCond ctx;
+    auto req = MockObjectListSnapsRequest::create(
+      &mock_image_ctx, 0, {{0, mock_image_ctx.layout.object_size}},
+      {4, CEPH_NOSNAP}, 0, {}, &snapshot_delta, &ctx);
+    req->send();
+    ASSERT_EQ(0, ctx.wait());
+
+    SnapshotDelta expected_snapshot_delta;
+    EXPECT_EQ(expected_snapshot_delta, snapshot_delta);
+  }
+
+  expect_list_snaps(mock_image_ctx, snap_set, 0);
+
+  {
+    SnapshotDelta snapshot_delta;
+    C_SaferCond ctx;
+    auto req = MockObjectListSnapsRequest::create(
+      &mock_image_ctx, 0, {{0, mock_image_ctx.layout.object_size}},
+      {3, 4, 5, CEPH_NOSNAP}, 0, {}, &snapshot_delta, &ctx);
+    req->send();
+    ASSERT_EQ(0, ctx.wait());
+
+    SnapshotDelta expected_snapshot_delta;
+    expected_snapshot_delta[{4,4}].insert(
+      512, 1536, {SPARSE_EXTENT_STATE_DATA, 1536});
+    expected_snapshot_delta[{5,5}].insert(
+      2048, 1024, {SPARSE_EXTENT_STATE_DATA, 1024});
+    expected_snapshot_delta[{CEPH_NOSNAP,CEPH_NOSNAP}].insert(
+      2048, 1024, {SPARSE_EXTENT_STATE_ZEROED, 1024});
+    EXPECT_EQ(expected_snapshot_delta, snapshot_delta);
+  }
+}
+
+TEST_F(TestMockIoObjectRequest, ListSnapsTruncateToAboveSizeAtStart) {
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockTestImageCtx mock_image_ctx(*ictx);
+  mock_image_ctx.snaps = {3, 4, 5};
+
+  librados::snap_set_t snap_set;
+  snap_set.seq = 5;
+  librados::clone_info_t clone_info;
+
+  clone_info.cloneid = 3;
+  clone_info.snaps = {3};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 512}};
+  clone_info.size = 512;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = 4;
+  clone_info.snaps = {4};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 2048}};
+  clone_info.size = 2048;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = 5;
+  clone_info.snaps = {5};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{{0, 2560}};
+  clone_info.size = 3072;
+  snap_set.clones.push_back(clone_info);
+
+  clone_info.cloneid = CEPH_NOSNAP;
+  clone_info.snaps = {};
+  clone_info.overlap = {};
+  clone_info.size = 2560;
+  snap_set.clones.push_back(clone_info);
+
+  expect_list_snaps(mock_image_ctx, snap_set, 0);
+
+  {
+    SnapshotDelta snapshot_delta;
+    C_SaferCond ctx;
+    auto req = MockObjectListSnapsRequest::create(
+      &mock_image_ctx, 0, {{0, mock_image_ctx.layout.object_size}},
+      {4, CEPH_NOSNAP}, 0, {}, &snapshot_delta, &ctx);
+    req->send();
+    ASSERT_EQ(0, ctx.wait());
+
+    SnapshotDelta expected_snapshot_delta;
+    expected_snapshot_delta[{CEPH_NOSNAP,CEPH_NOSNAP}].insert(
+      2048, 512, {SPARSE_EXTENT_STATE_DATA, 512});
+    EXPECT_EQ(expected_snapshot_delta, snapshot_delta);
+  }
+
+  expect_list_snaps(mock_image_ctx, snap_set, 0);
+
+  {
+    SnapshotDelta snapshot_delta;
+    C_SaferCond ctx;
+    auto req = MockObjectListSnapsRequest::create(
+      &mock_image_ctx, 0, {{0, mock_image_ctx.layout.object_size}},
+      {3, 4, 5, CEPH_NOSNAP}, 0, {}, &snapshot_delta, &ctx);
+    req->send();
+    ASSERT_EQ(0, ctx.wait());
+
+    SnapshotDelta expected_snapshot_delta;
+    expected_snapshot_delta[{4,4}].insert(
+      512, 1536, {SPARSE_EXTENT_STATE_DATA, 1536});
+    expected_snapshot_delta[{5,5}].insert(
+      2048, 1024, {SPARSE_EXTENT_STATE_DATA, 1024});
+    expected_snapshot_delta[{CEPH_NOSNAP,CEPH_NOSNAP}].insert(
+      2560, 512, {SPARSE_EXTENT_STATE_ZEROED, 512});
+    EXPECT_EQ(expected_snapshot_delta, snapshot_delta);
+  }
+}
+
 TEST_F(TestMockIoObjectRequest, ListSnapsENOENT) {
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));