From: Jason Dillaman Date: Mon, 27 Jan 2020 18:16:56 +0000 (-0500) Subject: rbd-mirror: remove unnecessary get mirror info call from bootstrap X-Git-Tag: v15.1.1~472^2~26 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=9641673b47c7d725148c77173e2c4bfabcaa95e5;p=ceph-ci.git rbd-mirror: remove unnecessary get mirror info call from bootstrap Both the prepare local and remote state machines will now retrieve their respective mirror info data so it does not need to be repeated at the higher-level bootstrap state machine. Signed-off-by: Jason Dillaman --- diff --git a/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc b/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc index c9805f6d622..24822f6e493 100644 --- a/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc +++ b/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc @@ -3,7 +3,6 @@ #include "test/rbd_mirror/test_mock_fixture.h" #include "librbd/journal/TypeTraits.h" -#include "librbd/mirror/GetInfoRequest.h" #include "tools/rbd_mirror/BaseRequest.h" #include "tools/rbd_mirror/InstanceWatcher.h" #include "tools/rbd_mirror/Threads.h" @@ -30,42 +29,6 @@ struct MockTestImageCtx : public librbd::MockImageCtx { }; } // anonymous namespace - -namespace mirror { - -template<> -struct GetInfoRequest { - static GetInfoRequest* s_instance; - cls::rbd::MirrorImage *mirror_image; - PromotionState *promotion_state; - Context *on_finish = nullptr; - - static GetInfoRequest* create(librbd::MockTestImageCtx &image_ctx, - cls::rbd::MirrorImage *mirror_image, - PromotionState *promotion_state, - std::string* primary_mirror_uuid, - Context *on_finish) { - ceph_assert(s_instance != nullptr); - s_instance->mirror_image = mirror_image; - s_instance->promotion_state = promotion_state; - s_instance->on_finish = on_finish; - return s_instance; - } - - GetInfoRequest() { - ceph_assert(s_instance == nullptr); - s_instance = this; - } - ~GetInfoRequest() { - s_instance = nullptr; - } - - MOCK_METHOD0(send, void()); -}; - -GetInfoRequest* GetInfoRequest::s_instance = nullptr; - -} // namespace mirror } // namespace librbd namespace rbd { @@ -366,7 +329,6 @@ public: typedef PrepareLocalImageRequest MockPrepareLocalImageRequest; typedef PrepareRemoteImageRequest MockPrepareRemoteImageRequest; typedef StateBuilder MockStateBuilder; - typedef librbd::mirror::GetInfoRequest MockGetMirrorInfoRequest; typedef std::list Tags; void SetUp() override { @@ -463,20 +425,6 @@ public: })); } - void expect_get_remote_mirror_info( - MockGetMirrorInfoRequest &mock_get_mirror_info_request, - const cls::rbd::MirrorImage &mirror_image, - librbd::mirror::PromotionState promotion_state, int r) { - EXPECT_CALL(mock_get_mirror_info_request, send()) - .WillOnce(Invoke([this, &mock_get_mirror_info_request, mirror_image, - promotion_state, r]() { - *mock_get_mirror_info_request.mirror_image = mirror_image; - *mock_get_mirror_info_request.promotion_state = promotion_state; - m_threads->work_queue->queue( - mock_get_mirror_info_request.on_finish, r); - })); - } - void expect_create_local_image(MockStateBuilder& mock_state_builder, const std::string& local_image_id, int r) { EXPECT_CALL(mock_state_builder, @@ -572,13 +520,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, Success) { expect_open_image(mock_open_image_request, m_remote_io_ctx, mock_remote_image_ctx.id, mock_remote_image_ctx, 0); - // test if remote image is primary - MockGetMirrorInfoRequest mock_get_mirror_info_request; - expect_get_remote_mirror_info(mock_get_mirror_info_request, - {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, "uuid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, - librbd::mirror::PROMOTION_STATE_PRIMARY, 0); - // open the local image librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockOpenLocalImageRequest mock_open_local_image_request; @@ -624,13 +565,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, OpenLocalImageError) { expect_open_image(mock_open_image_request, m_remote_io_ctx, mock_remote_image_ctx.id, mock_remote_image_ctx, 0); - // test if remote image is primary - MockGetMirrorInfoRequest mock_get_mirror_info_request; - expect_get_remote_mirror_info(mock_get_mirror_info_request, - {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, "uuid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, - librbd::mirror::PROMOTION_STATE_PRIMARY, 0); - // open the local image librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockOpenLocalImageRequest mock_open_local_image_request; @@ -673,13 +607,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, OpenLocalImageDNE) { expect_open_image(mock_open_image_request, m_remote_io_ctx, mock_remote_image_ctx.id, mock_remote_image_ctx, 0); - // test if remote image is primary - MockGetMirrorInfoRequest mock_get_mirror_info_request; - expect_get_remote_mirror_info(mock_get_mirror_info_request, - {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, "uuid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, - librbd::mirror::PROMOTION_STATE_PRIMARY, 0); - // open the local image librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockOpenLocalImageRequest mock_open_local_image_request; @@ -733,13 +660,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, OpenLocalImagePrimary) { expect_open_image(mock_open_image_request, m_remote_io_ctx, mock_remote_image_ctx.id, mock_remote_image_ctx, 0); - // test if remote image is primary - MockGetMirrorInfoRequest mock_get_mirror_info_request; - expect_get_remote_mirror_info(mock_get_mirror_info_request, - {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, "uuid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, - librbd::mirror::PROMOTION_STATE_PRIMARY, 0); - // open the local image librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockOpenLocalImageRequest mock_open_local_image_request; @@ -782,13 +702,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, CreateLocalImageError) { expect_open_image(mock_open_image_request, m_remote_io_ctx, mock_remote_image_ctx.id, mock_remote_image_ctx, 0); - // test if remote image is primary - MockGetMirrorInfoRequest mock_get_mirror_info_request; - expect_get_remote_mirror_info(mock_get_mirror_info_request, - {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, "uuid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, - librbd::mirror::PROMOTION_STATE_PRIMARY, 0); - // create local image expect_create_local_image(mock_state_builder, "local image id", -EINVAL); @@ -827,13 +740,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, PrepareReplayError) { expect_open_image(mock_open_image_request, m_remote_io_ctx, mock_remote_image_ctx.id, mock_remote_image_ctx, 0); - // test if remote image is primary - MockGetMirrorInfoRequest mock_get_mirror_info_request; - expect_get_remote_mirror_info(mock_get_mirror_info_request, - {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, "uuid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, - librbd::mirror::PROMOTION_STATE_PRIMARY, 0); - // open the local image librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockOpenLocalImageRequest mock_open_local_image_request; @@ -878,13 +784,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, PrepareReplayResyncRequested) { expect_open_image(mock_open_image_request, m_remote_io_ctx, mock_remote_image_ctx.id, mock_remote_image_ctx, 0); - // test if remote image is primary - MockGetMirrorInfoRequest mock_get_mirror_info_request; - expect_get_remote_mirror_info(mock_get_mirror_info_request, - {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, "uuid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, - librbd::mirror::PROMOTION_STATE_PRIMARY, 0); - // open the local image librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockOpenLocalImageRequest mock_open_local_image_request; @@ -930,13 +829,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, PrepareReplaySyncing) { expect_open_image(mock_open_image_request, m_remote_io_ctx, mock_remote_image_ctx.id, mock_remote_image_ctx, 0); - // test if remote image is primary - MockGetMirrorInfoRequest mock_get_mirror_info_request; - expect_get_remote_mirror_info(mock_get_mirror_info_request, - {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, "uuid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, - librbd::mirror::PROMOTION_STATE_PRIMARY, 0); - // open the local image librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockOpenLocalImageRequest mock_open_local_image_request; @@ -986,13 +878,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, PrepareReplayDisconnected) { expect_open_image(mock_open_image_request, m_remote_io_ctx, mock_remote_image_ctx.id, mock_remote_image_ctx, 0); - // test if remote image is primary - MockGetMirrorInfoRequest mock_get_mirror_info_request; - expect_get_remote_mirror_info(mock_get_mirror_info_request, - {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, "uuid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, - librbd::mirror::PROMOTION_STATE_PRIMARY, 0); - // open the local image librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockOpenLocalImageRequest mock_open_local_image_request; @@ -1038,13 +923,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, ImageSyncError) { expect_open_image(mock_open_image_request, m_remote_io_ctx, mock_remote_image_ctx.id, mock_remote_image_ctx, 0); - // test if remote image is primary - MockGetMirrorInfoRequest mock_get_mirror_info_request; - expect_get_remote_mirror_info(mock_get_mirror_info_request, - {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, "uuid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, - librbd::mirror::PROMOTION_STATE_PRIMARY, 0); - // open the local image librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockOpenLocalImageRequest mock_open_local_image_request; @@ -1094,13 +972,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, ImageSyncCanceled) { expect_open_image(mock_open_image_request, m_remote_io_ctx, mock_remote_image_ctx.id, mock_remote_image_ctx, 0); - // test if remote image is primary - MockGetMirrorInfoRequest mock_get_mirror_info_request; - expect_get_remote_mirror_info(mock_get_mirror_info_request, - {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, "uuid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, - librbd::mirror::PROMOTION_STATE_PRIMARY, 0); - // open the local image librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockOpenLocalImageRequest mock_open_local_image_request; @@ -1147,13 +1018,6 @@ TEST_F(TestMockImageReplayerBootstrapRequest, CloseRemoteImageError) { expect_open_image(mock_open_image_request, m_remote_io_ctx, mock_remote_image_ctx.id, mock_remote_image_ctx, 0); - // test if remote image is primary - MockGetMirrorInfoRequest mock_get_mirror_info_request; - expect_get_remote_mirror_info(mock_get_mirror_info_request, - {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, "uuid", - cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, - librbd::mirror::PROMOTION_STATE_PRIMARY, 0); - // open the local image librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); MockOpenLocalImageRequest mock_open_local_image_request; diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc index 80d40b60242..bfa252275c7 100644 --- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc @@ -20,7 +20,6 @@ #include "librbd/Journal.h" #include "librbd/Utils.h" #include "librbd/journal/Types.h" -#include "librbd/mirror/GetInfoRequest.h" #include "tools/rbd_mirror/BaseRequest.h" #include "tools/rbd_mirror/ImageSync.h" #include "tools/rbd_mirror/ProgressContext.h" @@ -180,6 +179,10 @@ void BootstrapRequest::handle_prepare_remote_image(int r) { dout(5) << "local image is primary" << dendl; finish(-ENOMSG); return; + } else if (r == -EREMOTEIO) { + dout(10) << "remote-image is non-primary" << cpp_strerror(r) << dendl; + finish(r); + return; } else if (r == -ENOENT || state_builder == nullptr) { dout(10) << "remote image does not exist" << dendl; @@ -232,66 +235,7 @@ void BootstrapRequest::handle_open_remote_image(int r) { return; } - get_remote_mirror_info(); -} - -template -void BootstrapRequest::get_remote_mirror_info() { - dout(15) << dendl; - - update_progress("GET_REMOTE_MIRROR_INFO"); - - Context *ctx = create_context_callback< - BootstrapRequest, &BootstrapRequest::handle_get_remote_mirror_info>( - this); - auto request = librbd::mirror::GetInfoRequest::create( - *m_remote_image_ctx, &m_mirror_image, &m_promotion_state, - &m_remote_primary_mirror_uuid, ctx); - request->send(); -} - -template -void BootstrapRequest::handle_get_remote_mirror_info(int r) { - dout(15) << "r=" << r << dendl; - - if (r == -ENOENT) { - dout(5) << "remote image is not mirrored" << dendl; - m_ret_val = -EREMOTEIO; - close_remote_image(); - return; - } else if (r < 0) { - derr << "error querying remote image primary status: " << cpp_strerror(r) - << dendl; - m_ret_val = r; - close_remote_image(); - return; - } - - if (m_mirror_image.state == cls::rbd::MIRROR_IMAGE_STATE_DISABLING) { - dout(5) << "remote image mirroring is being disabled" << dendl; - m_ret_val = -EREMOTEIO; - close_remote_image(); - return; - } - - if (m_mirror_image.mode != cls::rbd::MIRROR_IMAGE_MODE_JOURNAL) { - dout(5) << ": remote image is in unsupported mode: " << m_mirror_image.mode - << dendl; - m_ret_val = -EOPNOTSUPP; - close_remote_image(); - return; - } - ceph_assert(*m_state_builder != nullptr); - if (m_promotion_state != librbd::mirror::PROMOTION_STATE_PRIMARY && - (*m_state_builder)->local_image_id.empty()) { - // no local image and remote isn't primary -- don't sync it - dout(5) << "remote image is not primary -- not syncing" << dendl; - m_ret_val = -EREMOTEIO; - close_remote_image(); - return; - } - if ((*m_state_builder)->local_image_id.empty()) { create_local_image(); return; diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h index bfad0b0942d..7d94d847c0d 100644 --- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h +++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.h @@ -97,10 +97,7 @@ private: * v (error) * * OPEN_REMOTE_IMAGE * * * * * * * * * * * * * * * * * * * * | * - * v * - * GET_REMOTE_MIRROR_INFO * * * * * * * * * * * * * * * * - * | * * - * | * * + * | * * \----> CREATE_LOCAL_IMAGE * * * * * * * * * * * * * * | | ^ * * * | | . * * @@ -143,10 +140,6 @@ private: bool m_canceled = false; ImageCtxT *m_remote_image_ctx = nullptr; - cls::rbd::MirrorImage m_mirror_image; - librbd::mirror::PromotionState m_promotion_state = - librbd::mirror::PROMOTION_STATE_NON_PRIMARY; - std::string m_remote_primary_mirror_uuid; int m_ret_val = 0; std::string m_local_image_name; @@ -164,9 +157,6 @@ private: void open_remote_image(); void handle_open_remote_image(int r); - void get_remote_mirror_info(); - void handle_get_remote_mirror_info(int r); - void open_local_image(); void handle_open_local_image(int r); diff --git a/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.cc b/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.cc index ccb578848ff..652c22d7122 100644 --- a/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.cc @@ -96,12 +96,24 @@ void PrepareRemoteImageRequest::handle_get_mirror_info(int r) { return; } - if (*m_state_builder != nullptr && - (*m_state_builder)->get_mirror_image_mode() != m_mirror_image.mode) { + auto state_builder = *m_state_builder; + if (state_builder != nullptr && + state_builder->get_mirror_image_mode() != m_mirror_image.mode) { derr << "local and remote mirror image using different mirroring modes " << "for image " << m_global_image_id << ": split-brain" << dendl; finish(-EEXIST); return; + } else if (m_mirror_image.state == cls::rbd::MIRROR_IMAGE_STATE_DISABLING) { + dout(5) << "remote image mirroring is being disabled" << dendl; + finish(-EREMOTEIO); + return; + } else if (m_promotion_state != librbd::mirror::PROMOTION_STATE_PRIMARY && + (state_builder == nullptr || + state_builder->local_image_id.empty())) { + // no local image and remote isn't primary -- don't sync it + dout(5) << "remote image is not primary -- not syncing" << dendl; + finish(-EREMOTEIO); + return; } switch (m_mirror_image.mode) {