rbd-mirror: image sync object copy was not properly mapping snapshots

When the snapshot sequence is out-of-sync between remote and local
clusters (expected), the objects would not be written with the
correct snapshot context.

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

diff --git a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
index 23497bab1cb7df793502941b51edce873bf4d01e..38567ce77de1d8878deeb9c34df8493ff8fb0b2c 100644 (file)
--- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
+++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
@@ -35,6 +35,11 @@ ObjectCopyRequest<I>::ObjectCopyRequest(I *local_image_ctx, I *remote_image_ctx,
 
   m_remote_io_ctx.dup(m_remote_image_ctx->data_ctx);
   m_remote_oid = m_remote_image_ctx->get_object_name(object_number);
+
+  CephContext *cct = m_local_image_ctx->cct;
+  ldout(cct, 20) << ": "
+                 << "remote_oid=" << m_remote_oid << ", "
+                 << "local_oid=" << m_local_oid << dendl;
 }
 
 template <typename I>
@@ -97,11 +102,8 @@ void ObjectCopyRequest<I>::send_read_object() {
   assert(!sync_ops.empty());
 
   // map the sync op start snap id back to the necessary read snap id
-  auto snap_map_it = m_snap_map->upper_bound(
-    m_snap_sync_ops.begin()->first);
-  assert(snap_map_it != m_snap_map->end());
-  librados::snap_t snap_seq = snap_map_it->first;
-  m_remote_io_ctx.snap_set_read(snap_seq);
+  librados::snap_t remote_snap_seq = m_snap_sync_ops.begin()->first;
+  m_remote_io_ctx.snap_set_read(remote_snap_seq);
 
   bool read_required = false;
   librados::ObjectReadOperation op;
@@ -109,7 +111,7 @@ void ObjectCopyRequest<I>::send_read_object() {
     switch (std::get<0>(sync_op)) {
     case SYNC_OP_TYPE_WRITE:
       if (!read_required) {
-        ldout(cct, 20) << ": snap_seq=" << snap_seq << dendl;
+        ldout(cct, 20) << ": remote_snap_seq=" << remote_snap_seq << dendl;
         read_required = true;
       }
 
@@ -154,18 +156,26 @@ void ObjectCopyRequest<I>::handle_read_object(int r) {
 template <typename I>
 void ObjectCopyRequest<I>::send_write_object() {
   // retrieve the local snap context for the op
-  SnapIds snap_ids;
-  librados::snap_t snap_seq = m_snap_sync_ops.begin()->first;
-  if (snap_seq != 0) {
-    auto snap_map_it = m_snap_map->find(snap_seq);
+  SnapIds local_snap_ids;
+  librados::snap_t local_snap_seq = 0;
+  librados::snap_t remote_snap_seq = m_snap_sync_ops.begin()->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());
-    snap_ids = snap_map_it->second;
+
+    // write snapshot context should be before actual snapshot
+    if (snap_map_it != m_snap_map->begin()) {
+      --snap_map_it;
+      assert(!snap_map_it->second.empty());
+      local_snap_seq = snap_map_it->second.front();
+      local_snap_ids = snap_map_it->second;
+    }
   }
 
   CephContext *cct = m_local_image_ctx->cct;
   ldout(cct, 20) << ": "
-                 << "snap_seq=" << snap_seq << ", "
-                 << "snaps=" << snap_ids << dendl;
+                 << "local_snap_seq=" << local_snap_seq << ", "
+                 << "local_snaps=" << local_snap_ids << dendl;
 
   auto &sync_ops = m_snap_sync_ops.begin()->second;
   assert(!sync_ops.empty());
@@ -193,8 +203,8 @@ void ObjectCopyRequest<I>::send_write_object() {
 
   librados::AioCompletion *comp = create_rados_safe_callback<
     ObjectCopyRequest<I>, &ObjectCopyRequest<I>::handle_write_object>(this);
-  int r = m_local_io_ctx.aio_operate(m_local_oid, comp, &op, snap_seq,
-                                     snap_ids);
+  int r = m_local_io_ctx.aio_operate(m_local_oid, comp, &op, local_snap_seq,
+                                     local_snap_ids);
   assert(r == 0);
   comp->release();
 }
@@ -241,7 +251,7 @@ void ObjectCopyRequest<I>::send_update_object_map() {
 
   CephContext *cct = m_local_image_ctx->cct;
   ldout(cct, 20) << ": "
-                 << "snap_id=" << snap_object_state.first << ", "
+                 << "local_snap_id=" << snap_object_state.first << ", "
                  << "object_state=" << static_cast<uint32_t>(
                       snap_object_state.second)
                  << dendl;
@@ -277,21 +287,22 @@ void ObjectCopyRequest<I>::compute_diffs() {
 
   uint64_t prev_end_size = 0;
   bool prev_exists = false;
-  librados::snap_t start_snap_id = 0;
-  librados::snap_t end_snap_id;
+  librados::snap_t start_remote_snap_id = 0;
   for (auto &pair : *m_snap_map) {
     assert(!pair.second.empty());
-    end_snap_id = pair.second.front();
+    librados::snap_t end_remote_snap_id = pair.first;
+    librados::snap_t end_local_snap_id = pair.second.front();
 
     interval_set<uint64_t> diff;
     uint64_t end_size;
     bool exists;
-    calc_snap_set_diff(cct, m_snap_set, start_snap_id, end_snap_id, &diff,
-                       &end_size, &exists);
+    calc_snap_set_diff(cct, m_snap_set, start_remote_snap_id,
+                       end_remote_snap_id, &diff, &end_size, &exists);
 
     ldout(cct, 20) << ": "
-                   << "start_snap=" << start_snap_id << ", "
-                   << "end_snap_id=" << end_snap_id << ", "
+                   << "start_remote_snap=" << start_remote_snap_id << ", "
+                   << "end_remote_snap_id=" << end_remote_snap_id << ", "
+                   << "end_local_snap_id=" << end_local_snap_id << ", "
                    << "diff=" << diff << ", "
                    << "end_size=" << end_size << ", "
                    << "exists=" << exists << dendl;
@@ -315,35 +326,36 @@ void ObjectCopyRequest<I>::compute_diffs() {
             diff.empty() && end_size == prev_end_size) {
           object_state = OBJECT_EXISTS_CLEAN;
         }
-        m_snap_object_states[end_snap_id] = object_state;
+        m_snap_object_states[end_local_snap_id] = object_state;
       }
 
       // object write/zero, or truncate
       for (auto it = diff.begin(); it != diff.end(); ++it) {
         ldout(cct, 20) << ": read/write op: " << it.get_start() << "~"
                        << it.get_len() << dendl;
-        m_snap_sync_ops[start_snap_id].emplace_back(SYNC_OP_TYPE_WRITE,
-                                                    it.get_start(),
-                                                    it.get_len(),
-                                                    bufferlist());
+        m_snap_sync_ops[end_remote_snap_id].emplace_back(SYNC_OP_TYPE_WRITE,
+                                                         it.get_start(),
+                                                         it.get_len(),
+                                                         bufferlist());
       }
       if (end_size < prev_end_size) {
         ldout(cct, 20) << ": trunc op: " << end_size << dendl;
-        m_snap_sync_ops[start_snap_id].emplace_back(SYNC_OP_TYPE_TRUNC,
-                                                    end_size, 0U, bufferlist());
+        m_snap_sync_ops[end_remote_snap_id].emplace_back(SYNC_OP_TYPE_TRUNC,
+                                                         end_size, 0U,
+                                                         bufferlist());
       }
     } else {
       if (prev_exists) {
         // object remove
         ldout(cct, 20) << ": remove op" << dendl;
-        m_snap_sync_ops[start_snap_id].emplace_back(SYNC_OP_TYPE_REMOVE, 0U, 0U,
-                                                    bufferlist());
+        m_snap_sync_ops[end_remote_snap_id].emplace_back(SYNC_OP_TYPE_REMOVE,
+                                                         0U, 0U, bufferlist());
       }
     }
 
     prev_end_size = end_size;
     prev_exists = exists;
-    start_snap_id = end_snap_id;
+    start_remote_snap_id = end_remote_snap_id;
   }
 }