From e842463004a959da985c5ce1ea495537e328b094 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Mon, 27 Jan 2020 14:21:29 -0500 Subject: [PATCH] rbd-mirror: move remote image life-cycle management to state builder snapshot-based mirroring will need to leave the remote image open during replay. Signed-off-by: Jason Dillaman --- .../test_mock_BootstrapRequest.cc | 83 +++++-------------- .../image_replayer/BootstrapRequest.cc | 5 +- .../rbd_mirror/image_replayer/StateBuilder.cc | 29 +++++++ .../rbd_mirror/image_replayer/StateBuilder.h | 3 + .../image_replayer/journal/StateBuilder.cc | 7 +- 5 files changed, 59 insertions(+), 68 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 a765c460672..3eb42cf08e1 100644 --- a/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc +++ b/src/test/rbd_mirror/image_replayer/test_mock_BootstrapRequest.cc @@ -7,7 +7,6 @@ #include "tools/rbd_mirror/InstanceWatcher.h" #include "tools/rbd_mirror/Threads.h" #include "tools/rbd_mirror/image_replayer/BootstrapRequest.h" -#include "tools/rbd_mirror/image_replayer/CloseImageRequest.h" #include "tools/rbd_mirror/image_replayer/OpenImageRequest.h" #include "tools/rbd_mirror/image_replayer/OpenLocalImageRequest.h" #include "tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.h" @@ -89,33 +88,6 @@ struct InstanceWatcher { namespace image_replayer { -template<> -struct CloseImageRequest { - static CloseImageRequest* s_instance; - librbd::MockTestImageCtx **image_ctx = nullptr; - Context *on_finish = nullptr; - - static CloseImageRequest* create(librbd::MockTestImageCtx **image_ctx, - Context *on_finish) { - ceph_assert(s_instance != nullptr); - s_instance->image_ctx = image_ctx; - s_instance->on_finish = on_finish; - s_instance->construct(*image_ctx); - return s_instance; - } - - CloseImageRequest() { - ceph_assert(s_instance == nullptr); - s_instance = this; - } - ~CloseImageRequest() { - s_instance = nullptr; - } - - MOCK_METHOD1(construct, void(librbd::MockTestImageCtx *image_ctx)); - MOCK_METHOD0(send, void()); -}; - template<> struct OpenImageRequest { static OpenImageRequest* s_instance; @@ -262,6 +234,8 @@ struct StateBuilder { MOCK_CONST_METHOD0(is_local_primary, bool()); MOCK_CONST_METHOD0(is_linked, bool()); + MOCK_METHOD1(close_remote_image, void(Context*)); + MOCK_METHOD5(create_local_image_request, BaseRequest*(Threads*, librados::IoCtx&, @@ -279,8 +253,6 @@ struct StateBuilder { } }; -CloseImageRequest* - CloseImageRequest::s_instance = nullptr; OpenImageRequest* OpenImageRequest::s_instance = nullptr; OpenLocalImageRequest* @@ -321,7 +293,6 @@ class TestMockImageReplayerBootstrapRequest : public TestMockFixture { public: typedef Threads MockThreads; typedef BootstrapRequest MockBootstrapRequest; - typedef CloseImageRequest MockCloseImageRequest; typedef ImageSync MockImageSync; typedef InstanceWatcher MockInstanceWatcher; typedef OpenImageRequest MockOpenImageRequest; @@ -415,13 +386,13 @@ public: })); } - void expect_close_image(MockCloseImageRequest &mock_close_image_request, - librbd::MockTestImageCtx &mock_image_ctx, int r) { - EXPECT_CALL(mock_close_image_request, construct(&mock_image_ctx)); - EXPECT_CALL(mock_close_image_request, send()) - .WillOnce(Invoke([this, &mock_close_image_request, r]() { - *mock_close_image_request.image_ctx = nullptr; - m_threads->work_queue->queue(mock_close_image_request.on_finish, r); + void expect_close_remote_image( + MockStateBuilder& mock_state_builder, int r) { + EXPECT_CALL(mock_state_builder, close_remote_image(_)) + .WillOnce(Invoke([this, &mock_state_builder, r] + (Context* on_finish) { + mock_state_builder.remote_image_ctx = nullptr; + on_finish->complete(r); })); } @@ -531,8 +502,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, Success) { expect_is_disconnected(mock_state_builder, false); // close remote image - MockCloseImageRequest mock_close_image_request; - expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + expect_close_remote_image(mock_state_builder, 0); C_SaferCond ctx; MockThreads mock_threads(m_threads); @@ -573,8 +543,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, OpenLocalImageError) { -EINVAL); // close remote image - MockCloseImageRequest mock_close_image_request; - expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + expect_close_remote_image(mock_state_builder, 0); C_SaferCond ctx; MockThreads mock_threads(m_threads); @@ -626,8 +595,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, OpenLocalImageDNE) { expect_is_disconnected(mock_state_builder, false); // close remote image - MockCloseImageRequest mock_close_image_request; - expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + expect_close_remote_image(mock_state_builder, 0); C_SaferCond ctx; MockThreads mock_threads(m_threads); @@ -668,8 +636,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, OpenLocalImagePrimary) { -EREMOTEIO); // close remote image - MockCloseImageRequest mock_close_image_request; - expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + expect_close_remote_image(mock_state_builder, 0); C_SaferCond ctx; MockThreads mock_threads(m_threads); @@ -706,8 +673,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, CreateLocalImageError) { expect_create_local_image(mock_state_builder, "local image id", -EINVAL); // close remote image - MockCloseImageRequest mock_close_image_request; - expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + expect_close_remote_image(mock_state_builder, 0); C_SaferCond ctx; MockThreads mock_threads(m_threads); @@ -750,8 +716,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, PrepareReplayError) { expect_prepare_replay(mock_state_builder, false, false, -EINVAL); // close remote image - MockCloseImageRequest mock_close_image_request; - expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + expect_close_remote_image(mock_state_builder, 0); C_SaferCond ctx; MockThreads mock_threads(m_threads); @@ -794,8 +759,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, PrepareReplayResyncRequested) { expect_prepare_replay(mock_state_builder, true, false, 0); // close remote image - MockCloseImageRequest mock_close_image_request; - expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + expect_close_remote_image(mock_state_builder, 0); C_SaferCond ctx; MockThreads mock_threads(m_threads); @@ -844,8 +808,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, PrepareReplaySyncing) { expect_image_sync(mock_image_sync, 0); // close remote image - MockCloseImageRequest mock_close_image_request; - expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + expect_close_remote_image(mock_state_builder, 0); C_SaferCond ctx; MockThreads mock_threads(m_threads); @@ -889,8 +852,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, PrepareReplayDisconnected) { expect_is_disconnected(mock_state_builder, false); // close remote image - MockCloseImageRequest mock_close_image_request; - expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + expect_close_remote_image(mock_state_builder, 0); C_SaferCond ctx; MockThreads mock_threads(m_threads); @@ -938,8 +900,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, ImageSyncError) { expect_image_sync(mock_image_sync, -EINVAL); // close remote image - MockCloseImageRequest mock_close_image_request; - expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + expect_close_remote_image(mock_state_builder, 0); C_SaferCond ctx; MockThreads mock_threads(m_threads); @@ -983,8 +944,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, ImageSyncCanceled) { expect_is_disconnected(mock_state_builder, false); // close remote image - MockCloseImageRequest mock_close_image_request; - expect_close_image(mock_close_image_request, mock_remote_image_ctx, 0); + expect_close_remote_image(mock_state_builder, 0); C_SaferCond ctx; MockThreads mock_threads(m_threads); @@ -1028,8 +988,7 @@ TEST_F(TestMockImageReplayerBootstrapRequest, CloseRemoteImageError) { expect_prepare_replay(mock_state_builder, false, false, 0); expect_is_disconnected(mock_state_builder, false); - MockCloseImageRequest mock_close_image_request; - expect_close_image(mock_close_image_request, mock_remote_image_ctx, -EINVAL); + expect_close_remote_image(mock_state_builder, -EINVAL); C_SaferCond ctx; MockThreads mock_threads(m_threads); diff --git a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc index 739a5599f1b..7f338cc3e08 100644 --- a/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/BootstrapRequest.cc @@ -3,7 +3,6 @@ #include "include/compat.h" #include "BootstrapRequest.h" -#include "CloseImageRequest.h" #include "CreateImageRequest.h" #include "OpenImageRequest.h" #include "OpenLocalImageRequest.h" @@ -426,9 +425,7 @@ void BootstrapRequest::close_remote_image() { BootstrapRequest, &BootstrapRequest::handle_close_remote_image>(this); ceph_assert(*m_state_builder != nullptr); - auto request = CloseImageRequest::create( - &(*m_state_builder)->remote_image_ctx, ctx); - request->send(); + (*m_state_builder)->close_remote_image(ctx); } template diff --git a/src/tools/rbd_mirror/image_replayer/StateBuilder.cc b/src/tools/rbd_mirror/image_replayer/StateBuilder.cc index 1e8420bbbc8..20a5378d47a 100644 --- a/src/tools/rbd_mirror/image_replayer/StateBuilder.cc +++ b/src/tools/rbd_mirror/image_replayer/StateBuilder.cc @@ -78,6 +78,35 @@ void StateBuilder::handle_close_local_image(int r, Context* on_finish) { on_finish->complete(r); } +template +void StateBuilder::close_remote_image(Context* on_finish) { + if (remote_image_ctx == nullptr) { + on_finish->complete(0); + return; + } + + dout(10) << dendl; + auto ctx = new LambdaContext([this, on_finish](int r) { + handle_close_remote_image(r, on_finish); + }); + auto request = image_replayer::CloseImageRequest::create( + &remote_image_ctx, ctx); + request->send(); +} + +template +void StateBuilder::handle_close_remote_image(int r, Context* on_finish) { + dout(10) << "r=" << r << dendl; + + ceph_assert(remote_image_ctx == nullptr); + if (r < 0) { + derr << "failed to close remote image for image " << global_image_id << ": " + << cpp_strerror(r) << dendl; + } + + on_finish->complete(r); +} + template void StateBuilder::destroy_sync_point_handler() { if (m_sync_point_handler == nullptr) { diff --git a/src/tools/rbd_mirror/image_replayer/StateBuilder.h b/src/tools/rbd_mirror/image_replayer/StateBuilder.h index c6bf3ea8fd0..a85cb3b779a 100644 --- a/src/tools/rbd_mirror/image_replayer/StateBuilder.h +++ b/src/tools/rbd_mirror/image_replayer/StateBuilder.h @@ -49,6 +49,8 @@ public: virtual image_sync::SyncPointHandler* create_sync_point_handler() = 0; void destroy_sync_point_handler(); + void close_remote_image(Context* on_finish); + virtual BaseRequest* create_local_image_request( Threads* threads, librados::IoCtx& local_io_ctx, @@ -91,6 +93,7 @@ protected: private: void handle_close_local_image(int r, Context* on_finish); + void handle_close_remote_image(int r, Context* on_finish); }; } // namespace image_replayer diff --git a/src/tools/rbd_mirror/image_replayer/journal/StateBuilder.cc b/src/tools/rbd_mirror/image_replayer/journal/StateBuilder.cc index 01d51e7d569..9056785475b 100644 --- a/src/tools/rbd_mirror/image_replayer/journal/StateBuilder.cc +++ b/src/tools/rbd_mirror/image_replayer/journal/StateBuilder.cc @@ -43,10 +43,13 @@ void StateBuilder::close(Context* on_finish) { // close the remote journaler after closing the local image // in case we have lost contact w/ the remote cluster and // will block - auto ctx = new LambdaContext([this, on_finish](int) { + on_finish = new LambdaContext([this, on_finish](int) { shut_down_remote_journaler(on_finish); }); - this->close_local_image(ctx); + on_finish = new LambdaContext([this, on_finish](int) { + this->close_local_image(on_finish); + }); + this->close_remote_image(on_finish); } template -- 2.39.5