From 2fb99b5ed39c36879d1b6180ac47629bc5a3b315 Mon Sep 17 00:00:00 2001 From: Mykola Golub Date: Wed, 7 Feb 2018 13:09:54 +0200 Subject: [PATCH] rbd-mirror: fix potential infinite loop when formatting status message The improvements include: - tag_tid values should always be increasing, so loop only if master.tag_tid > mirror_tag_tid in calculate_behind_master_or_send_update; - in send_update_tag_cache don't refetch a tag if it is already in the cache; - make fake tags with tag_data.predecessor.tag_tid set to zero; - make sure the new tag is inserted to the cache if an old entry with this id happens to exist. Fixes: http://tracker.ceph.com/issues/22932 Signed-off-by: Mykola Golub --- .../image_replayer/ReplayStatusFormatter.cc | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/tools/rbd_mirror/image_replayer/ReplayStatusFormatter.cc b/src/tools/rbd_mirror/image_replayer/ReplayStatusFormatter.cc index 7b01fb5b8382d..3174a32d1dc2c 100644 --- a/src/tools/rbd_mirror/image_replayer/ReplayStatusFormatter.cc +++ b/src/tools/rbd_mirror/image_replayer/ReplayStatusFormatter.cc @@ -112,7 +112,7 @@ bool ReplayStatusFormatter::calculate_behind_master_or_send_update() { cls::journal::ObjectPosition master = m_master_position; uint64_t mirror_tag_tid = m_mirror_position.tag_tid; - while (master.tag_tid != mirror_tag_tid) { + while (master.tag_tid > mirror_tag_tid) { auto tag_it = m_tag_cache.find(master.tag_tid); if (tag_it == m_tag_cache.end()) { send_update_tag_cache(master.tag_tid, mirror_tag_tid); @@ -120,10 +120,12 @@ bool ReplayStatusFormatter::calculate_behind_master_or_send_update() { } librbd::journal::TagData &tag_data = tag_it->second; m_entries_behind_master += master.entry_tid; - master = cls::journal::ObjectPosition(0, tag_data.predecessor.tag_tid, - tag_data.predecessor.entry_tid); + master = {0, tag_data.predecessor.tag_tid, tag_data.predecessor.entry_tid}; + } + if (master.tag_tid == mirror_tag_tid && + master.entry_tid > m_mirror_position.entry_tid) { + m_entries_behind_master += master.entry_tid - m_mirror_position.entry_tid; } - m_entries_behind_master += master.entry_tid - m_mirror_position.entry_tid; dout(20) << "clearing tags not needed any more (below mirror position)" << dendl; @@ -152,11 +154,8 @@ bool ReplayStatusFormatter::calculate_behind_master_or_send_update() { template void ReplayStatusFormatter::send_update_tag_cache(uint64_t master_tag_tid, uint64_t mirror_tag_tid) { - - dout(20) << "master_tag_tid=" << master_tag_tid << ", mirror_tag_tid=" - << mirror_tag_tid << dendl; - - if (master_tag_tid == mirror_tag_tid) { + if (master_tag_tid <= mirror_tag_tid || + m_tag_cache.find(master_tag_tid) != m_tag_cache.end()) { Context *on_finish = nullptr; { Mutex::Locker locker(m_lock); @@ -168,6 +167,9 @@ void ReplayStatusFormatter::send_update_tag_cache(uint64_t master_tag_tid, return; } + dout(20) << "master_tag_tid=" << master_tag_tid << ", mirror_tag_tid=" + << mirror_tag_tid << dendl; + FunctionContext *ctx = new FunctionContext( [this, master_tag_tid, mirror_tag_tid](int r) { handle_update_tag_cache(master_tag_tid, mirror_tag_tid, r); @@ -201,16 +203,12 @@ void ReplayStatusFormatter::handle_update_tag_cache(uint64_t master_tag_tid, tag_data.predecessor.mirror_uuid != librbd::Journal<>::ORPHAN_MIRROR_UUID) { dout(20) << "hit remote image non-primary epoch" << dendl; - tag_data.predecessor.tag_tid = mirror_tag_tid; - } else if (tag_data.predecessor.tag_tid == 0) { - // We failed. Don't consider this fatal, just terminate retrieving. - dout(20) << "making fake tag" << dendl; - tag_data.predecessor.tag_tid = mirror_tag_tid; + tag_data.predecessor = {}; } dout(20) << "decoded tag " << master_tag_tid << ": " << tag_data << dendl; - m_tag_cache.insert(std::make_pair(master_tag_tid, tag_data)); + m_tag_cache[master_tag_tid] = tag_data; send_update_tag_cache(tag_data.predecessor.tag_tid, mirror_tag_tid); } -- 2.39.5