From 3ccdc3f934058c31987f731c92a77bf3efbe331e Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 14 Dec 2017 16:47:13 -0500 Subject: [PATCH] rbd-mirror: image deleter now only removes images from the trash Signed-off-by: Jason Dillaman --- .../image_deleter/test_mock_RemoveRequest.cc | 307 +----------------- src/test/rbd_mirror/test_ImageDeleter.cc | 207 ++---------- src/tools/rbd_mirror/ImageDeleter.cc | 83 ++--- src/tools/rbd_mirror/ImageDeleter.h | 33 +- .../rbd_mirror/image_deleter/RemoveRequest.cc | 156 +-------- .../rbd_mirror/image_deleter/RemoveRequest.h | 46 +-- 6 files changed, 75 insertions(+), 757 deletions(-) diff --git a/src/test/rbd_mirror/image_deleter/test_mock_RemoveRequest.cc b/src/test/rbd_mirror/image_deleter/test_mock_RemoveRequest.cc index 8eed1f8e8f5fb..2392baeb41c66 100644 --- a/src/test/rbd_mirror/image_deleter/test_mock_RemoveRequest.cc +++ b/src/test/rbd_mirror/image_deleter/test_mock_RemoveRequest.cc @@ -4,7 +4,6 @@ #include "test/rbd_mirror/test_mock_fixture.h" #include "cls/rbd/cls_rbd_types.h" #include "librbd/ImageCtx.h" -#include "librbd/Journal.h" #include "librbd/Utils.h" #include "librbd/image/RemoveRequest.h" #include "tools/rbd_mirror/Threads.h" @@ -25,29 +24,6 @@ struct MockTestImageCtx : public librbd::MockImageCtx { } // 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) { - 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; - namespace image { template <> @@ -66,7 +42,7 @@ struct RemoveRequest { assert(s_instance != nullptr); EXPECT_TRUE(image_name.empty()); EXPECT_TRUE(force); - EXPECT_FALSE(remove_from_trash); + EXPECT_TRUE(remove_from_trash); s_instance->construct(image_id); s_instance->on_finish = on_finish; return s_instance; @@ -135,31 +111,8 @@ class TestMockImageDeleterRemoveRequest : public TestMockFixture { public: typedef RemoveRequest MockRemoveRequest; typedef SnapshotPurgeRequest MockSnapshotPurgeRequest; - typedef librbd::Journal MockJournal; typedef librbd::image::RemoveRequest MockImageRemoveRequest; - void expect_mirror_image_get_image_id(const std::string& image_id, int r) { - bufferlist bl; - ::encode(image_id, bl); - - EXPECT_CALL(get_mock_io_ctx(m_local_io_ctx), - exec(RBD_MIRRORING, _, StrEq("rbd"), StrEq("mirror_image_get_image_id"), _, _, _)) - .WillOnce(DoAll(WithArg<5>(Invoke([bl](bufferlist *out_bl) { - *out_bl = bl; - })), - 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_snapcontext(const std::string& image_id, const ::SnapContext &snapc, int r) { bufferlist bl; @@ -174,24 +127,6 @@ public: Return(r))); } - void expect_mirror_image_set(const std::string& image_id, - const std::string& global_image_id, - cls::rbd::MirrorImageState mirror_image_state, - 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); - - EXPECT_CALL(get_mock_io_ctx(m_local_io_ctx), - exec(RBD_MIRRORING, _, StrEq("rbd"), - StrEq("mirror_image_set"), ContentsEqual(bl), _, _)) - .WillOnce(Return(r)); - } - void expect_snapshot_purge(MockSnapshotPurgeRequest &snapshot_purge_request, const std::string &image_id, int r) { EXPECT_CALL(snapshot_purge_request, construct(image_id)); @@ -209,29 +144,11 @@ public: m_threads->work_queue->queue(image_remove_request.on_finish, r); })); } - - void expect_mirror_image_remove(const std::string& image_id, - int r) { - bufferlist bl; - ::encode(image_id, bl); - - EXPECT_CALL(get_mock_io_ctx(m_local_io_ctx), - exec(RBD_MIRRORING, _, StrEq("rbd"), - StrEq("mirror_image_remove"), ContentsEqual(bl), _, _)) - .WillOnce(Return(r)); - } }; TEST_F(TestMockImageDeleterRemoveRequest, Success) { InSequence seq; - expect_mirror_image_get_image_id("image id", 0); - - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", "remote uuid", 0); - expect_get_snapcontext("image id", {1, {1}}, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); MockSnapshotPurgeRequest mock_snapshot_purge_request; expect_snapshot_purge(mock_snapshot_purge_request, "image id", 0); @@ -239,140 +156,25 @@ TEST_F(TestMockImageDeleterRemoveRequest, Success) { MockImageRemoveRequest mock_image_remove_request; expect_image_remove(mock_image_remove_request, "image id", 0); - expect_mirror_image_remove("image id", 0); - - C_SaferCond ctx; - ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, - &error_result, m_threads->work_queue, - &ctx); - req->send(); - ASSERT_EQ(0, ctx.wait()); -} - -TEST_F(TestMockImageDeleterRemoveRequest, GetImageIdDNE) { - InSequence seq; - expect_mirror_image_get_image_id("image id", -ENOENT); - - C_SaferCond ctx; - ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, - &error_result, m_threads->work_queue, - &ctx); - req->send(); - ASSERT_EQ(-ENOENT, ctx.wait()); - ASSERT_EQ(ERROR_RESULT_COMPLETE, error_result); -} - -TEST_F(TestMockImageDeleterRemoveRequest, GetImageIdError) { - InSequence seq; - expect_mirror_image_get_image_id("image id", -EINVAL); - - C_SaferCond ctx; - ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, - &error_result, m_threads->work_queue, - &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - -TEST_F(TestMockImageDeleterRemoveRequest, GetTagOwnerLocalPrimary) { - 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); - - C_SaferCond ctx; - ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, - &error_result, m_threads->work_queue, - &ctx); - req->send(); - ASSERT_EQ(-EPERM, ctx.wait()); - ASSERT_EQ(ERROR_RESULT_COMPLETE, error_result); -} - -TEST_F(TestMockImageDeleterRemoveRequest, GetTagOwnerOrphan) { - 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); - - C_SaferCond ctx; - ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", false, - &error_result, m_threads->work_queue, - &ctx); - req->send(); - ASSERT_EQ(-EPERM, ctx.wait()); - ASSERT_EQ(ERROR_RESULT_COMPLETE, error_result); -} - -TEST_F(TestMockImageDeleterRemoveRequest, 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_get_snapcontext("image id", {1, {1}}, -ENOENT); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); - - MockImageRemoveRequest mock_image_remove_request; - expect_image_remove(mock_image_remove_request, "image id", 0); - - expect_mirror_image_remove("image id", 0); - C_SaferCond ctx; ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, + auto req = MockRemoveRequest::create(m_local_io_ctx, "image id", &error_result, m_threads->work_queue, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } -TEST_F(TestMockImageDeleterRemoveRequest, GetTagOwnerError) { - 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); - - C_SaferCond ctx; - ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, - &error_result, m_threads->work_queue, - &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - TEST_F(TestMockImageDeleterRemoveRequest, GetSnapContextDNE) { InSequence seq; - expect_mirror_image_get_image_id("image id", 0); - - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", "remote uuid", 0); - expect_get_snapcontext("image id", {1, {1}}, -ENOENT); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); MockImageRemoveRequest mock_image_remove_request; expect_image_remove(mock_image_remove_request, "image id", 0); - expect_mirror_image_remove("image id", 0); - C_SaferCond ctx; ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, + auto req = MockRemoveRequest::create(m_local_io_ctx, "image id", &error_result, m_threads->work_queue, &ctx); req->send(); @@ -381,57 +183,11 @@ TEST_F(TestMockImageDeleterRemoveRequest, GetSnapContextDNE) { TEST_F(TestMockImageDeleterRemoveRequest, GetSnapContextError) { InSequence seq; - expect_mirror_image_get_image_id("image id", 0); - - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", "remote uuid", 0); - expect_get_snapcontext("image id", {1, {1}}, -EINVAL); C_SaferCond ctx; ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, - &error_result, m_threads->work_queue, - &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - -TEST_F(TestMockImageDeleterRemoveRequest, SetMirrorDNE) { - InSequence seq; - expect_mirror_image_get_image_id("image id", 0); - - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", "remote uuid", 0); - - expect_get_snapcontext("image id", {1, {1}}, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, -ENOENT); - - C_SaferCond ctx; - ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, - &error_result, m_threads->work_queue, - &ctx); - req->send(); - ASSERT_EQ(-ENOENT, ctx.wait()); - ASSERT_EQ(ERROR_RESULT_COMPLETE, error_result); -} - -TEST_F(TestMockImageDeleterRemoveRequest, SetMirrorError) { - InSequence seq; - expect_mirror_image_get_image_id("image id", 0); - - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", "remote uuid", 0); - - expect_get_snapcontext("image id", {1, {1}}, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, -EINVAL); - - C_SaferCond ctx; - ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, + auto req = MockRemoveRequest::create(m_local_io_ctx, "image id", &error_result, m_threads->work_queue, &ctx); req->send(); @@ -440,21 +196,14 @@ TEST_F(TestMockImageDeleterRemoveRequest, SetMirrorError) { TEST_F(TestMockImageDeleterRemoveRequest, PurgeSnapshotBusy) { InSequence seq; - expect_mirror_image_get_image_id("image id", 0); - - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", "remote uuid", 0); - expect_get_snapcontext("image id", {1, {1}}, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); MockSnapshotPurgeRequest mock_snapshot_purge_request; expect_snapshot_purge(mock_snapshot_purge_request, "image id", -EBUSY); C_SaferCond ctx; ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, + auto req = MockRemoveRequest::create(m_local_io_ctx, "image id", &error_result, m_threads->work_queue, &ctx); req->send(); @@ -464,21 +213,14 @@ TEST_F(TestMockImageDeleterRemoveRequest, PurgeSnapshotBusy) { TEST_F(TestMockImageDeleterRemoveRequest, PurgeSnapshotError) { InSequence seq; - expect_mirror_image_get_image_id("image id", 0); - - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", "remote uuid", 0); - expect_get_snapcontext("image id", {1, {1}}, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); MockSnapshotPurgeRequest mock_snapshot_purge_request; expect_snapshot_purge(mock_snapshot_purge_request, "image id", -EINVAL); C_SaferCond ctx; ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, + auto req = MockRemoveRequest::create(m_local_io_ctx, "image id", &error_result, m_threads->work_queue, &ctx); req->send(); @@ -487,14 +229,7 @@ TEST_F(TestMockImageDeleterRemoveRequest, PurgeSnapshotError) { TEST_F(TestMockImageDeleterRemoveRequest, RemoveError) { InSequence seq; - expect_mirror_image_get_image_id("image id", 0); - - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", "remote uuid", 0); - expect_get_snapcontext("image id", {1, {1}}, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); MockSnapshotPurgeRequest mock_snapshot_purge_request; expect_snapshot_purge(mock_snapshot_purge_request, "image id", 0); @@ -504,35 +239,7 @@ TEST_F(TestMockImageDeleterRemoveRequest, RemoveError) { C_SaferCond ctx; ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, - &error_result, m_threads->work_queue, - &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - -TEST_F(TestMockImageDeleterRemoveRequest, RemoveMirrorError) { - InSequence seq; - expect_mirror_image_get_image_id("image id", 0); - - MockJournal mock_journal; - expect_get_tag_owner(mock_journal, "image id", "remote uuid", 0); - - expect_get_snapcontext("image id", {1, {1}}, 0); - expect_mirror_image_set("image id", "global image id", - cls::rbd::MIRROR_IMAGE_STATE_DISABLING, 0); - - MockSnapshotPurgeRequest mock_snapshot_purge_request; - expect_snapshot_purge(mock_snapshot_purge_request, "image id", 0); - - MockImageRemoveRequest mock_image_remove_request; - expect_image_remove(mock_image_remove_request, "image id", 0); - - expect_mirror_image_remove("image id", -EINVAL); - - C_SaferCond ctx; - ErrorResult error_result; - auto req = MockRemoveRequest::create(m_local_io_ctx, "global image id", true, + auto req = MockRemoveRequest::create(m_local_io_ctx, "image id", &error_result, m_threads->work_queue, &ctx); req->send(); diff --git a/src/test/rbd_mirror/test_ImageDeleter.cc b/src/test/rbd_mirror/test_ImageDeleter.cc index d2e36793aa487..37276f8032961 100644 --- a/src/test/rbd_mirror/test_ImageDeleter.cc +++ b/src/test/rbd_mirror/test_ImageDeleter.cc @@ -214,120 +214,61 @@ public: &mirror_image)); } + int trash_move(const std::string& global_image_id) { + C_SaferCond ctx; + rbd::mirror::ImageDeleter<>::trash_move(m_local_io_ctx, global_image_id, + true, m_threads->work_queue, &ctx); + return ctx.wait(); + } + librbd::RBD rbd; std::string m_local_image_id; std::unique_ptr> m_service_daemon; rbd::mirror::ImageDeleter<> *m_deleter; }; -TEST_F(TestImageDeleter, Delete_NonPrimary_Image) { - init_image_deleter(); - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, false, nullptr); - - C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(GLOBAL_IMAGE_ID, &ctx); - EXPECT_EQ(0, ctx.wait()); - - ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); - ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); - - check_image_deleted(); -} - -TEST_F(TestImageDeleter, Delete_Split_Brain_Image) { - init_image_deleter(); - promote_image(); - demote_image(); - - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, true, nullptr); +TEST_F(TestImageDeleter, ExistingTrashMove) { + ASSERT_EQ(0, trash_move(GLOBAL_IMAGE_ID)); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(GLOBAL_IMAGE_ID, &ctx); - EXPECT_EQ(0, ctx.wait()); - - ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); - ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); - - check_image_deleted(); -} - -TEST_F(TestImageDeleter, Fail_Delete_Primary_Image) { - init_image_deleter(); - promote_image(); - - C_SaferCond ctx; - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, false, &ctx); - EXPECT_EQ(-EPERM, ctx.wait()); - - ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); - ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); -} - -TEST_F(TestImageDeleter, Fail_Delete_Orphan_Image) { + m_deleter->wait_for_deletion(m_local_image_id, false, &ctx); init_image_deleter(); - promote_image(); - demote_image(); - C_SaferCond ctx; - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, false, &ctx); - EXPECT_EQ(-EPERM, ctx.wait()); - - ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); - ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); + ASSERT_EQ(0, ctx.wait()); } -TEST_F(TestImageDeleter, Delete_Image_With_Child) { +TEST_F(TestImageDeleter, LiveTrashMove) { init_image_deleter(); - create_snapshot(); - - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, false, nullptr); C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(GLOBAL_IMAGE_ID, &ctx); - EXPECT_EQ(0, ctx.wait()); + m_deleter->wait_for_deletion(m_local_image_id, false, &ctx); - ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); - ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); + ASSERT_EQ(0, trash_move(GLOBAL_IMAGE_ID)); + ASSERT_EQ(0, ctx.wait()); } -TEST_F(TestImageDeleter, Delete_Image_With_Children) { +TEST_F(TestImageDeleter, Delete_Image_With_Snapshots) { init_image_deleter(); create_snapshot("snap1"); create_snapshot("snap2"); - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, false, nullptr); - C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(GLOBAL_IMAGE_ID, &ctx); + m_deleter->wait_for_deletion(m_local_image_id, false, &ctx); + ASSERT_EQ(0, trash_move(GLOBAL_IMAGE_ID)); EXPECT_EQ(0, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); } -TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChild) { - init_image_deleter(); - create_snapshot("snap1", true); - - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, false, nullptr); - - C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(GLOBAL_IMAGE_ID, &ctx); - EXPECT_EQ(0, ctx.wait()); - - ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); - ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); -} - -TEST_F(TestImageDeleter, Delete_Image_With_ProtectedChildren) { +TEST_F(TestImageDeleter, Delete_Image_With_ProtectedSnapshots) { init_image_deleter(); create_snapshot("snap1", true); create_snapshot("snap2", true); - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, false, nullptr); - C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(GLOBAL_IMAGE_ID, &ctx); + m_deleter->wait_for_deletion(m_local_image_id, false, &ctx); + ASSERT_EQ(0, trash_move(GLOBAL_IMAGE_ID)); EXPECT_EQ(0, ctx.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); @@ -338,114 +279,22 @@ TEST_F(TestImageDeleter, Delete_Image_With_Clone) { init_image_deleter(); std::string clone_id = create_clone(); - C_SaferCond ctx; - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, false, &ctx); + C_SaferCond ctx1; m_deleter->set_busy_timer_interval(0.1); - EXPECT_EQ(-EBUSY, ctx.wait()); + m_deleter->wait_for_deletion(m_local_image_id, false, &ctx1); + ASSERT_EQ(0, trash_move(GLOBAL_IMAGE_ID)); + EXPECT_EQ(-EBUSY, ctx1.wait()); C_SaferCond ctx2; - m_deleter->schedule_image_delete(GLOBAL_CLONE_IMAGE_ID, false, &ctx2); + m_deleter->wait_for_deletion(clone_id, false, &ctx2); + ASSERT_EQ(0, trash_move(GLOBAL_CLONE_IMAGE_ID)); EXPECT_EQ(0, ctx2.wait()); C_SaferCond ctx3; - m_deleter->wait_for_scheduled_deletion(GLOBAL_IMAGE_ID, &ctx3); + m_deleter->wait_for_deletion(m_local_image_id, true, &ctx3); EXPECT_EQ(0, ctx3.wait()); ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); } -TEST_F(TestImageDeleter, Delete_NonExistent_Image) { - init_image_deleter(); - remove_image(); - - cls::rbd::MirrorImage mirror_image(GLOBAL_IMAGE_ID, - MirrorImageState::MIRROR_IMAGE_STATE_ENABLED); - EXPECT_EQ(0, cls_client::mirror_image_set(&m_local_io_ctx, m_local_image_id, - mirror_image)); - - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, false, nullptr); - - C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(GLOBAL_IMAGE_ID, &ctx); - EXPECT_EQ(0, ctx.wait()); - - ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); - ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); - - check_image_deleted(); -} - -TEST_F(TestImageDeleter, Delete_NonExistent_Image_With_MirroringState) { - init_image_deleter(); - remove_image(true); - - cls::rbd::MirrorImage mirror_image(GLOBAL_IMAGE_ID, - MirrorImageState::MIRROR_IMAGE_STATE_ENABLED); - EXPECT_EQ(0, cls_client::mirror_image_set(&m_local_io_ctx, m_local_image_id, - mirror_image)); - mirror_image.state = MirrorImageState::MIRROR_IMAGE_STATE_DISABLING; - EXPECT_EQ(0, cls_client::mirror_image_set(&m_local_io_ctx, m_local_image_id, - mirror_image)); - - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, false, nullptr); - - C_SaferCond ctx; - m_deleter->wait_for_scheduled_deletion(GLOBAL_IMAGE_ID, &ctx); - EXPECT_EQ(0, ctx.wait()); - - ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); - ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); - - check_image_deleted(); -} - -TEST_F(TestImageDeleter, Delete_NonExistent_Image_Without_MirroringState) { - init_image_deleter(); - remove_image(); - - C_SaferCond ctx; - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, false, &ctx); - EXPECT_EQ(-ENOENT, ctx.wait()); - - ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); - ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); - - check_image_deleted(); -} - -TEST_F(TestImageDeleter, Fail_Delete_NonPrimary_Image) { - init_image_deleter(); - ImageCtx *ictx = new ImageCtx("", m_local_image_id, "", m_local_io_ctx, - false); - EXPECT_EQ(0, ictx->state->open(false)); - - C_SaferCond ctx; - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, false, &ctx); - EXPECT_EQ(-EBUSY, ctx.wait()); - - EXPECT_EQ(0, ictx->state->close()); -} - -TEST_F(TestImageDeleter, Retry_Failed_Deletes) { - init_image_deleter(); - EXPECT_EQ(0, g_ceph_context->_conf->set_val("rbd_mirror_delete_retry_interval", "0.1")); - ImageCtx *ictx = new ImageCtx("", m_local_image_id, "", m_local_io_ctx, - false); - EXPECT_EQ(0, ictx->state->open(false)); - - C_SaferCond ctx; - m_deleter->schedule_image_delete(GLOBAL_IMAGE_ID, false, &ctx); - EXPECT_EQ(-EBUSY, ctx.wait()); - - EXPECT_EQ(0, ictx->state->close()); - - C_SaferCond ctx2; - m_deleter->wait_for_scheduled_deletion(GLOBAL_IMAGE_ID, &ctx2); - EXPECT_EQ(0, ctx2.wait()); - - ASSERT_EQ(0u, m_deleter->get_delete_queue_items().size()); - ASSERT_EQ(0u, m_deleter->get_failed_queue_items().size()); - - check_image_deleted(); -} diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc index 46a4bdc78c301..0e799b704f161 100644 --- a/src/tools/rbd_mirror/ImageDeleter.cc +++ b/src/tools/rbd_mirror/ImageDeleter.cc @@ -211,7 +211,7 @@ void ImageDeleter::cancel_all_deletions(Context* on_finish) { assert(m_in_flight_delete_queue.empty()); for (auto& queue : {&m_delete_queue, &m_retry_delete_queue}) { for (auto& info : *queue) { - notify_on_delete(info->global_image_id, -ECANCELED); + notify_on_delete(info->image_id, -ECANCELED); } queue->clear(); } @@ -220,68 +220,25 @@ void ImageDeleter::cancel_all_deletions(Context* on_finish) { } template -void ImageDeleter::schedule_image_delete(const std::string& global_image_id, - bool ignore_orphaned, - Context *on_delete) { - dout(5) << "global_image_id=" << global_image_id << dendl; - - if (on_delete != nullptr) { - on_delete = new FunctionContext([this, on_delete](int r) { - m_threads->work_queue->queue(on_delete, r); - }); - } - - { - Mutex::Locker locker(m_lock); - notify_on_delete(global_image_id, -ESTALE); - if (on_delete != nullptr) { - m_on_delete_contexts[global_image_id] = on_delete; - } - - auto del_info = find_delete_info(global_image_id); - if (del_info != nullptr) { - dout(20) << "image " << global_image_id << " " - << "was already scheduled for deletion" << dendl; - if (ignore_orphaned) { - del_info->ignore_orphaned = true; - } - return; - } - - m_delete_queue.emplace_back(new DeleteInfo(global_image_id, - ignore_orphaned)); - } - remove_images(); -} - -template -void ImageDeleter::wait_for_deletion(const std::string& global_image_id, +void ImageDeleter::wait_for_deletion(const std::string& image_id, bool scheduled_only, Context* on_finish) { - dout(5) << "global_image_id=" << global_image_id << dendl; + dout(5) << "image_id=" << image_id << dendl; on_finish = new FunctionContext([this, on_finish](int r) { m_threads->work_queue->queue(on_finish, r); }); Mutex::Locker locker(m_lock); - auto del_info = find_delete_info(global_image_id); + auto del_info = find_delete_info(image_id); if (!del_info && scheduled_only) { // image not scheduled for deletion on_finish->complete(0); return; } - notify_on_delete(global_image_id, -ESTALE); - m_on_delete_contexts[global_image_id] = on_finish; -} - -template -void ImageDeleter::cancel_waiter(const std::string &global_image_id) { - dout(5) << "global_image_id=" << global_image_id << dendl; - - Mutex::Locker locker(m_lock); - notify_on_delete(global_image_id, -ECANCELED); + notify_on_delete(image_id, -ESTALE); + m_on_delete_contexts[image_id] = on_finish; } template @@ -289,7 +246,7 @@ void ImageDeleter::complete_active_delete(DeleteInfoRef* delete_info, int r) { dout(20) << "info=" << *delete_info << ", r=" << r << dendl; Mutex::Locker locker(m_lock); - notify_on_delete((*delete_info)->global_image_id, r); + notify_on_delete((*delete_info)->image_id, r); delete_info->reset(); } @@ -308,7 +265,7 @@ void ImageDeleter::enqueue_failed_delete(DeleteInfoRef* delete_info, Mutex::Locker timer_locker(m_threads->timer_lock); Mutex::Locker locker(m_lock); auto& delete_info_ref = *delete_info; - notify_on_delete(delete_info_ref->global_image_id, error_code); + notify_on_delete(delete_info_ref->image_id, error_code); delete_info_ref->error_code = error_code; ++delete_info_ref->retries; delete_info_ref->retry_time = ceph_clock_now(); @@ -320,13 +277,13 @@ void ImageDeleter::enqueue_failed_delete(DeleteInfoRef* delete_info, template typename ImageDeleter::DeleteInfoRef -ImageDeleter::find_delete_info(const std::string &global_image_id) { +ImageDeleter::find_delete_info(const std::string &image_id) { assert(m_lock.is_locked()); DeleteQueue delete_queues[] = {m_in_flight_delete_queue, m_retry_delete_queue, m_delete_queue}; - DeleteInfo delete_info{global_image_id}; + DeleteInfo delete_info{image_id}; for (auto& queue : delete_queues) { auto it = std::find_if(queue.begin(), queue.end(), [&delete_info](const DeleteInfoRef& ref) { @@ -375,7 +332,7 @@ vector ImageDeleter::get_delete_queue_items() { Mutex::Locker l(m_lock); for (const auto& del_info : m_delete_queue) { - items.push_back(del_info->global_image_id); + items.push_back(del_info->image_id); } return items; @@ -387,7 +344,7 @@ vector > ImageDeleter::get_failed_queue_items() { Mutex::Locker l(m_lock); for (const auto& del_info : m_retry_delete_queue) { - items.push_back(make_pair(del_info->global_image_id, + items.push_back(make_pair(del_info->image_id, del_info->error_code)); } @@ -430,9 +387,8 @@ void ImageDeleter::remove_image(DeleteInfoRef delete_info) { }); auto req = image_deleter::RemoveRequest::create( - m_local_io_ctx, delete_info->global_image_id, - delete_info->ignore_orphaned, &delete_info->error_result, m_threads->work_queue, - ctx); + m_local_io_ctx, delete_info->image_id, &delete_info->error_result, + m_threads->work_queue, ctx); req->send(); } @@ -538,7 +494,6 @@ void ImageDeleter::handle_trash_image(const std::string& image_id, Mutex::Locker timer_locker(m_threads->timer_lock); Mutex::Locker locker(m_lock); - // TODO auto del_info = find_delete_info(image_id); if (del_info != nullptr) { dout(20) << "image " << image_id << " " @@ -549,7 +504,7 @@ void ImageDeleter::handle_trash_image(const std::string& image_id, dout(10) << "image_id=" << image_id << ", " << "deferment_end_time=" << deferment_end_time << dendl; - del_info.reset(new DeleteInfo(image_id, false)); + del_info.reset(new DeleteInfo(image_id)); del_info->retry_time = deferment_end_time; m_retry_delete_queue.push_back(del_info); @@ -557,10 +512,10 @@ void ImageDeleter::handle_trash_image(const std::string& image_id, } template -void ImageDeleter::notify_on_delete(const std::string& global_image_id, +void ImageDeleter::notify_on_delete(const std::string& image_id, int r) { - dout(10) << "global_image_id=" << global_image_id << ", r=" << r << dendl; - auto it = m_on_delete_contexts.find(global_image_id); + dout(10) << "image_id=" << image_id << ", r=" << r << dendl; + auto it = m_on_delete_contexts.find(image_id); if (it == m_on_delete_contexts.end()) { return; } @@ -574,7 +529,7 @@ void ImageDeleter::DeleteInfo::print_status(Formatter *f, stringstream *ss, bool print_failure_info) { if (f) { f->open_object_section("delete_info"); - f->dump_string("global_image_id", global_image_id); + f->dump_string("image_id", image_id); if (print_failure_info) { f->dump_string("error_code", cpp_strerror(error_code)); f->dump_int("retries", retries); diff --git a/src/tools/rbd_mirror/ImageDeleter.h b/src/tools/rbd_mirror/ImageDeleter.h index f3fc953d35b6e..71967c153c6cd 100644 --- a/src/tools/rbd_mirror/ImageDeleter.h +++ b/src/tools/rbd_mirror/ImageDeleter.h @@ -61,21 +61,11 @@ public: void init(Context* on_finish); void shut_down(Context* on_finish); - void schedule_image_delete(const std::string& global_image_id, - bool ignore_orphaned, - Context *on_finish); - void wait_for_deletion(const std::string &global_image_id, - bool scheduled_only, Context* on_finish); - void wait_for_scheduled_deletion(const std::string &global_image_id, - Context* on_finish) { - wait_for_deletion(global_image_id, true, on_finish); - } - - void cancel_waiter(const std::string &global_image_id); - void print_status(Formatter *f, std::stringstream *ss); // for testing purposes + void wait_for_deletion(const std::string &image_id, + bool scheduled_only, Context* on_finish); std::vector get_delete_queue_items(); std::vector > get_failed_queue_items(); @@ -98,28 +88,23 @@ private: }; struct DeleteInfo { - std::string global_image_id; - bool ignore_orphaned = false; + std::string image_id; image_deleter::ErrorResult error_result = {}; int error_code = 0; utime_t retry_time = {}; int retries = 0; - DeleteInfo(const std::string& global_image_id) - : global_image_id(global_image_id) { - } - - DeleteInfo(const std::string& global_image_id, bool ignore_orphaned) - : global_image_id(global_image_id), ignore_orphaned(ignore_orphaned) { + DeleteInfo(const std::string& image_id) + : image_id(image_id) { } inline bool operator==(const DeleteInfo& delete_info) const { - return (global_image_id == delete_info.global_image_id); + return (image_id == delete_info.image_id); } friend std::ostream& operator<<(std::ostream& os, DeleteInfo& delete_info) { - os << "[global_image_id=" << delete_info.global_image_id << "]"; + os << "[image_id=" << delete_info.image_id << "]"; return os; } @@ -160,7 +145,7 @@ private: void enqueue_failed_delete(DeleteInfoRef* delete_info, int error_code, double retry_delay); - DeleteInfoRef find_delete_info(const std::string &global_image_id); + DeleteInfoRef find_delete_info(const std::string &image_id); void remove_images(); void remove_image(DeleteInfoRef delete_info); @@ -177,7 +162,7 @@ private: void wait_for_ops(Context* on_finish); void cancel_all_deletions(Context* on_finish); - void notify_on_delete(const std::string& global_image_id, int r); + void notify_on_delete(const std::string& image_id, int r); }; diff --git a/src/tools/rbd_mirror/image_deleter/RemoveRequest.cc b/src/tools/rbd_mirror/image_deleter/RemoveRequest.cc index fde01c62336cd..e411fa33ca9ca 100644 --- a/src/tools/rbd_mirror/image_deleter/RemoveRequest.cc +++ b/src/tools/rbd_mirror/image_deleter/RemoveRequest.cc @@ -30,84 +30,6 @@ template void RemoveRequest::send() { *m_error_result = ERROR_RESULT_RETRY; - get_mirror_image_id(); -} - -template -void RemoveRequest::get_mirror_image_id() { - dout(10) << dendl; - - librados::ObjectReadOperation op; - librbd::cls_client::mirror_image_get_image_id_start(&op, m_global_image_id); - - auto aio_comp = create_rados_callback< - RemoveRequest, &RemoveRequest::handle_get_mirror_image_id>(this); - m_out_bl.clear(); - int r = m_io_ctx.aio_operate(RBD_MIRRORING, aio_comp, &op, &m_out_bl); - assert(r == 0); - aio_comp->release(); -} - -template -void RemoveRequest::handle_get_mirror_image_id(int r) { - dout(10) << "r=" << r << dendl; - - if (r == 0) { - auto bl_it = m_out_bl.begin(); - r = librbd::cls_client::mirror_image_get_image_id_finish(&bl_it, - &m_image_id); - } - if (r == -ENOENT) { - dout(10) << "image " << m_global_image_id << " is not mirrored" << dendl; - *m_error_result = ERROR_RESULT_COMPLETE; - finish(r); - return; - } else if (r < 0) { - derr << "error retrieving local id for image " << m_global_image_id - << ": " << cpp_strerror(r) << dendl; - finish(r); - return; - } - - get_tag_owner(); -} - -template -void RemoveRequest::get_tag_owner() { - dout(10) << dendl; - - auto ctx = create_context_callback< - RemoveRequest, &RemoveRequest::handle_get_tag_owner>(this); - librbd::Journal::get_tag_owner(m_io_ctx, m_image_id, &m_mirror_uuid, - m_op_work_queue, ctx); -} - -template -void RemoveRequest::handle_get_tag_owner(int r) { - dout(10) << "r=" << r << dendl; - - if (r < 0 && r != -ENOENT) { - 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; - - *m_error_result = ERROR_RESULT_COMPLETE; - finish(-EPERM); - return; - } else if (m_mirror_uuid == librbd::Journal<>::ORPHAN_MIRROR_UUID && - !m_ignore_orphaned) { - dout(10) << "image " << m_global_image_id << " is orphaned" << dendl; - - *m_error_result = ERROR_RESULT_COMPLETE; - finish(-EPERM); - return; - } - } - get_snap_context(); } @@ -139,57 +61,12 @@ void RemoveRequest::handle_get_snap_context(int r) { } if (r < 0 && r != -ENOENT) { derr << "error retrieving snapshot context for image " - << m_global_image_id << ": " << cpp_strerror(r) << dendl; + << m_image_id << ": " << cpp_strerror(r) << dendl; finish(r); return; } m_has_snapshots = (!snapc.empty()); - set_mirror_image_disabling(); -} - -template -void RemoveRequest::set_mirror_image_disabling() { - 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; - - librados::ObjectWriteOperation op; - librbd::cls_client::mirror_image_set(&op, m_image_id, mirror_image); - - auto aio_comp = create_rados_callback< - RemoveRequest, - &RemoveRequest::handle_set_mirror_image_disabling>(this); - int r = m_io_ctx.aio_operate(RBD_MIRRORING, aio_comp, &op); - assert(r == 0); - aio_comp->release(); -} - -template -void RemoveRequest::handle_set_mirror_image_disabling(int r) { - dout(10) << "r=" << r << dendl; - - if (r == -ENOENT) { - dout(10) << "local image is not mirrored, aborting deletion." << dendl; - *m_error_result = ERROR_RESULT_COMPLETE; - finish(r); - return; - } else if (r == -EEXIST || r == -EINVAL) { - derr << "cannot disable mirroring for image " << m_global_image_id - << ": global_image_id has changed/reused: " - << cpp_strerror(r) << dendl; - *m_error_result = ERROR_RESULT_COMPLETE; - finish(r); - return; - } else if (r < 0) { - derr << "cannot disable mirroring for image " << m_global_image_id - << ": " << cpp_strerror(r) << dendl; - finish(r); - return; - } - purge_snapshots(); } @@ -232,7 +109,7 @@ void RemoveRequest::remove_image() { auto ctx = create_context_callback< RemoveRequest, &RemoveRequest::handle_remove_image>(this); auto req = librbd::image::RemoveRequest::create( - m_io_ctx, "", m_image_id, true, false, m_progress_ctx, m_op_work_queue, + m_io_ctx, "", m_image_id, true, true, m_progress_ctx, m_op_work_queue, ctx); req->send(); } @@ -241,40 +118,13 @@ template void RemoveRequest::handle_remove_image(int r) { dout(10) << "r=" << r << dendl; if (r < 0 && r != -ENOENT) { - derr << "error removing image " << m_global_image_id << " " + derr << "error removing image " << m_image_id << " " << "(" << m_image_id << ") from local pool: " << cpp_strerror(r) << dendl; finish(r); return; } - remove_mirror_image(); -} - -template -void RemoveRequest::remove_mirror_image() { - dout(10) << dendl; - - librados::ObjectWriteOperation op; - librbd::cls_client::mirror_image_remove(&op, m_image_id); - - auto aio_comp = create_rados_callback< - RemoveRequest, &RemoveRequest::handle_remove_mirror_image>(this); - int r = m_io_ctx.aio_operate(RBD_MIRRORING, aio_comp, &op); - assert(r == 0); - aio_comp->release(); -} - -template -void RemoveRequest::handle_remove_mirror_image(int r) { - dout(10) << "r=" << r << dendl; - if (r < 0 && r != -ENOENT) { - derr << "error removing image " << m_global_image_id << " from mirroring " - << "directory: " << cpp_strerror(r) << dendl; - finish(r); - return; - } - finish(0); } diff --git a/src/tools/rbd_mirror/image_deleter/RemoveRequest.h b/src/tools/rbd_mirror/image_deleter/RemoveRequest.h index da9107afab965..75353aa43d127 100644 --- a/src/tools/rbd_mirror/image_deleter/RemoveRequest.h +++ b/src/tools/rbd_mirror/image_deleter/RemoveRequest.h @@ -23,18 +23,17 @@ template class RemoveRequest { public: static RemoveRequest* create(librados::IoCtx &io_ctx, - const std::string &global_image_id, - bool ignore_orphaned, ErrorResult *error_result, + const std::string &image_id, + ErrorResult *error_result, ContextWQ *op_work_queue, Context *on_finish) { - return new RemoveRequest(io_ctx, global_image_id, ignore_orphaned, - error_result, op_work_queue, on_finish); + return new RemoveRequest(io_ctx, image_id, error_result, op_work_queue, + on_finish); } - RemoveRequest(librados::IoCtx &io_ctx, const std::string &global_image_id, - bool ignore_orphaned, ErrorResult *error_result, - ContextWQ *op_work_queue, Context *on_finish) - : m_io_ctx(io_ctx), m_global_image_id(global_image_id), - m_ignore_orphaned(ignore_orphaned), m_error_result(error_result), + RemoveRequest(librados::IoCtx &io_ctx, const std::string &image_id, + ErrorResult *error_result, ContextWQ *op_work_queue, + Context *on_finish) + : m_io_ctx(io_ctx), m_image_id(image_id), m_error_result(error_result), m_op_work_queue(op_work_queue), m_on_finish(on_finish) { } @@ -47,66 +46,39 @@ private: * * | * v - * GET_MIRROR_IMAGE_ID - * | - * v - * GET_TAG_OWNER - * | - * v * GET_SNAP_CONTEXT * | * v - * SET_MIRROR_IMAGE_DISABLING - * | - * v * PURGE_SNAPSHOTS * | * v * REMOVE_IMAGE * | * v - * REMOVE_MIRROR_IMAGE - * | - * v * * * @endverbatim */ librados::IoCtx &m_io_ctx; - std::string m_global_image_id; - bool m_ignore_orphaned; + std::string m_image_id; ErrorResult *m_error_result; ContextWQ *m_op_work_queue; Context *m_on_finish; ceph::bufferlist m_out_bl; - std::string m_image_id; - std::string m_mirror_uuid; bool m_has_snapshots = false; librbd::NoOpProgressContext m_progress_ctx; - 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_snap_context(); void handle_get_snap_context(int r); - void set_mirror_image_disabling(); - void handle_set_mirror_image_disabling(int r); - void purge_snapshots(); void handle_purge_snapshots(int r); void remove_image(); void handle_remove_image(int r); - void remove_mirror_image(); - void handle_remove_mirror_image(int r); - void finish(int r); }; -- 2.39.5