rbd-mirror: object copy should always reference valid snapshots

If a remote snapshot is deleted while an image sync is in-progress,
associate the read request against the most recent, valid remote
snapshot for a given snapshot object clone.

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

diff --git a/src/librados/snap_set_diff.cc b/src/librados/snap_set_diff.cc
index 9fb166f5817c5cdc8b7703b401657da710d41cec..fcf42d080dabc17100b7ee976036c5ca4807bf84 100644 (file)
--- a/src/librados/snap_set_diff.cc
+++ b/src/librados/snap_set_diff.cc
@@ -17,7 +17,7 @@
 void calc_snap_set_diff(CephContext *cct, const librados::snap_set_t& snap_set,
                        librados::snap_t start, librados::snap_t end,
                        interval_set<uint64_t> *diff, uint64_t *end_size,
-                        bool *end_exists)
+                        bool *end_exists, librados::snap_t *clone_end_snap_id)
 {
   ldout(cct, 10) << "calc_snap_set_diff start " << start << " end " << end
                 << ", snap_set seq " << snap_set.seq << dendl;
@@ -26,6 +26,7 @@ void calc_snap_set_diff(CephContext *cct, const librados::snap_set_t& snap_set,
   diff->clear();
   *end_size = 0;
   *end_exists = false;
+  *clone_end_snap_id = 0;
 
   for (vector<librados::clone_info_t>::const_iterator r = snap_set.clones.begin();
        r != snap_set.clones.end();
@@ -79,6 +80,7 @@ void calc_snap_set_diff(CephContext *cct, const librados::snap_set_t& snap_set,
     if (end <= b) {
       ldout(cct, 20) << " end" << dendl;
       *end_exists = true;
+      *clone_end_snap_id = b;
       break;
     }
 

diff --git a/src/librados/snap_set_diff.h b/src/librados/snap_set_diff.h
index acb71965b52cbfd6387344f204cae90ccb425fc9..aba03ee317ec4ab76a31390b7bbfc70f0fd8bd10 100644 (file)
--- a/src/librados/snap_set_diff.h
+++ b/src/librados/snap_set_diff.h
@@ -12,6 +12,6 @@ void calc_snap_set_diff(CephContext *cct,
                        const librados::snap_set_t& snap_set,
                        librados::snap_t start, librados::snap_t end,
                        interval_set<uint64_t> *diff, uint64_t *end_size,
-                       bool *end_exists);
+                       bool *end_exists, librados::snap_t *clone_end_snap_id);
 
 #endif

diff --git a/src/librbd/DiffIterate.cc b/src/librbd/DiffIterate.cc
index c3b9511330aac18fd737ab95117cd64c81a9fb68..0ef400a3ef7ec290ed5284d8b6838eaaaa301c2d 100644 (file)
--- a/src/librbd/DiffIterate.cc
+++ b/src/librbd/DiffIterate.cc
@@ -125,9 +125,10 @@ private:
     interval_set<uint64_t> diff;
     uint64_t end_size;
     bool end_exists;
+    librados::snap_t clone_end_snap_id;
     calc_snap_set_diff(cct, m_snap_set, m_diff_context.from_snap_id,
                        m_diff_context.end_snap_id, &diff, &end_size,
-                       &end_exists);
+                       &end_exists, &clone_end_snap_id);
     ldout(cct, 20) << "  diff " << diff << " end_exists=" << end_exists
                    << dendl;
     if (diff.empty()) {

diff --git a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
index 900a5f7162400ee40d2e27e764a6c53a638f46e8..5ebfa5c38609b278a7a093de9add3a66c6d554e3 100644 (file)
--- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
+++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
@@ -98,7 +98,7 @@ void ObjectCopyRequest<I>::send_read_object() {
   assert(!sync_ops.empty());
 
   // map the sync op start snap id back to the necessary read snap id
-  librados::snap_t remote_snap_seq = m_snap_sync_ops.begin()->first;
+  librados::snap_t remote_snap_seq = m_snap_sync_ops.begin()->first.second;
   m_remote_io_ctx.snap_set_read(remote_snap_seq);
 
   bool read_required = false;
@@ -154,7 +154,7 @@ void ObjectCopyRequest<I>::send_write_object() {
   // retrieve the local snap context for the op
   SnapIds local_snap_ids;
   librados::snap_t local_snap_seq = 0;
-  librados::snap_t remote_snap_seq = m_snap_sync_ops.begin()->first;
+  librados::snap_t remote_snap_seq = m_snap_sync_ops.begin()->first.first;
   if (remote_snap_seq != 0) {
     auto snap_map_it = m_snap_map->find(remote_snap_seq);
     assert(snap_map_it != m_snap_map->end());
@@ -313,6 +313,7 @@ template <typename I>
 void ObjectCopyRequest<I>::compute_diffs() {
   CephContext *cct = m_local_image_ctx->cct;
 
+  librados::snap_t remote_sync_pont_snap_id = m_snap_map->rbegin()->first;
   uint64_t prev_end_size = 0;
   bool prev_exists = false;
   librados::snap_t start_remote_snap_id = 0;
@@ -324,12 +325,15 @@ void ObjectCopyRequest<I>::compute_diffs() {
     interval_set<uint64_t> diff;
     uint64_t end_size;
     bool exists;
+    librados::snap_t clone_end_snap_id;
     calc_snap_set_diff(cct, m_snap_set, start_remote_snap_id,
-                       end_remote_snap_id, &diff, &end_size, &exists);
+                       end_remote_snap_id, &diff, &end_size, &exists,
+                       &clone_end_snap_id);
 
     dout(20) << ": "
              << "start_remote_snap=" << start_remote_snap_id << ", "
              << "end_remote_snap_id=" << end_remote_snap_id << ", "
+             << "clone_end_snap_id=" << clone_end_snap_id << ", "
              << "end_local_snap_id=" << end_local_snap_id << ", "
              << "diff=" << diff << ", "
              << "end_size=" << end_size << ", "
@@ -350,32 +354,44 @@ void ObjectCopyRequest<I>::compute_diffs() {
         uint8_t object_state = OBJECT_EXISTS;
         if (m_local_image_ctx->test_features(RBD_FEATURE_FAST_DIFF,
                                              m_local_image_ctx->snap_lock) &&
-            diff.empty() && end_size == prev_end_size) {
+            prev_exists && diff.empty() && end_size == prev_end_size) {
           object_state = OBJECT_EXISTS_CLEAN;
         }
         m_snap_object_states[end_local_snap_id] = object_state;
       }
 
+      // reads should be issued against the newest (existing) snapshot within
+      // the associated snapshot object clone. writes should be issued
+      // against the oldest snapshot in the snap_map.
+      assert(clone_end_snap_id >= end_remote_snap_id);
+      if (clone_end_snap_id > remote_sync_pont_snap_id) {
+        // do not read past the sync point snapshot
+        clone_end_snap_id = remote_sync_pont_snap_id;
+      }
+
       // object write/zero, or truncate
+      // NOTE: a single snapshot clone might represent multiple snapshots, but
+      // the write/zero and truncate ops will only be associated with the first
+      // snapshot encountered within the clone since the diff will be empty for
+      // subsequent snapshots and the size will remain constant for a clone.
       for (auto it = diff.begin(); it != diff.end(); ++it) {
         dout(20) << ": read/write op: " << it.get_start() << "~"
                  << it.get_len() << dendl;
-        m_snap_sync_ops[end_remote_snap_id].emplace_back(SYNC_OP_TYPE_WRITE,
-                                                         it.get_start(),
-                                                         it.get_len());
+        m_snap_sync_ops[{end_remote_snap_id, clone_end_snap_id}].emplace_back(
+          SYNC_OP_TYPE_WRITE, it.get_start(), it.get_len());
       }
       if (end_size < prev_end_size) {
         dout(20) << ": trunc op: " << end_size << dendl;
-        m_snap_sync_ops[end_remote_snap_id].emplace_back(SYNC_OP_TYPE_TRUNC,
-                                                         end_size, 0U);
+        m_snap_sync_ops[{end_remote_snap_id, clone_end_snap_id}].emplace_back(
+          SYNC_OP_TYPE_TRUNC, end_size, 0U);
       }
       m_snap_object_sizes[end_remote_snap_id] = end_size;
     } else {
       if (prev_exists) {
         // object remove
         dout(20) << ": remove op" << dendl;
-        m_snap_sync_ops[end_remote_snap_id].emplace_back(SYNC_OP_TYPE_REMOVE,
-                                                         0U, 0U);
+        m_snap_sync_ops[{end_remote_snap_id, end_remote_snap_id}].emplace_back(
+          SYNC_OP_TYPE_REMOVE, 0U, 0U);
       }
     }
 

diff --git a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.h b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.h
index 2ade599787f11fa607933538b09f6c9b51f78a6f..17545d301987bd8c5fd9cbfe17398335571be8eb 100644 (file)
--- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.h
+++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.h
@@ -96,7 +96,8 @@ private:
   };
 
   typedef std::list<SyncOp> SyncOps;
-  typedef std::map<librados::snap_t, SyncOps> SnapSyncOps;
+  typedef std::pair<librados::snap_t, librados::snap_t> WriteReadSnapIds;
+  typedef std::map<WriteReadSnapIds, SyncOps> SnapSyncOps;
   typedef std::map<librados::snap_t, uint8_t> SnapObjectStates;
   typedef std::map<librados::snap_t, uint64_t> SnapObjectSizes;