From 57084cb3da1c3de5101c0e13efa6c64dff0fb60d Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 28 Jan 2020 20:37:44 -0500 Subject: [PATCH] rbd-mirror: image deleter should support snapshot-based mirroring Read the current mirror info from the local image instead of directly reading from the journal. The image needs to be opened before potentially resetting the journal since journaling might not be enabled. Signed-off-by: Jason Dillaman --- .../test_mock_TrashMoveRequest.cc | 414 ++++++++++++------ src/tools/rbd_mirror/ImageDeleter.cc | 1 - .../image_deleter/TrashMoveRequest.cc | 120 ++--- .../image_deleter/TrashMoveRequest.h | 26 +- .../image_deleter/TrashRemoveRequest.cc | 1 - 5 files changed, 356 insertions(+), 206 deletions(-) diff --git a/src/test/rbd_mirror/image_deleter/test_mock_TrashMoveRequest.cc b/src/test/rbd_mirror/image_deleter/test_mock_TrashMoveRequest.cc index 4b101221a5d..7a712999cdd 100644 --- a/src/test/rbd_mirror/image_deleter/test_mock_TrashMoveRequest.cc +++ b/src/test/rbd_mirror/image_deleter/test_mock_TrashMoveRequest.cc @@ -8,6 +8,7 @@ #include "librbd/Operations.h" #include "librbd/TrashWatcher.h" #include "librbd/journal/ResetRequest.h" +#include "librbd/mirror/GetInfoRequest.h" #include "librbd/trash/MoveRequest.h" #include "tools/rbd_mirror/Threads.h" #include "tools/rbd_mirror/image_deleter/TrashMoveRequest.h" @@ -41,29 +42,6 @@ MockTestImageCtx *MockTestImageCtx::s_instance = nullptr; } // anonymous namespace -template <> -struct Journal { - static Journal *s_instance; - - static void get_tag_owner(librados::IoCtx &io_ctx, - const std::string &image_id, - std::string *mirror_uuid, - ContextWQ *work_queue, - Context *on_finish) { - ceph_assert(s_instance != nullptr); - s_instance->get_tag_owner(image_id, mirror_uuid, on_finish); - } - - MOCK_METHOD3(get_tag_owner, void(const std::string &, std::string *, - Context *)); - - Journal() { - s_instance = this; - } -}; - -Journal* Journal::s_instance = nullptr; - template<> struct TrashWatcher { static TrashWatcher* s_instance; @@ -114,6 +92,45 @@ ResetRequest* ResetRequest::s_instance = nul } // namespace journal +namespace mirror { + +template<> +struct GetInfoRequest { + static GetInfoRequest* s_instance; + cls::rbd::MirrorImage *mirror_image; + PromotionState *promotion_state; + std::string *primary_mirror_uuid; + Context *on_finish = nullptr; + + static GetInfoRequest* create(librados::IoCtx& io_ctx, + ContextWQ* context_wq, + const std::string& image_id, + 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->primary_mirror_uuid = primary_mirror_uuid; + 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 trash { template <> @@ -165,8 +182,8 @@ using ::testing::WithArgs; class TestMockImageDeleterTrashMoveRequest : public TestMockFixture { public: typedef TrashMoveRequest MockTrashMoveRequest; - typedef librbd::Journal MockJournal; typedef librbd::journal::ResetRequest MockJournalResetRequest; + typedef librbd::mirror::GetInfoRequest MockGetMirrorInfoRequest; typedef librbd::trash::MoveRequest MockLibrbdTrashMoveRequest; typedef librbd::TrashWatcher MockTrashWatcher; @@ -190,14 +207,21 @@ public: Return(r))); } - void expect_get_tag_owner(MockJournal &mock_journal, - const std::string &image_id, - const std::string &tag_owner, int r) { - EXPECT_CALL(mock_journal, get_tag_owner(image_id, _, _)) - .WillOnce(WithArgs<1, 2>(Invoke([this, tag_owner, r](std::string *owner, Context *on_finish) { - *owner = tag_owner; - m_threads->work_queue->queue(on_finish, r); - }))); + void expect_get_mirror_info( + MockGetMirrorInfoRequest &mock_get_mirror_info_request, + const cls::rbd::MirrorImage &mirror_image, + librbd::mirror::PromotionState promotion_state, + const std::string& primary_mirror_uuid, int r) { + EXPECT_CALL(mock_get_mirror_info_request, send()) + .WillOnce(Invoke([this, &mock_get_mirror_info_request, mirror_image, + promotion_state, primary_mirror_uuid, r]() { + *mock_get_mirror_info_request.mirror_image = mirror_image; + *mock_get_mirror_info_request.promotion_state = promotion_state; + *mock_get_mirror_info_request.primary_mirror_uuid = + primary_mirror_uuid; + m_threads->work_queue->queue( + mock_get_mirror_info_request.on_finish, r); + })); } void expect_set_journal_policy(librbd::MockTestImageCtx &mock_image_ctx) { @@ -237,13 +261,8 @@ public: } void expect_mirror_image_set(const std::string& image_id, - const std::string& global_image_id, - cls::rbd::MirrorImageState mirror_image_state, + const cls::rbd::MirrorImage& mirror_image, int r) { - cls::rbd::MirrorImage mirror_image; - mirror_image.global_image_id = global_image_id; - mirror_image.state = mirror_image_state; - bufferlist bl; encode(image_id, bl); encode(mirror_image, bl); @@ -303,7 +322,7 @@ public: librbd::ImageCtx *m_local_image_ctx; }; -TEST_F(TestMockImageDeleterTrashMoveRequest, Success) { +TEST_F(TestMockImageDeleterTrashMoveRequest, SuccessJournal) { librbd::MockTestImageCtx mock_image_ctx(*m_local_image_ctx); librbd::MockExclusiveLock mock_exclusive_lock; mock_image_ctx.exclusive_lock = &mock_exclusive_lock; @@ -311,17 +330,25 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, Success) { InSequence seq; expect_mirror_image_get_image_id("image id", 0); - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", - librbd::Journal<>::ORPHAN_MIRROR_UUID, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_ORPHAN, + "remote mirror uuid", 0); - MockJournalResetRequest mock_journal_reset_request; - expect_journal_reset(mock_journal_reset_request, 0); + expect_mirror_image_set("image id", + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_DISABLING}, 0); expect_set_journal_policy(mock_image_ctx); expect_open(mock_image_ctx, 0); + + MockJournalResetRequest mock_journal_reset_request; + expect_journal_reset(mock_journal_reset_request, 0); + expect_block_requests(mock_image_ctx); expect_acquire_lock(mock_image_ctx, 0); @@ -345,6 +372,48 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, Success) { ASSERT_EQ(0, ctx.wait()); } +TEST_F(TestMockImageDeleterTrashMoveRequest, SuccessSnapshot) { + librbd::MockTestImageCtx mock_image_ctx(*m_local_image_ctx); + + InSequence seq; + expect_mirror_image_get_image_id("image id", 0); + + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_NON_PRIMARY, + "remote mirror uuid", 0); + + expect_mirror_image_set("image id", + {cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_DISABLING}, 0); + + expect_set_journal_policy(mock_image_ctx); + expect_open(mock_image_ctx, 0); + + MockLibrbdTrashMoveRequest mock_librbd_trash_move_request; + expect_trash_move(mock_librbd_trash_move_request, m_image_name, "image id", + {}, 0); + expect_mirror_image_remove(m_local_io_ctx, 0); + + expect_close(mock_image_ctx, 0); + expect_destroy(mock_image_ctx); + + MockTrashWatcher mock_trash_watcher; + expect_notify_image_added(mock_trash_watcher, "image id"); + + C_SaferCond ctx; + auto req = MockTrashMoveRequest::create(m_local_io_ctx, "global image id", + false, + m_local_image_ctx->op_work_queue, + &ctx); + req->send(); + ASSERT_EQ(0, ctx.wait()); +} + TEST_F(TestMockImageDeleterTrashMoveRequest, GetImageIdDNE) { InSequence seq; expect_mirror_image_get_image_id("image id", -ENOENT); @@ -371,14 +440,17 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, GetImageIdError) { ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockImageDeleterTrashMoveRequest, GetTagOwnerLocalPrimary) { +TEST_F(TestMockImageDeleterTrashMoveRequest, GetMirrorInfoLocalPrimary) { InSequence seq; expect_mirror_image_get_image_id("image id", 0); - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", - librbd::Journal<>::LOCAL_MIRROR_UUID, 0); - + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_PRIMARY, + "remote mirror uuid", 0); C_SaferCond ctx; auto req = MockTrashMoveRequest::create(m_local_io_ctx, "global image id", @@ -389,14 +461,17 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, GetTagOwnerLocalPrimary) { ASSERT_EQ(-EPERM, ctx.wait()); } -TEST_F(TestMockImageDeleterTrashMoveRequest, GetTagOwnerOrphan) { +TEST_F(TestMockImageDeleterTrashMoveRequest, GetMirrorInfoOrphan) { InSequence seq; expect_mirror_image_get_image_id("image id", 0); - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", - librbd::Journal<>::ORPHAN_MIRROR_UUID, 0); - + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_ORPHAN, + "remote mirror uuid", 0); C_SaferCond ctx; auto req = MockTrashMoveRequest::create(m_local_io_ctx, "global image id", @@ -407,7 +482,7 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, GetTagOwnerOrphan) { ASSERT_EQ(-EPERM, ctx.wait()); } -TEST_F(TestMockImageDeleterTrashMoveRequest, GetTagOwnerDNE) { +TEST_F(TestMockImageDeleterTrashMoveRequest, GetMirrorInfoDNE) { librbd::MockTestImageCtx mock_image_ctx(*m_local_image_ctx); librbd::MockExclusiveLock mock_exclusive_lock; mock_image_ctx.exclusive_lock = &mock_exclusive_lock; @@ -415,29 +490,13 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, GetTagOwnerDNE) { InSequence seq; expect_mirror_image_get_image_id("image id", 0); - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", "remote uuid", -ENOENT); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); - - MockJournalResetRequest mock_journal_reset_request; - expect_journal_reset(mock_journal_reset_request, 0); - - expect_set_journal_policy(mock_image_ctx); - expect_open(mock_image_ctx, 0); - expect_block_requests(mock_image_ctx); - expect_acquire_lock(mock_image_ctx, 0); - - MockLibrbdTrashMoveRequest mock_librbd_trash_move_request; - expect_trash_move(mock_librbd_trash_move_request, m_image_name, "image id", - {}, 0); - expect_mirror_image_remove(m_local_io_ctx, 0); - - expect_close(mock_image_ctx, 0); - expect_destroy(mock_image_ctx); - - MockTrashWatcher mock_trash_watcher; - expect_notify_image_added(mock_trash_watcher, "image id"); + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_ORPHAN, + "remote mirror uuid", -ENOENT); C_SaferCond ctx; auto req = MockTrashMoveRequest::create(m_local_io_ctx, "global image id", @@ -445,15 +504,20 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, GetTagOwnerDNE) { m_local_image_ctx->op_work_queue, &ctx); req->send(); - ASSERT_EQ(0, ctx.wait()); + ASSERT_EQ(-ENOENT, ctx.wait()); } -TEST_F(TestMockImageDeleterTrashMoveRequest, GetTagOwnerError) { +TEST_F(TestMockImageDeleterTrashMoveRequest, GetMirrorInfoError) { InSequence seq; expect_mirror_image_get_image_id("image id", 0); - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", "remote uuid", -EINVAL); + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_ORPHAN, + "remote mirror uuid", -EINVAL); C_SaferCond ctx; auto req = MockTrashMoveRequest::create(m_local_io_ctx, "global image id", @@ -468,11 +532,18 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, DisableMirrorImageError) { InSequence seq; expect_mirror_image_get_image_id("image id", 0); - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", - librbd::Journal<>::ORPHAN_MIRROR_UUID, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, -EINVAL); + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_ORPHAN, + "remote mirror uuid", 0); + + expect_mirror_image_set("image id", + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_DISABLING}, -EINVAL); C_SaferCond ctx; auto req = MockTrashMoveRequest::create(m_local_io_ctx, "global image id", @@ -483,18 +554,30 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, DisableMirrorImageError) { ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockImageDeleterTrashMoveRequest, ResetJournalError) { +TEST_F(TestMockImageDeleterTrashMoveRequest, OpenImageError) { + librbd::MockTestImageCtx mock_image_ctx(*m_local_image_ctx); + librbd::MockExclusiveLock mock_exclusive_lock; + mock_image_ctx.exclusive_lock = &mock_exclusive_lock; + InSequence seq; expect_mirror_image_get_image_id("image id", 0); - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", - librbd::Journal<>::ORPHAN_MIRROR_UUID, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_ORPHAN, + "remote mirror uuid", 0); - MockJournalResetRequest mock_journal_reset_request; - expect_journal_reset(mock_journal_reset_request, -EINVAL); + expect_mirror_image_set("image id", + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_DISABLING}, 0); + + expect_set_journal_policy(mock_image_ctx); + expect_open(mock_image_ctx, -EINVAL); + expect_destroy(mock_image_ctx); C_SaferCond ctx; auto req = MockTrashMoveRequest::create(m_local_io_ctx, "global image id", @@ -505,7 +588,7 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, ResetJournalError) { ASSERT_EQ(-EINVAL, ctx.wait()); } -TEST_F(TestMockImageDeleterTrashMoveRequest, OpenImageError) { +TEST_F(TestMockImageDeleterTrashMoveRequest, ResetJournalError) { librbd::MockTestImageCtx mock_image_ctx(*m_local_image_ctx); librbd::MockExclusiveLock mock_exclusive_lock; mock_image_ctx.exclusive_lock = &mock_exclusive_lock; @@ -513,17 +596,26 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, OpenImageError) { InSequence seq; expect_mirror_image_get_image_id("image id", 0); - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", - librbd::Journal<>::ORPHAN_MIRROR_UUID, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_ORPHAN, + "remote mirror uuid", 0); - MockJournalResetRequest mock_journal_reset_request; - expect_journal_reset(mock_journal_reset_request, 0); + expect_mirror_image_set("image id", + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_DISABLING}, 0); expect_set_journal_policy(mock_image_ctx); - expect_open(mock_image_ctx, -EINVAL); + expect_open(mock_image_ctx, 0); + + MockJournalResetRequest mock_journal_reset_request; + expect_journal_reset(mock_journal_reset_request, -EINVAL); + + expect_close(mock_image_ctx, 0); expect_destroy(mock_image_ctx); C_SaferCond ctx; @@ -543,17 +635,25 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, AcquireLockError) { InSequence seq; expect_mirror_image_get_image_id("image id", 0); - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", - librbd::Journal<>::ORPHAN_MIRROR_UUID, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_ORPHAN, + "remote mirror uuid", 0); - MockJournalResetRequest mock_journal_reset_request; - expect_journal_reset(mock_journal_reset_request, 0); + expect_mirror_image_set("image id", + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_DISABLING}, 0); expect_set_journal_policy(mock_image_ctx); expect_open(mock_image_ctx, 0); + + MockJournalResetRequest mock_journal_reset_request; + expect_journal_reset(mock_journal_reset_request, 0); + expect_block_requests(mock_image_ctx); expect_acquire_lock(mock_image_ctx, -EINVAL); @@ -577,17 +677,25 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, TrashMoveError) { InSequence seq; expect_mirror_image_get_image_id("image id", 0); - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", - librbd::Journal<>::ORPHAN_MIRROR_UUID, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_ORPHAN, + "remote mirror uuid", 0); - MockJournalResetRequest mock_journal_reset_request; - expect_journal_reset(mock_journal_reset_request, 0); + expect_mirror_image_set("image id", + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_DISABLING}, 0); expect_set_journal_policy(mock_image_ctx); expect_open(mock_image_ctx, 0); + + MockJournalResetRequest mock_journal_reset_request; + expect_journal_reset(mock_journal_reset_request, 0); + expect_block_requests(mock_image_ctx); expect_acquire_lock(mock_image_ctx, 0); @@ -615,17 +723,25 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, RemoveMirrorImageError) { InSequence seq; expect_mirror_image_get_image_id("image id", 0); - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", - librbd::Journal<>::ORPHAN_MIRROR_UUID, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_ORPHAN, + "remote mirror uuid", 0); - MockJournalResetRequest mock_journal_reset_request; - expect_journal_reset(mock_journal_reset_request, 0); + expect_mirror_image_set("image id", + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_DISABLING}, 0); expect_set_journal_policy(mock_image_ctx); expect_open(mock_image_ctx, 0); + + MockJournalResetRequest mock_journal_reset_request; + expect_journal_reset(mock_journal_reset_request, 0); + expect_block_requests(mock_image_ctx); expect_acquire_lock(mock_image_ctx, 0); @@ -657,17 +773,25 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, CloseImageError) { InSequence seq; expect_mirror_image_get_image_id("image id", 0); - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", - librbd::Journal<>::ORPHAN_MIRROR_UUID, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_ORPHAN, + "remote mirror uuid", 0); - MockJournalResetRequest mock_journal_reset_request; - expect_journal_reset(mock_journal_reset_request, 0); + expect_mirror_image_set("image id", + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_DISABLING}, 0); expect_set_journal_policy(mock_image_ctx); expect_open(mock_image_ctx, 0); + + MockJournalResetRequest mock_journal_reset_request; + expect_journal_reset(mock_journal_reset_request, 0); + expect_block_requests(mock_image_ctx); expect_acquire_lock(mock_image_ctx, 0); @@ -700,17 +824,25 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, DelayedDelation) { InSequence seq; expect_mirror_image_get_image_id("image id", 0); - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", - librbd::Journal<>::ORPHAN_MIRROR_UUID, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); + MockGetMirrorInfoRequest mock_get_mirror_info_request; + expect_get_mirror_info(mock_get_mirror_info_request, + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_ENABLED}, + librbd::mirror::PROMOTION_STATE_NON_PRIMARY, + "remote mirror uuid", 0); - MockJournalResetRequest mock_journal_reset_request; - expect_journal_reset(mock_journal_reset_request, 0); + expect_mirror_image_set("image id", + {cls::rbd::MIRROR_IMAGE_MODE_JOURNAL, + "global image id", + cls::rbd::MIRROR_IMAGE_STATE_DISABLING}, 0); expect_set_journal_policy(mock_image_ctx); expect_open(mock_image_ctx, 0); + + MockJournalResetRequest mock_journal_reset_request; + expect_journal_reset(mock_journal_reset_request, 0); + expect_block_requests(mock_image_ctx); expect_acquire_lock(mock_image_ctx, 0); diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc index e3c808c0b53..0f3f5162297 100644 --- a/src/tools/rbd_mirror/ImageDeleter.cc +++ b/src/tools/rbd_mirror/ImageDeleter.cc @@ -23,7 +23,6 @@ #include "librbd/internal.h" #include "librbd/ImageCtx.h" #include "librbd/ImageState.h" -#include "librbd/Journal.h" #include "librbd/Operations.h" #include "cls/rbd/cls_rbd_client.h" #include "cls/rbd/cls_rbd_types.h" diff --git a/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc b/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc index 6c206dcba25..87b48fdc828 100644 --- a/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc +++ b/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc @@ -13,6 +13,7 @@ #include "librbd/TrashWatcher.h" #include "librbd/Utils.h" #include "librbd/journal/ResetRequest.h" +#include "librbd/mirror/GetInfoRequest.h" #include "librbd/trash/MoveRequest.h" #include "tools/rbd_mirror/image_deleter/Types.h" @@ -69,39 +70,45 @@ void TrashMoveRequest::handle_get_mirror_image_id(int r) { return; } - get_tag_owner(); + get_mirror_info(); } template -void TrashMoveRequest::get_tag_owner() { +void TrashMoveRequest::get_mirror_info() { dout(10) << dendl; auto ctx = create_context_callback< - TrashMoveRequest, &TrashMoveRequest::handle_get_tag_owner>(this); - librbd::Journal::get_tag_owner(m_io_ctx, m_image_id, &m_mirror_uuid, - m_op_work_queue, ctx); + TrashMoveRequest, &TrashMoveRequest::handle_get_mirror_info>(this); + auto req = librbd::mirror::GetInfoRequest::create( + m_io_ctx, m_op_work_queue, m_image_id, &m_mirror_image, &m_promotion_state, + &m_primary_mirror_uuid, ctx); + req->send(); } template -void TrashMoveRequest::handle_get_tag_owner(int r) { +void TrashMoveRequest::handle_get_mirror_info(int r) { dout(10) << "r=" << r << dendl; - if (r < 0 && r != -ENOENT) { + if (r == -ENOENT) { + dout(5) << "image " << m_global_image_id << " is not mirrored" << dendl; + finish(r); + return; + } else if (r < 0) { derr << "error retrieving image primary info for image " << m_global_image_id << ": " << cpp_strerror(r) << dendl; finish(r); return; - } else if (r != -ENOENT) { - if (m_mirror_uuid == librbd::Journal<>::LOCAL_MIRROR_UUID) { - dout(10) << "image " << m_global_image_id << " is local primary" << dendl; - finish(-EPERM); - return; - } else if (m_mirror_uuid == librbd::Journal<>::ORPHAN_MIRROR_UUID && - !m_resync) { - dout(10) << "image " << m_global_image_id << " is orphaned" << dendl; - finish(-EPERM); - return; - } + } + + if (m_promotion_state == librbd::mirror::PROMOTION_STATE_PRIMARY) { + dout(10) << "image " << m_global_image_id << " is local primary" << dendl; + finish(-EPERM); + return; + } else if (m_promotion_state == librbd::mirror::PROMOTION_STATE_ORPHAN && + !m_resync) { + dout(10) << "image " << m_global_image_id << " is orphaned" << dendl; + finish(-EPERM); + return; } disable_mirror_image(); @@ -111,12 +118,10 @@ template void TrashMoveRequest::disable_mirror_image() { dout(10) << dendl; - cls::rbd::MirrorImage mirror_image; - mirror_image.global_image_id = m_global_image_id; - mirror_image.state = cls::rbd::MIRROR_IMAGE_STATE_DISABLING; + m_mirror_image.state = cls::rbd::MIRROR_IMAGE_STATE_DISABLING; librados::ObjectWriteOperation op; - librbd::cls_client::mirror_image_set(&op, m_image_id, mirror_image); + librbd::cls_client::mirror_image_set(&op, m_image_id, m_mirror_image); auto aio_comp = create_rados_callback< TrashMoveRequest, @@ -147,32 +152,6 @@ void TrashMoveRequest::handle_disable_mirror_image(int r) { return; } - reset_journal(); -} - -template -void TrashMoveRequest::reset_journal() { - dout(10) << dendl; - - // ensure that if the image is recovered any peers will split-brain - auto ctx = create_context_callback< - TrashMoveRequest, &TrashMoveRequest::handle_reset_journal>(this); - auto req = librbd::journal::ResetRequest::create( - m_io_ctx, m_image_id, librbd::Journal<>::IMAGE_CLIENT_ID, - librbd::Journal<>::LOCAL_MIRROR_UUID, m_op_work_queue, ctx); - req->send(); -} - -template -void TrashMoveRequest::handle_reset_journal(int r) { - dout(10) << "r=" << r << dendl; - - if (r < 0 && r != -ENOENT) { - derr << "failed to reset journal: " << cpp_strerror(r) << dendl; - finish(r); - return; - } - open_image(); } @@ -212,6 +191,39 @@ void TrashMoveRequest::handle_open_image(int r) { return; } + reset_journal(); +} + +template +void TrashMoveRequest::reset_journal() { + if (m_mirror_image.mode == cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT) { + // snapshot-based mirroring doesn't require journal feature + acquire_lock(); + return; + } + + dout(10) << dendl; + + // ensure that if the image is recovered any peers will split-brain + auto ctx = create_context_callback< + TrashMoveRequest, &TrashMoveRequest::handle_reset_journal>(this); + auto req = librbd::journal::ResetRequest::create( + m_io_ctx, m_image_id, librbd::Journal<>::IMAGE_CLIENT_ID, + librbd::Journal<>::LOCAL_MIRROR_UUID, m_op_work_queue, ctx); + req->send(); +} + +template +void TrashMoveRequest::handle_reset_journal(int r) { + dout(10) << "r=" << r << dendl; + + if (r < 0 && r != -ENOENT) { + derr << "failed to reset journal: " << cpp_strerror(r) << dendl; + m_ret_val = r; + close_image(); + return; + } + acquire_lock(); } @@ -219,10 +231,16 @@ template void TrashMoveRequest::acquire_lock() { m_image_ctx->owner_lock.lock_shared(); if (m_image_ctx->exclusive_lock == nullptr) { - derr << "exclusive lock feature not enabled" << dendl; m_image_ctx->owner_lock.unlock_shared(); - m_ret_val = -EINVAL; - close_image(); + + if (m_mirror_image.mode == cls::rbd::MIRROR_IMAGE_MODE_SNAPSHOT) { + // snapshot-based mirroring doesn't require exclusive-lock + trash_move(); + } else { + derr << "exclusive lock feature not enabled" << dendl; + m_ret_val = -EINVAL; + close_image(); + } return; } diff --git a/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.h b/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.h index 07b7432edf1..fa0d7daca1b 100644 --- a/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.h +++ b/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.h @@ -7,7 +7,7 @@ #include "include/buffer.h" #include "include/rados/librados.hpp" #include "cls/rbd/cls_rbd_types.h" -#include +#include "librbd/mirror/Types.h" #include struct Context; @@ -47,18 +47,18 @@ private: * GET_MIRROR_IMAGE_ID * | * v - * GET_TAG_OWNER + * GET_MIRROR_INFO * | * v * DISABLE_MIRROR_IMAGE * | * v - * RESET_JOURNAL - * | - * v * OPEN_IMAGE * | - * v + * v (skip if not needed) + * RESET_JOURNAL + * | + * v (skip if not needed) * ACQUIRE_LOCK * | * v @@ -87,7 +87,9 @@ private: ceph::bufferlist m_out_bl; std::string m_image_id; - std::string m_mirror_uuid; + cls::rbd::MirrorImage m_mirror_image; + librbd::mirror::PromotionState m_promotion_state; + std::string m_primary_mirror_uuid; cls::rbd::TrashImageSpec m_trash_image_spec; ImageCtxT *m_image_ctx = nullptr;; int m_ret_val = 0; @@ -96,18 +98,18 @@ private: void get_mirror_image_id(); void handle_get_mirror_image_id(int r); - void get_tag_owner(); - void handle_get_tag_owner(int r); + void get_mirror_info(); + void handle_get_mirror_info(int r); void disable_mirror_image(); void handle_disable_mirror_image(int r); - void reset_journal(); - void handle_reset_journal(int r); - void open_image(); void handle_open_image(int r); + void reset_journal(); + void handle_reset_journal(int r); + void acquire_lock(); void handle_acquire_lock(int r); diff --git a/src/tools/rbd_mirror/image_deleter/TrashRemoveRequest.cc b/src/tools/rbd_mirror/image_deleter/TrashRemoveRequest.cc index e7c725dc9c2..2ffcdfa4bd0 100644 --- a/src/tools/rbd_mirror/image_deleter/TrashRemoveRequest.cc +++ b/src/tools/rbd_mirror/image_deleter/TrashRemoveRequest.cc @@ -8,7 +8,6 @@ #include "common/WorkQueue.h" #include "cls/rbd/cls_rbd_client.h" #include "librbd/ImageCtx.h" -#include "librbd/Journal.h" #include "librbd/TrashWatcher.h" #include "librbd/Utils.h" #include "librbd/trash/RemoveRequest.h" -- 2.39.5