From 5dcd9ed0d5240c15fce966cacb2fc4dd5c0144c2 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 19 Jul 2017 12:20:34 -0400 Subject: [PATCH] rbd-mirror: image replayer should query remote mirror uuid / image id Signed-off-by: Jason Dillaman --- src/test/rbd_mirror/test_ImageDeleter.cc | 1 - src/test/rbd_mirror/test_ImageReplayer.cc | 23 +-- .../rbd_mirror/test_mock_ImageReplayer.cc | 136 ++++++++++++++++++ src/tools/rbd_mirror/ImageReplayer.cc | 44 +++++- src/tools/rbd_mirror/ImageReplayer.h | 6 + 5 files changed, 196 insertions(+), 14 deletions(-) diff --git a/src/test/rbd_mirror/test_ImageDeleter.cc b/src/test/rbd_mirror/test_ImageDeleter.cc index 5ac67331423..c06a4662532 100644 --- a/src/test/rbd_mirror/test_ImageDeleter.cc +++ b/src/test/rbd_mirror/test_ImageDeleter.cc @@ -17,7 +17,6 @@ #include "cls/rbd/cls_rbd_types.h" #include "cls/rbd/cls_rbd_client.h" #include "tools/rbd_mirror/ImageDeleter.h" -#include "tools/rbd_mirror/ImageReplayer.h" #include "tools/rbd_mirror/ServiceDaemon.h" #include "tools/rbd_mirror/Threads.h" #include "librbd/ImageCtx.h" diff --git a/src/test/rbd_mirror/test_ImageReplayer.cc b/src/test/rbd_mirror/test_ImageReplayer.cc index 7b30e30cabf..4c9103eb930 100644 --- a/src/test/rbd_mirror/test_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_ImageReplayer.cc @@ -115,6 +115,7 @@ public: EXPECT_EQ(0, librbd::create(m_remote_ioctx, m_image_name.c_str(), 1 << 22, false, features, &order, 0, 0)); m_remote_image_id = get_image_id(m_remote_ioctx, m_image_name); + m_global_image_id = get_global_image_id(m_remote_ioctx, m_remote_image_id); m_threads.reset(new rbd::mirror::Threads<>(reinterpret_cast( m_local_ioctx.cct()))); @@ -148,7 +149,7 @@ public: m_replayer = new ImageReplayerT( m_threads.get(), m_image_deleter.get(), m_instance_watcher, rbd::mirror::RadosRef(new librados::Rados(m_local_ioctx)), - m_local_mirror_uuid, m_local_ioctx.get_id(), "global image id"); + m_local_mirror_uuid, m_local_ioctx.get_id(), m_global_image_id); m_replayer->add_remote_image(m_remote_mirror_uuid, m_remote_image_id, m_remote_ioctx); } @@ -205,6 +206,14 @@ public: return id; } + std::string get_global_image_id(librados::IoCtx& io_ctx, + const std::string& image_id) { + cls::rbd::MirrorImage mirror_image; + EXPECT_EQ(0, librbd::cls_client::mirror_image_get(&io_ctx, image_id, + &mirror_image)); + return mirror_image.global_image_id; + } + void open_image(librados::IoCtx &ioctx, const std::string &image_name, bool readonly, librbd::ImageCtx **ictxp) { @@ -385,6 +394,7 @@ public: std::string m_image_name; int64_t m_remote_pool_id; std::string m_remote_image_id; + std::string m_global_image_id; rbd::mirror::ImageReplayer<> *m_replayer; C_WatchCtx *m_watch_ctx; uint64_t m_watch_handle; @@ -412,14 +422,7 @@ TEST_F(TestImageReplayer, BootstrapErrorLocalImageExists) TEST_F(TestImageReplayer, BootstrapErrorNoJournal) { - // disable remote image journaling - librbd::ImageCtx *ictx; - open_remote_image(&ictx); - uint64_t features; - ASSERT_EQ(0, librbd::get_features(ictx, &features)); - ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_JOURNALING, - false)); - close_image(ictx); + ASSERT_EQ(0, librbd::Journal<>::remove(m_remote_ioctx, m_remote_image_id)); create_replayer<>(); C_SaferCond cond; @@ -522,7 +525,7 @@ TEST_F(TestImageReplayer, ErrorNoJournal) C_SaferCond cond; m_replayer->start(&cond); - ASSERT_EQ(-ENOENT, cond.wait()); + ASSERT_EQ(0, cond.wait()); } TEST_F(TestImageReplayer, StartStop) diff --git a/src/test/rbd_mirror/test_mock_ImageReplayer.cc b/src/test/rbd_mirror/test_mock_ImageReplayer.cc index 71a94e5896b..b5d4ce6e23a 100644 --- a/src/test/rbd_mirror/test_mock_ImageReplayer.cc +++ b/src/test/rbd_mirror/test_mock_ImageReplayer.cc @@ -11,6 +11,7 @@ #include "tools/rbd_mirror/image_replayer/CloseImageRequest.h" #include "tools/rbd_mirror/image_replayer/EventPreprocessor.h" #include "tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.h" +#include "tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.h" #include "test/rbd_mirror/test_mock_fixture.h" #include "test/journal/mock/MockJournaler.h" #include "test/librbd/mock/MockImageCtx.h" @@ -110,6 +111,32 @@ struct PrepareLocalImageRequest { MOCK_METHOD0(send, void()); }; +template<> +struct PrepareRemoteImageRequest { + static PrepareRemoteImageRequest* s_instance; + std::string *remote_mirror_uuid = nullptr; + std::string *remote_image_id = nullptr; + Context *on_finish = nullptr; + + static PrepareRemoteImageRequest* create(librados::IoCtx &, + const std::string &global_image_id, + std::string *remote_mirror_uuid, + std::string *remote_image_id, + Context *on_finish) { + assert(s_instance != nullptr); + s_instance->remote_mirror_uuid = remote_mirror_uuid; + s_instance->remote_image_id = remote_image_id; + s_instance->on_finish = on_finish; + return s_instance; + } + + PrepareRemoteImageRequest() { + s_instance = this; + } + + MOCK_METHOD0(send, void()); +}; + template<> struct BootstrapRequest { static BootstrapRequest* s_instance; @@ -247,6 +274,7 @@ BootstrapRequest* BootstrapRequest* CloseImageRequest::s_instance = nullptr; EventPreprocessor* EventPreprocessor::s_instance = nullptr; PrepareLocalImageRequest* PrepareLocalImageRequest::s_instance = nullptr; +PrepareRemoteImageRequest* PrepareRemoteImageRequest::s_instance = nullptr; ReplayStatusFormatter* ReplayStatusFormatter::s_instance = nullptr; } // namespace image_replayer @@ -266,6 +294,7 @@ public: typedef CloseImageRequest MockCloseImageRequest; typedef EventPreprocessor MockEventPreprocessor; typedef PrepareLocalImageRequest MockPrepareLocalImageRequest; + typedef PrepareRemoteImageRequest MockPrepareRemoteImageRequest; typedef ReplayStatusFormatter MockReplayStatusFormatter; typedef librbd::journal::Replay MockReplay; typedef ImageReplayer MockImageReplayer; @@ -318,6 +347,17 @@ public: })); } + void expect_send(MockPrepareRemoteImageRequest& mock_request, + const std::string& mirror_uuid, const std::string& image_id, + int r) { + EXPECT_CALL(mock_request, send()) + .WillOnce(Invoke([&mock_request, image_id, mirror_uuid, r]() { + *mock_request.remote_mirror_uuid = mirror_uuid; + *mock_request.remote_image_id = image_id; + mock_request.on_finish->complete(r); + })); + } + void expect_send(MockBootstrapRequest &mock_bootstrap_request, librbd::MockTestImageCtx &mock_local_image_ctx, bool do_resync, int r) { @@ -474,6 +514,7 @@ TEST_F(TestMockImageReplayer, StartStop) { journal::MockJournaler mock_remote_journaler; MockPrepareLocalImageRequest mock_prepare_local_image_request; + MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; MockReplay mock_local_replay; MockEventPreprocessor mock_event_preprocessor; @@ -484,6 +525,8 @@ TEST_F(TestMockImageReplayer, StartStop) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); + expect_send(mock_prepare_remote_image_request, "remote mirror uuid", + m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); expect_send(mock_bootstrap_request, mock_local_image_ctx, false, 0); @@ -554,6 +597,7 @@ TEST_F(TestMockImageReplayer, LocalImageDNE) { journal::MockJournaler mock_remote_journaler; MockPrepareLocalImageRequest mock_prepare_local_image_request; + MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; MockReplayStatusFormatter mock_replay_status_formatter; @@ -561,6 +605,8 @@ TEST_F(TestMockImageReplayer, LocalImageDNE) { InSequence seq; expect_send(mock_prepare_local_image_request, "", "", -ENOENT); + expect_send(mock_prepare_remote_image_request, "remote mirror uuid", + m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); expect_send(mock_bootstrap_request, mock_local_image_ctx, false, -EREMOTEIO); @@ -596,6 +642,78 @@ TEST_F(TestMockImageReplayer, PrepareLocalImageError) { ASSERT_EQ(-EINVAL, start_ctx.wait()); } +TEST_F(TestMockImageReplayer, GetRemoteImageIdDNE) { + create_local_image(); + librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + + MockPrepareLocalImageRequest mock_prepare_local_image_request; + MockPrepareRemoteImageRequest mock_prepare_remote_image_request; + MockReplayStatusFormatter mock_replay_status_formatter; + + expect_get_or_send_update(mock_replay_status_formatter); + + InSequence seq; + expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, + "remote mirror uuid", 0); + expect_send(mock_prepare_remote_image_request, "remote mirror uuid", + "", -ENOENT); + + MockImageDeleter mock_image_deleter; + create_image_replayer(mock_image_deleter); + + C_SaferCond start_ctx; + m_image_replayer->start(&start_ctx); + ASSERT_EQ(0, start_ctx.wait()); +} + +TEST_F(TestMockImageReplayer, GetRemoteImageIdNonLinkedDNE) { + create_local_image(); + librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + + MockPrepareLocalImageRequest mock_prepare_local_image_request; + MockPrepareRemoteImageRequest mock_prepare_remote_image_request; + MockReplayStatusFormatter mock_replay_status_formatter; + + expect_get_or_send_update(mock_replay_status_formatter); + + InSequence seq; + expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, + "some other mirror uuid", 0); + expect_send(mock_prepare_remote_image_request, "remote mirror uuid", + "", -ENOENT); + + MockImageDeleter mock_image_deleter; + create_image_replayer(mock_image_deleter); + + C_SaferCond start_ctx; + m_image_replayer->start(&start_ctx); + ASSERT_EQ(-ENOENT, start_ctx.wait()); +} + +TEST_F(TestMockImageReplayer, GetRemoteImageIdError) { + create_local_image(); + librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx); + + MockPrepareLocalImageRequest mock_prepare_local_image_request; + MockPrepareRemoteImageRequest mock_prepare_remote_image_request; + MockReplayStatusFormatter mock_replay_status_formatter; + + expect_get_or_send_update(mock_replay_status_formatter); + + InSequence seq; + expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, + "remote mirror uuid", 0); + expect_send(mock_prepare_remote_image_request, "remote mirror uuid", + m_remote_image_ctx->id, -EINVAL); + + MockImageDeleter mock_image_deleter; + create_image_replayer(mock_image_deleter); + + C_SaferCond start_ctx; + m_image_replayer->start(&start_ctx); + ASSERT_EQ(-EINVAL, start_ctx.wait()); +} + TEST_F(TestMockImageReplayer, BootstrapError) { create_local_image(); @@ -603,6 +721,7 @@ TEST_F(TestMockImageReplayer, BootstrapError) { journal::MockJournaler mock_remote_journaler; MockPrepareLocalImageRequest mock_prepare_local_image_request; + MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; MockReplayStatusFormatter mock_replay_status_formatter; @@ -611,6 +730,8 @@ TEST_F(TestMockImageReplayer, BootstrapError) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); + expect_send(mock_prepare_remote_image_request, "remote mirror uuid", + m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); expect_send(mock_bootstrap_request, mock_local_image_ctx, false, -EINVAL); @@ -636,6 +757,7 @@ TEST_F(TestMockImageReplayer, StartExternalReplayError) { journal::MockJournaler mock_remote_journaler; MockPrepareLocalImageRequest mock_prepare_local_image_request; + MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; MockReplay mock_local_replay; MockEventPreprocessor mock_event_preprocessor; @@ -646,6 +768,8 @@ TEST_F(TestMockImageReplayer, StartExternalReplayError) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); + expect_send(mock_prepare_remote_image_request, "remote mirror uuid", + m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); expect_send(mock_bootstrap_request, mock_local_image_ctx, false, 0); @@ -686,6 +810,7 @@ TEST_F(TestMockImageReplayer, StopError) { journal::MockJournaler mock_remote_journaler; MockPrepareLocalImageRequest mock_prepare_local_image_request; + MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; MockReplay mock_local_replay; MockEventPreprocessor mock_event_preprocessor; @@ -696,6 +821,8 @@ TEST_F(TestMockImageReplayer, StopError) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); + expect_send(mock_prepare_remote_image_request, "remote mirror uuid", + m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); expect_send(mock_bootstrap_request, mock_local_image_ctx, false, 0); @@ -746,6 +873,7 @@ TEST_F(TestMockImageReplayer, Replay) { journal::MockJournaler mock_remote_journaler; MockPrepareLocalImageRequest mock_prepare_local_image_request; + MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; MockReplay mock_local_replay; MockEventPreprocessor mock_event_preprocessor; @@ -759,6 +887,8 @@ TEST_F(TestMockImageReplayer, Replay) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); + expect_send(mock_prepare_remote_image_request, "remote mirror uuid", + m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); expect_send(mock_bootstrap_request, mock_local_image_ctx, false, 0); @@ -847,6 +977,7 @@ TEST_F(TestMockImageReplayer, DecodeError) { journal::MockJournaler mock_remote_journaler; MockPrepareLocalImageRequest mock_prepare_local_image_request; + MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; MockReplay mock_local_replay; MockEventPreprocessor mock_event_preprocessor; @@ -859,6 +990,8 @@ TEST_F(TestMockImageReplayer, DecodeError) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); + expect_send(mock_prepare_remote_image_request, "remote mirror uuid", + m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); expect_send(mock_bootstrap_request, mock_local_image_ctx, false, 0); @@ -940,6 +1073,7 @@ TEST_F(TestMockImageReplayer, DelayedReplay) { journal::MockJournaler mock_remote_journaler; MockPrepareLocalImageRequest mock_prepare_local_image_request; + MockPrepareRemoteImageRequest mock_prepare_remote_image_request; MockBootstrapRequest mock_bootstrap_request; MockReplay mock_local_replay; MockEventPreprocessor mock_event_preprocessor; @@ -953,6 +1087,8 @@ TEST_F(TestMockImageReplayer, DelayedReplay) { InSequence seq; expect_send(mock_prepare_local_image_request, mock_local_image_ctx.id, "remote mirror uuid", 0); + expect_send(mock_prepare_remote_image_request, "remote mirror uuid", + m_remote_image_ctx->id, 0); EXPECT_CALL(mock_remote_journaler, construct()); expect_send(mock_bootstrap_request, mock_local_image_ctx, false, 0); diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc index f68d51d40d8..e43cfb93956 100644 --- a/src/tools/rbd_mirror/ImageReplayer.cc +++ b/src/tools/rbd_mirror/ImageReplayer.cc @@ -26,6 +26,7 @@ #include "tools/rbd_mirror/image_replayer/CloseImageRequest.h" #include "tools/rbd_mirror/image_replayer/EventPreprocessor.h" #include "tools/rbd_mirror/image_replayer/PrepareLocalImageRequest.h" +#include "tools/rbd_mirror/image_replayer/PrepareRemoteImageRequest.h" #include "tools/rbd_mirror/image_replayer/ReplayStatusFormatter.h" #define dout_context g_ceph_context @@ -452,13 +453,12 @@ void ImageReplayer::handle_prepare_local_image(int r) { } // local image doesn't exist or is non-primary - bootstrap(); + prepare_remote_image(); } template -void ImageReplayer::bootstrap() { +void ImageReplayer::prepare_remote_image() { dout(20) << dendl; - if (m_remote_images.empty()) { on_start_fail(-EREMOTEIO, "waiting for primary remote image"); return; @@ -467,6 +467,44 @@ void ImageReplayer::bootstrap() { // TODO bootstrap will need to support multiple remote images m_remote_image = *m_remote_images.begin(); + Context *ctx = create_context_callback< + ImageReplayer, &ImageReplayer::handle_prepare_remote_image>(this); + auto req = PrepareRemoteImageRequest::create( + m_remote_image.io_ctx, m_global_image_id, &m_remote_image.mirror_uuid, + &m_remote_image.image_id, ctx); + req->send(); +} + +template +void ImageReplayer::handle_prepare_remote_image(int r) { + dout(20) << "r=" << r << dendl; + + if (r == -ENOENT) { + dout(20) << "remote image does not exist" << dendl; + + if (!m_local_image_id.empty() && + m_local_image_tag_owner == m_remote_image.mirror_uuid) { + // local image exists and is non-primary and linked to the missing + // remote image + + // TODO schedule image deletion + on_start_fail(0, "remote image no longer exists"); + } else { + on_start_fail(-ENOENT, "remote image does not exist"); + } + return; + } else if (r < 0) { + on_start_fail(r, "error retrieving remote image id"); + return; + } + + bootstrap(); +} + +template +void ImageReplayer::bootstrap() { + dout(20) << dendl; + CephContext *cct = static_cast(m_local->cct()); journal::Settings settings; settings.commit_interval = cct->_conf->rbd_mirror_journal_commit_age; diff --git a/src/tools/rbd_mirror/ImageReplayer.h b/src/tools/rbd_mirror/ImageReplayer.h index 3ea41dc5378..11ff0b35794 100644 --- a/src/tools/rbd_mirror/ImageReplayer.h +++ b/src/tools/rbd_mirror/ImageReplayer.h @@ -138,6 +138,9 @@ protected: * PREPARE_LOCAL_IMAGE * * * * * * * * * * * * * * * * * * * | * * v (error) * + * PREPARE_REMOTE_IMAGE * * * * * * * * * * * * * * * * * * + * | * + * v (error) * * BOOTSTRAP_IMAGE * * * * * * * * * * * * * * * * * * * * * | * * v (error) * @@ -397,6 +400,9 @@ private: void prepare_local_image(); void handle_prepare_local_image(int r); + void prepare_remote_image(); + void handle_prepare_remote_image(int r); + void bootstrap(); void handle_bootstrap(int r); -- 2.39.5