From de4f83ec77d0c79415d310ebc33d5cf5de5aba7d Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 20 Feb 2020 12:40:15 -0500 Subject: [PATCH] rbd-mirror: expand lock scope for snapshot replayer The status callback will need access to the current state, so the lock should be held while the snapshot scan is in-progress. Signed-off-by: Jason Dillaman --- .../image_replayer/snapshot/Replayer.cc | 57 ++++++++++++------- .../image_replayer/snapshot/Replayer.h | 6 +- 2 files changed, 39 insertions(+), 24 deletions(-) diff --git a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc index 1516713b2cf..7f8b4d6017a 100644 --- a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc +++ b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.cc @@ -204,7 +204,8 @@ void Replayer::handle_refresh_local_image(int r) { template void Replayer::refresh_remote_image() { if (!m_state_builder->remote_image_ctx->state->is_refresh_required()) { - scan_local_mirror_snapshots(); + std::unique_lock locker{m_lock}; + scan_local_mirror_snapshots(&locker); return; } @@ -224,12 +225,14 @@ void Replayer::handle_refresh_remote_image(int r) { return; } - scan_local_mirror_snapshots(); + std::unique_lock locker{m_lock}; + scan_local_mirror_snapshots(&locker); } template -void Replayer::scan_local_mirror_snapshots() { - if (is_replay_interrupted()) { +void Replayer::scan_local_mirror_snapshots( + std::unique_lock* locker) { + if (is_replay_interrupted(locker)) { return; } @@ -275,12 +278,14 @@ void Replayer::scan_local_mirror_snapshots() { m_local_snap_id_end = CEPH_NOSNAP; } else { derr << "incomplete local primary snapshot" << dendl; - handle_replay_complete(-EINVAL, "incomplete local primary snapshot"); + handle_replay_complete(locker, -EINVAL, + "incomplete local primary snapshot"); return; } } else { derr << "unknown local mirror snapshot state" << dendl; - handle_replay_complete(-EINVAL, "invalid local mirror snapshot state"); + handle_replay_complete(locker, -EINVAL, + "invalid local mirror snapshot state"); return; } } @@ -293,12 +298,13 @@ void Replayer::scan_local_mirror_snapshots() { // TODO support multiple peers derr << "local image linked to unknown peer: " << m_local_mirror_snap_ns.primary_mirror_uuid << dendl; - handle_replay_complete(-EEXIST, "local image linked to unknown peer"); + handle_replay_complete(locker, -EEXIST, + "local image linked to unknown peer"); return; } else if (m_local_mirror_snap_ns.state == cls::rbd::MIRROR_SNAPSHOT_STATE_PRIMARY) { dout(5) << "local image promoted" << dendl; - handle_replay_complete(0, "force promoted"); + handle_replay_complete(locker, 0, "force promoted"); return; } @@ -314,18 +320,16 @@ void Replayer::scan_local_mirror_snapshots() { // we don't have any mirror snapshots or only completed non-primary // mirror snapshots - scan_remote_mirror_snapshots(); + scan_remote_mirror_snapshots(locker); } template -void Replayer::scan_remote_mirror_snapshots() { +void Replayer::scan_remote_mirror_snapshots( + std::unique_lock* locker) { dout(10) << dendl; - { - // reset state in case new snapshot is added while we are scanning - std::unique_lock locker{m_lock}; - m_remote_image_updated = false; - } + // reset state in case new snapshot is added while we are scanning + m_remote_image_updated = false; bool remote_demoted = false; auto remote_image_ctx = m_state_builder->remote_image_ctx; @@ -343,7 +347,8 @@ void Replayer::scan_remote_mirror_snapshots() { << "mirror_ns=" << *mirror_ns << dendl; if (!mirror_ns->is_primary() && !mirror_ns->is_non_primary()) { derr << "unknown remote mirror snapshot state" << dendl; - handle_replay_complete(-EINVAL, "invalid remote mirror snapshot state"); + handle_replay_complete(locker, -EINVAL, + "invalid remote mirror snapshot state"); return; } else { remote_demoted = (mirror_ns->is_primary() && mirror_ns->is_demoted()); @@ -420,6 +425,8 @@ void Replayer::scan_remote_mirror_snapshots() { << "remote_snap_id_end=" << m_remote_snap_id_end << ", " << "remote_snap_ns=" << m_remote_mirror_snap_ns << dendl; if (m_remote_mirror_snap_ns.complete) { + locker->unlock(); + if (m_local_snap_id_end != CEPH_NOSNAP && !m_local_mirror_snap_ns.complete) { // attempt to resume image-sync @@ -437,11 +444,10 @@ void Replayer::scan_remote_mirror_snapshots() { } } - std::unique_lock locker{m_lock}; if (m_remote_image_updated) { // received update notification while scanning image, restart ... m_remote_image_updated = false; - locker.unlock(); + locker->unlock(); dout(10) << "restarting snapshot scan due to remote update notification" << dendl; @@ -449,13 +455,11 @@ void Replayer::scan_remote_mirror_snapshots() { return; } - if (is_replay_interrupted(&locker)) { + if (is_replay_interrupted(locker)) { return; } else if (remote_demoted) { - locker.unlock(); - dout(10) << "remote image demoted" << dendl; - handle_replay_complete(0, "remote image demoted"); + handle_replay_complete(locker, 0, "remote image demoted"); return; } @@ -910,6 +914,15 @@ template void Replayer::handle_replay_complete(int r, const std::string& description) { std::unique_lock locker{m_lock}; + handle_replay_complete(&locker, r, description); +} + +template +void Replayer::handle_replay_complete(std::unique_lock* locker, + int r, + const std::string& description) { + ceph_assert(ceph_mutex_is_locked_by_me(m_lock)); + if (m_error_code == 0) { m_error_code = r; m_error_description = description; diff --git a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h index 4fb7e90aed5..23cbfeb2f91 100644 --- a/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h +++ b/src/tools/rbd_mirror/image_replayer/snapshot/Replayer.h @@ -212,8 +212,8 @@ private: void refresh_remote_image(); void handle_refresh_remote_image(int r); - void scan_local_mirror_snapshots(); - void scan_remote_mirror_snapshots(); + void scan_local_mirror_snapshots(std::unique_lock* locker); + void scan_remote_mirror_snapshots(std::unique_lock* locker); void copy_snapshots(); void handle_copy_snapshots(int r); @@ -256,6 +256,8 @@ private: void handle_remote_image_update_notify(); void handle_replay_complete(int r, const std::string& description); + void handle_replay_complete(std::unique_lock* locker, + int r, const std::string& description); void notify_status_updated(); bool is_replay_interrupted(); -- 2.39.5