From: Jason Dillaman Date: Wed, 4 May 2016 14:23:49 +0000 (-0400) Subject: rbd-mirror: image sync object copy was not properly mapping snapshots X-Git-Tag: v10.2.1~14^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=32189573c2da9e1922b6bfae232a62a909797e1a;p=ceph.git 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 (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 23497bab1cb7..38567ce77de1 100644 --- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc +++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc @@ -35,6 +35,11 @@ ObjectCopyRequest::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 @@ -97,11 +102,8 @@ void ObjectCopyRequest::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::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::handle_read_object(int r) { template void ObjectCopyRequest::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::send_write_object() { librados::AioCompletion *comp = create_rados_safe_callback< ObjectCopyRequest, &ObjectCopyRequest::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::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( snap_object_state.second) << dendl; @@ -277,21 +287,22 @@ void ObjectCopyRequest::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 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::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; } }