librbd/io: track object non-existence when computing snapshot deltas

Re-use the existing DNE state to track whether or not the object
already exists when computing snapshot deltas from an arbitrary
set of snapshots. Previously, the non-existence of the object was
only computed for snap id 0 for tracking whiteouts. In a future
commit, the deep-copy object-copy state machine will be able to
properly update the object-map state to indicate exists clean
vs non-existent state.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit 89d2a13db67945bfd7c6d6fe79584f00574b8ba3)

diff --git a/src/librbd/api/DiffIterate.cc b/src/librbd/api/DiffIterate.cc
index 70f1074e8fadd3f6cc4fe7ba2e26efead7d9e05e..dcd07ea56a3dde924aeec10b48c5a5dcf2fe514f 100644 (file)
--- a/src/librbd/api/DiffIterate.cc
+++ b/src/librbd/api/DiffIterate.cc
@@ -122,8 +122,9 @@ private:
         auto state = snapshot_extent.get_val().state;
 
         // ignore DNE object (and parent)
-        if (key == io::WriteReadSnapIds{0, 0} &&
-            state != io::SPARSE_EXTENT_STATE_DATA) {
+        if ((state == io::SPARSE_EXTENT_STATE_DNE) ||
+            (key == io::INITIAL_WRITE_READ_SNAP_IDS &&
+             state == io::SPARSE_EXTENT_STATE_ZEROED)) {
           continue;
         }
 

diff --git a/src/librbd/deep_copy/ObjectCopyRequest.cc b/src/librbd/deep_copy/ObjectCopyRequest.cc
index 80b06c15cfbbe1bc33a66a4b38d6c20ab826cd83..3c8c596b19db139027ac720270be0d0c7c036082 100644 (file)
--- a/src/librbd/deep_copy/ObjectCopyRequest.cc
+++ b/src/librbd/deep_copy/ObjectCopyRequest.cc
@@ -465,7 +465,7 @@ void ObjectCopyRequest<I>::compute_read_ops() {
     io::WriteReadSnapIds write_read_snap_ids{key};
 
     // advance the src write snap id to the first valid snap id
-    if (write_read_snap_ids != io::INITIAL_WRITE_READ_SNAP_IDS) {
+    if (write_read_snap_ids.first > m_src_snap_id_start) {
       // don't attempt to read from snapshots that shouldn't exist in
       // case the OSD fails to give a correct snap list
       auto snap_map_it = m_snap_map.find(write_read_snap_ids.first);
@@ -485,10 +485,10 @@ void ObjectCopyRequest<I>::compute_read_ops() {
       auto state = image_interval.get_val().state;
       switch (state) {
       case io::SPARSE_EXTENT_STATE_DNE:
-        ceph_assert(write_read_snap_ids == io::INITIAL_WRITE_READ_SNAP_IDS);
-        if (read_from_parent) {
-          // special-case for DNE object-extents since when flattening we need
-          // to read data from the parent images extents
+        if (write_read_snap_ids == io::INITIAL_WRITE_READ_SNAP_IDS &&
+            read_from_parent) {
+          // special-case for DNE initial object-extents since when flattening
+          // we need to read data from the parent images extents
           ldout(m_cct, 20) << "DNE extent: "
                            << image_interval.get_off() << "~"
                            << image_interval.get_len() << dendl;
@@ -626,14 +626,15 @@ void ObjectCopyRequest<I>::compute_zero_ops() {
       auto state = image_interval.get_val().state;
       switch (state) {
       case io::SPARSE_EXTENT_STATE_ZEROED:
-        if (write_read_snap_ids != io::WriteReadSnapIds{0, 0}) {
+        if (write_read_snap_ids.first > m_src_snap_id_start) {
           ldout(m_cct, 20) << "zeroed extent: "
                            << "src_snap_seq=" << src_snap_seq << " "
                            << image_interval.get_off() << "~"
                            << image_interval.get_len() << dendl;
           m_dst_zero_interval[src_snap_seq].union_insert(
             image_interval.get_off(), image_interval.get_len());
-        } else if (hide_parent) {
+        } else if (write_read_snap_ids == io::INITIAL_WRITE_READ_SNAP_IDS &&
+                   hide_parent) {
           auto first_src_snap_id = m_snap_map.begin()->first;
           ldout(m_cct, 20) << "zeroed (hide parent) extent: "
                            << "src_snap_seq=" << first_src_snap_id << "  "

diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc
index 2bb3f52345037799d24bc46ca082f82ab83035e7..e4ee152cf36f1abae4fc4a2d606920f3d46920ca 100644 (file)
--- a/src/librbd/io/ObjectRequest.cc
+++ b/src/librbd/io/ObjectRequest.cc
@@ -784,9 +784,14 @@ void ObjectListSnapsRequest<I>::handle_list_snaps(int r) {
   m_snapshot_delta->clear();
   auto& snapshot_delta = *m_snapshot_delta;
 
+  ceph_assert(!m_snap_ids.empty());
+  librados::snap_t start_snap_id = 0;
+  librados::snap_t first_snap_id = *m_snap_ids.begin();
+  librados::snap_t last_snap_id = *m_snap_ids.rbegin();
+
   if (r == -ENOENT) {
     // the object does not exist -- mark the missing extents
-    zero_initial_extent(true);
+    zero_extent(first_snap_id, true);
     list_from_parent();
     return;
   } else if (r < 0) {
@@ -800,11 +805,6 @@ void ObjectListSnapsRequest<I>::handle_list_snaps(int r) {
   librados::snap_set_t snap_set;
   convert_snap_set(m_snap_set, &snap_set);
 
-  ceph_assert(!m_snap_ids.empty());
-  librados::snap_t start_snap_id = 0;
-  librados::snap_t first_snap_id = *m_snap_ids.begin();
-  librados::snap_t last_snap_id = *m_snap_ids.rbegin();
-
   bool initial_extents_written = false;
 
   interval_set<uint64_t> object_interval;
@@ -924,7 +924,7 @@ void ObjectListSnapsRequest<I>::handle_list_snaps(int r) {
 
   bool snapshot_delta_empty = snapshot_delta.empty();
   if (!initial_extents_written) {
-    zero_initial_extent(false);
+    zero_extent(first_snap_id, first_snap_id > 0);
   }
   ldout(cct, 20) << "snapshot_delta=" << snapshot_delta << dendl;
 
@@ -1003,8 +1003,7 @@ void ObjectListSnapsRequest<I>::handle_list_from_parent(int r) {
                  << "parent_snapshot_delta=" << m_parent_snapshot_delta
                  << dendl;
 
-  // ignore special-case of fully empty dataset
-  m_parent_snapshot_delta.erase(INITIAL_WRITE_READ_SNAP_IDS);
+  // ignore special-case of fully empty dataset (we ignore zeroes)
   if (m_parent_snapshot_delta.empty()) {
     this->finish(0);
     return;
@@ -1036,27 +1035,24 @@ void ObjectListSnapsRequest<I>::handle_list_from_parent(int r) {
 }
 
 template <typename I>
-void ObjectListSnapsRequest<I>::zero_initial_extent(bool dne) {
+void ObjectListSnapsRequest<I>::zero_extent(uint64_t snap_id, bool dne) {
   I *image_ctx = this->m_ictx;
   auto cct = image_ctx->cct;
 
-  ceph_assert(!m_snap_ids.empty());
-  librados::snap_t snap_id_start = *m_snap_ids.begin();
-
   // the object does not exist or is (partially) under whiteout -- mark the
   // missing extents which would be any portion of the object that does not
   // have data in the initial snapshot set
-  if ((snap_id_start == 0) &&
-       ((m_list_snaps_flags & LIST_SNAPS_FLAG_IGNORE_ZEROED_EXTENTS) == 0)) {
+  if ((m_list_snaps_flags & LIST_SNAPS_FLAG_IGNORE_ZEROED_EXTENTS) == 0) {
     interval_set<uint64_t> interval;
     for (auto [object_offset, object_length] : m_object_extents) {
       interval.insert(object_offset, object_length);
     }
 
     for (auto [offset, length] : interval) {
-      ldout(cct, 20) << "zeroing initial extent " << offset << "~" << length
-                     << dendl;
-      (*m_snapshot_delta)[INITIAL_WRITE_READ_SNAP_IDS].insert(
+      ldout(cct, 20) << "snapshot " << snap_id << ": "
+                     << (dne ? "DNE" : "zeroed") << " extent "
+                     << offset << "~" << length << dendl;
+      (*m_snapshot_delta)[{snap_id, snap_id}].insert(
         offset, length,
         SparseExtent(
           (dne ? SPARSE_EXTENT_STATE_DNE : SPARSE_EXTENT_STATE_ZEROED),

diff --git a/src/librbd/io/ObjectRequest.h b/src/librbd/io/ObjectRequest.h
index 8ba244b3d4676dd52c30cc3a4dddb1041a7911a8..89ca224cc90e75179d305183944ebd70824e1156 100644 (file)
--- a/src/librbd/io/ObjectRequest.h
+++ b/src/librbd/io/ObjectRequest.h
@@ -484,7 +484,7 @@ private:
   void list_from_parent();
   void handle_list_from_parent(int r);
 
-  void zero_initial_extent(bool dne);
+  void zero_extent(uint64_t snap_id, bool dne);
 };
 
 } // namespace io

diff --git a/src/test/librbd/io/test_mock_ObjectRequest.cc b/src/test/librbd/io/test_mock_ObjectRequest.cc
index 18069b9c99d1a9bc6508fdc32effe69e49778f8f..c9fa33c43e1cff5024a91da5afa2ef8169fe7952 100644 (file)
--- a/src/test/librbd/io/test_mock_ObjectRequest.cc
+++ b/src/test/librbd/io/test_mock_ObjectRequest.cc
@@ -1777,7 +1777,7 @@ TEST_F(TestMockIoObjectRequest, ListSnaps) {
   ASSERT_EQ(expected_snapshot_delta, snapshot_delta);
 }
 
-TEST_F(TestMockIoObjectRequest, ListSnapsDNE) {
+TEST_F(TestMockIoObjectRequest, ListSnapsENOENT) {
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
 
@@ -1800,6 +1800,43 @@ TEST_F(TestMockIoObjectRequest, ListSnapsDNE) {
   ASSERT_EQ(expected_snapshot_delta, snapshot_delta);
 }
 
+TEST_F(TestMockIoObjectRequest, ListSnapsDNE) {
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockTestImageCtx mock_image_ctx(*ictx);
+  mock_image_ctx.snaps = {2, 3, 4};
+
+  librados::snap_set_t snap_set;
+  snap_set.seq = 6;
+  librados::clone_info_t clone_info;
+
+  clone_info.cloneid = 4;
+  clone_info.snaps = {3, 4};
+  clone_info.overlap = std::vector<std::pair<uint64_t,uint64_t>>{
+    {0, 4194304}};
+  clone_info.size = 4194304;
+  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,
+    {{440320, 1024}},
+    {2, 3, 4}, 0, {}, &snapshot_delta, &ctx);
+  req->send();
+  ASSERT_EQ(0, ctx.wait());
+
+  SnapshotDelta expected_snapshot_delta;
+  expected_snapshot_delta[{2,2}].insert(
+    440320, 1024, {SPARSE_EXTENT_STATE_DNE, 1024});
+  expected_snapshot_delta[{3,4}].insert(
+    440320, 1024, {SPARSE_EXTENT_STATE_DATA, 1024});
+  ASSERT_EQ(expected_snapshot_delta, snapshot_delta);
+}
+
 TEST_F(TestMockIoObjectRequest, ListSnapsEmpty) {
   librbd::ImageCtx *ictx;
   ASSERT_EQ(0, open_image(m_image_name, &ictx));