From 513a1b3bd00692eee7e66fe7b019e18ad6a1dc5c Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Mon, 27 Jan 2020 11:51:20 -0500 Subject: [PATCH] rbd-mirror: stop querying remote mirror uuid during image bootstrap The remote mirror uuid is now polled automatically by a higher layer. Signed-off-by: Jason Dillaman --- .../test_mock_BootstrapRequest.cc | 1 + .../test_mock_PrepareRemoteImageRequest.cc | 58 +++---------------- src/test/rbd_mirror/test_ImageReplayer.cc | 4 +- .../image_replayer/BootstrapRequest.cc | 6 +- .../PrepareRemoteImageRequest.cc | 49 +--------------- .../PrepareRemoteImageRequest.h | 13 ++--- .../rbd_mirror/image_replayer/StateBuilder.cc | 1 + 7 files changed, 26 insertions(+), 106 deletions(-) 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 e0dbb1c47df..cb25b351cfc 100644 --- a/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc +++ b/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc @@ -252,6 +252,7 @@ struct PrepareRemoteImageRequest { librados::IoCtx &, const std::string &global_image_id, const std::string &local_mirror_uuid, + const RemotePoolMeta& remote_pool_meta, ::journal::CacheManagerHandler *cache_manager_handler, StateBuilder** state_builder, Context *on_finish) { diff --git a/src/test/rbd_mirror/image_replayer/test_mock_PrepareRemoteImageRequest.cc b/src/test/rbd_mirror/image_replayer/test_mock_PrepareRemoteImageRequest.cc index dd48bef3135..49b22dac70f 100644 --- a/src/test/rbd_mirror/image_replayer/test_mock_PrepareRemoteImageRequest.cc +++ b/src/test/rbd_mirror/image_replayer/test_mock_PrepareRemoteImageRequest.cc @@ -78,8 +78,8 @@ struct GetMirrorImageIdRequest { template<> struct StateBuilder { std::string local_image_id; - std::string remote_mirror_uuid; std::string remote_image_id; + std::string remote_mirror_uuid; virtual ~StateBuilder() {} @@ -154,19 +154,6 @@ public: })); } - void expect_mirror_uuid_get(librados::IoCtx &io_ctx, - const std::string &mirror_uuid, int r) { - bufferlist bl; - encode(mirror_uuid, bl); - - EXPECT_CALL(get_mock_io_ctx(io_ctx), - exec(RBD_MIRRORING, _, StrEq("rbd"), StrEq("mirror_uuid_get"), _, _, _)) - .WillOnce(DoAll(WithArg<5>(Invoke([bl](bufferlist *out_bl) { - *out_bl = bl; - })), - Return(r))); - } - void expect_get_mirror_image(librados::IoCtx &io_ctx, cls::rbd::MirrorImageMode mode, int r) { cls::rbd::MirrorImage mirror_image; @@ -214,7 +201,6 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, Success) { MockThreads mock_threads(m_threads); InSequence seq; - expect_mirror_uuid_get(m_remote_io_ctx, "remote mirror uuid", 0); MockGetMirrorImageIdRequest mock_get_mirror_image_id_request; expect_get_mirror_image_id(mock_get_mirror_image_id_request, "remote image id", 0); @@ -242,6 +228,7 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, Success) { m_remote_io_ctx, "global image id", "local mirror uuid", + {"remote mirror uuid", ""}, nullptr, &mock_state_builder, &ctx); @@ -249,8 +236,6 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, Success) { ASSERT_EQ(0, ctx.wait()); ASSERT_TRUE(mock_state_builder != nullptr); - ASSERT_EQ(std::string("remote mirror uuid"), - mock_journal_state_builder.remote_mirror_uuid); ASSERT_EQ(std::string("remote image id"), mock_journal_state_builder.remote_image_id); ASSERT_TRUE(mock_journal_state_builder.remote_journaler != nullptr); @@ -263,7 +248,6 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, SuccessNotRegistered) { MockThreads mock_threads(m_threads); InSequence seq; - expect_mirror_uuid_get(m_remote_io_ctx, "remote mirror uuid", 0); MockGetMirrorImageIdRequest mock_get_mirror_image_id_request; expect_get_mirror_image_id(mock_get_mirror_image_id_request, "remote image id", 0); @@ -294,6 +278,7 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, SuccessNotRegistered) { m_remote_io_ctx, "global image id", "local mirror uuid", + {"remote mirror uuid", ""}, nullptr, &mock_state_builder, &ctx); @@ -301,8 +286,6 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, SuccessNotRegistered) { ASSERT_EQ(0, ctx.wait()); ASSERT_TRUE(mock_state_builder != nullptr); - ASSERT_EQ(std::string("remote mirror uuid"), - mock_journal_state_builder.remote_mirror_uuid); ASSERT_EQ(std::string("remote image id"), mock_journal_state_builder.remote_image_id); ASSERT_TRUE(mock_journal_state_builder.remote_journaler != nullptr); @@ -310,37 +293,11 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, SuccessNotRegistered) { mock_journal_state_builder.remote_client_state); } -TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, MirrorUuidError) { - ::journal::MockJournaler mock_remote_journaler; - MockThreads mock_threads(m_threads); - - InSequence seq; - expect_mirror_uuid_get(m_remote_io_ctx, "", -EINVAL); - - MockJournalStateBuilder mock_journal_state_builder; - MockStateBuilder* mock_state_builder = nullptr; - C_SaferCond ctx; - auto req = MockPrepareRemoteImageRequest::create(&mock_threads, - m_local_io_ctx, - m_remote_io_ctx, - "global image id", - "local mirror uuid", - nullptr, - &mock_state_builder, - &ctx); - req->send(); - - ASSERT_EQ(-EINVAL, ctx.wait()); - ASSERT_EQ(std::string(""), mock_journal_state_builder.remote_mirror_uuid); - ASSERT_TRUE(mock_journal_state_builder.remote_journaler == nullptr); -} - TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, MirrorImageIdError) { ::journal::MockJournaler mock_remote_journaler; MockThreads mock_threads(m_threads); InSequence seq; - expect_mirror_uuid_get(m_remote_io_ctx, "remote mirror uuid", 0); MockGetMirrorImageIdRequest mock_get_mirror_image_id_request; expect_get_mirror_image_id(mock_get_mirror_image_id_request, "", -EINVAL); @@ -352,14 +309,13 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, MirrorImageIdError) { m_remote_io_ctx, "global image id", "local mirror uuid", + {"remote mirror uuid", ""}, nullptr, &mock_state_builder, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); - ASSERT_EQ(std::string("remote mirror uuid"), - mock_journal_state_builder.remote_mirror_uuid); ASSERT_TRUE(mock_journal_state_builder.remote_journaler == nullptr); } @@ -368,7 +324,6 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, MirrorModeError) { MockThreads mock_threads(m_threads); InSequence seq; - expect_mirror_uuid_get(m_remote_io_ctx, "remote mirror uuid", 0); MockGetMirrorImageIdRequest mock_get_mirror_image_id_request; expect_get_mirror_image_id(mock_get_mirror_image_id_request, "remote image id", 0); @@ -384,6 +339,7 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, MirrorModeError) { m_remote_io_ctx, "global image id", "local mirror uuid", + {"remote mirror uuid", ""}, nullptr, &mock_state_builder, &ctx); @@ -398,7 +354,6 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, GetClientError) { MockThreads mock_threads(m_threads); InSequence seq; - expect_mirror_uuid_get(m_remote_io_ctx, "remote mirror uuid", 0); MockGetMirrorImageIdRequest mock_get_mirror_image_id_request; expect_get_mirror_image_id(mock_get_mirror_image_id_request, "remote image id", 0); @@ -420,6 +375,7 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, GetClientError) { m_remote_io_ctx, "global image id", "local mirror uuid", + {"remote mirror uuid", ""}, nullptr, &mock_state_builder, &ctx); @@ -434,7 +390,6 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, RegisterClientError) { MockThreads mock_threads(m_threads); InSequence seq; - expect_mirror_uuid_get(m_remote_io_ctx, "remote mirror uuid", 0); MockGetMirrorImageIdRequest mock_get_mirror_image_id_request; expect_get_mirror_image_id(mock_get_mirror_image_id_request, "remote image id", 0); @@ -465,6 +420,7 @@ TEST_F(TestMockImageReplayerPrepareRemoteImageRequest, RegisterClientError) { m_remote_io_ctx, "global image id", "local mirror uuid", + {"remote mirror uuid", ""}, nullptr, &mock_state_builder, &ctx); diff --git a/src/test/rbd_mirror/test_ImageReplayer.cc b/src/test/rbd_mirror/test_ImageReplayer.cc index fc7a3f86e6e..63bdc841059 100644 --- a/src/test/rbd_mirror/test_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_ImageReplayer.cc @@ -163,7 +163,9 @@ public: m_global_image_id, m_threads.get(), m_instance_watcher, m_local_status_updater, nullptr, &m_pool_meta_cache); - m_replayer->add_peer({"peer uuid", m_remote_ioctx, {}, nullptr}); + m_replayer->add_peer({"peer uuid", m_remote_ioctx, + {m_remote_mirror_uuid, "remote mirror peer uuid"}, + nullptr}); } void start() diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc index 2265f706892..e783989a28b 100644 --- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc @@ -163,7 +163,8 @@ void BootstrapRequest::prepare_remote_image() { BootstrapRequest, &BootstrapRequest::handle_prepare_remote_image>(this); auto req = image_replayer::PrepareRemoteImageRequest::create( m_threads, m_local_io_ctx, m_remote_io_ctx, m_global_image_id, - m_local_mirror_uuid, m_cache_manager_handler, m_state_builder, ctx); + m_local_mirror_uuid, m_remote_pool_meta, m_cache_manager_handler, + m_state_builder, ctx); req->send(); } @@ -172,6 +173,9 @@ void BootstrapRequest::handle_prepare_remote_image(int r) { dout(10) << "r=" << r << dendl; auto state_builder = *m_state_builder; + ceph_assert(state_builder == nullptr || + !state_builder->remote_mirror_uuid.empty()); + if (state_builder != nullptr && state_builder->is_local_primary()) { dout(5) << "local image is primary" << dendl; finish(-ENOMSG); diff --git a/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.cc b/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.cc index 462f8b69740..65ff88002c1 100644 --- a/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.cc @@ -33,51 +33,8 @@ using librbd::util::create_rados_callback; template void PrepareRemoteImageRequest::send() { - get_remote_mirror_uuid(); -} - -template -void PrepareRemoteImageRequest::get_remote_mirror_uuid() { - dout(10) << dendl; - - librados::ObjectReadOperation op; - librbd::cls_client::mirror_uuid_get_start(&op); - - librados::AioCompletion *aio_comp = create_rados_callback< - PrepareRemoteImageRequest, - &PrepareRemoteImageRequest::handle_get_remote_mirror_uuid>(this); - int r = m_remote_io_ctx.aio_operate(RBD_MIRRORING, aio_comp, &op, &m_out_bl); - ceph_assert(r == 0); - aio_comp->release(); -} - -template -void PrepareRemoteImageRequest::handle_get_remote_mirror_uuid(int r) { - if (r >= 0) { - auto it = m_out_bl.cbegin(); - r = librbd::cls_client::mirror_uuid_get_finish(&it, &m_remote_mirror_uuid); - if (r >= 0 && m_remote_mirror_uuid.empty()) { - r = -ENOENT; - } - } - - dout(10) << "r=" << r << dendl; - if (r < 0) { - if (r == -ENOENT) { - dout(5) << "remote mirror uuid missing" << dendl; - } else { - derr << "failed to retrieve remote mirror uuid: " << cpp_strerror(r) - << dendl; - } - finish(r); - return; - } - - auto state_builder = *m_state_builder; - if (state_builder != nullptr) { - // if the local image exists but the remote image doesn't, we still - // want to populate the remote mirror uuid that we've looked up - state_builder->remote_mirror_uuid = m_remote_mirror_uuid; + if (*m_state_builder != nullptr) { + (*m_state_builder)->remote_mirror_uuid = m_remote_pool_meta.mirror_uuid; } get_remote_image_id(); @@ -265,7 +222,7 @@ void PrepareRemoteImageRequest::finalize_journal_state_builder( *m_state_builder = state_builder; } - state_builder->remote_mirror_uuid = m_remote_mirror_uuid; + state_builder->remote_mirror_uuid = m_remote_pool_meta.mirror_uuid; state_builder->remote_image_id = m_remote_image_id; state_builder->remote_journaler = m_remote_journaler; state_builder->remote_client_state = client_state; diff --git a/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.h b/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.h index 616c18e13be..59a28feddf7 100644 --- a/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.h +++ b/src/tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.h @@ -10,6 +10,7 @@ #include "journal/Settings.h" #include "librbd/journal/Types.h" #include "librbd/journal/TypeTraits.h" +#include "tools/rbd_mirror/Types.h" #include namespace journal { class Journaler; } @@ -42,11 +43,13 @@ public: librados::IoCtx &remote_io_ctx, const std::string &global_image_id, const std::string &local_mirror_uuid, + const RemotePoolMeta& remote_pool_meta, ::journal::CacheManagerHandler *cache_manager_handler, StateBuilder** state_builder, Context *on_finish) { return new PrepareRemoteImageRequest(threads, local_io_ctx, remote_io_ctx, global_image_id, local_mirror_uuid, + remote_pool_meta, cache_manager_handler, state_builder, on_finish); } @@ -57,6 +60,7 @@ public: librados::IoCtx &remote_io_ctx, const std::string &global_image_id, const std::string &local_mirror_uuid, + const RemotePoolMeta& remote_pool_meta, ::journal::CacheManagerHandler *cache_manager_handler, StateBuilder** state_builder, Context *on_finish) @@ -65,6 +69,7 @@ public: m_remote_io_ctx(remote_io_ctx), m_global_image_id(global_image_id), m_local_mirror_uuid(local_mirror_uuid), + m_remote_pool_meta(remote_pool_meta), m_cache_manager_handler(cache_manager_handler), m_state_builder(state_builder), m_on_finish(on_finish) { @@ -79,9 +84,6 @@ private: * * | * v - * GET_REMOTE_MIRROR_UUID - * | - * v * GET_REMOTE_IMAGE_ID * | * v @@ -104,21 +106,18 @@ private: librados::IoCtx &m_remote_io_ctx; std::string m_global_image_id; std::string m_local_mirror_uuid; + RemotePoolMeta m_remote_pool_meta; ::journal::CacheManagerHandler *m_cache_manager_handler; StateBuilder** m_state_builder; Context *m_on_finish; bufferlist m_out_bl; - std::string m_remote_mirror_uuid; std::string m_remote_image_id; // journal-based mirroring Journaler *m_remote_journaler = nullptr; cls::journal::Client m_client; - void get_remote_mirror_uuid(); - void handle_get_remote_mirror_uuid(int r); - void get_remote_image_id(); void handle_get_remote_image_id(int r); diff --git a/src/tools/rbd_mirror/image_replayer/StateBuilder.cc b/src/tools/rbd_mirror/image_replayer/StateBuilder.cc index dda6204ed6a..4c543b7880a 100644 --- a/src/tools/rbd_mirror/image_replayer/StateBuilder.cc +++ b/src/tools/rbd_mirror/image_replayer/StateBuilder.cc @@ -42,6 +42,7 @@ bool StateBuilder::is_local_primary() const { template bool StateBuilder::is_linked() const { + ceph_assert(!remote_mirror_uuid.empty()); return (local_promotion_state == librbd::mirror::PROMOTION_STATE_NON_PRIMARY && local_primary_mirror_uuid == remote_mirror_uuid); -- 2.39.5