From 89df10a9c5c7f082b0e8894560c13d3c9c068319 Mon Sep 17 00:00:00 2001 From: Arthur Outhenin-Chalandre Date: Fri, 4 Jun 2021 18:29:37 +0200 Subject: [PATCH] rbd-mirror: fix mirror image removal Invoke ImageRemoveRequest instead of calling directly mirror_image_remove so that the MirrroringWatcher can pick up local image deletion. Fixes: https://tracker.ceph.com/issues/51031 Signed-off-by: Arthur Outhenin-Chalandre (cherry picked from commit 34082b7ee48a33e566348395395858e1e0db3013) Conflicts: src/test/rbd_mirror/image_deleter/test_mock_TrashMoveRequest.cc - Trivial conflict resolution --- .../test_mock_TrashMoveRequest.cc | 59 +++++++++++++++---- .../image_deleter/TrashMoveRequest.cc | 12 ++-- 2 files changed, 54 insertions(+), 17 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 2630915115d03..49bcbc844ba92 100644 --- a/src/test/rbd_mirror/image_deleter/test_mock_TrashMoveRequest.cc +++ b/src/test/rbd_mirror/image_deleter/test_mock_TrashMoveRequest.cc @@ -9,6 +9,7 @@ #include "librbd/TrashWatcher.h" #include "librbd/journal/ResetRequest.h" #include "librbd/mirror/GetInfoRequest.h" +#include "librbd/mirror/ImageRemoveRequest.h" #include "librbd/trash/MoveRequest.h" #include "tools/rbd_mirror/Threads.h" #include "tools/rbd_mirror/image_deleter/TrashMoveRequest.h" @@ -130,6 +131,37 @@ struct GetInfoRequest { GetInfoRequest* GetInfoRequest::s_instance = nullptr; +template<> +struct ImageRemoveRequest { + static ImageRemoveRequest* s_instance; + std::string global_image_id; + std::string image_id; + Context* on_finish; + + static ImageRemoveRequest *create(librados::IoCtx& io_ctx, + const std::string& global_image_id, + const std::string& image_id, + Context* on_finish) { + ceph_assert(s_instance != nullptr); + s_instance->global_image_id = global_image_id; + s_instance->image_id = image_id; + s_instance->on_finish = on_finish; + return s_instance; + } + + ImageRemoveRequest() { + ceph_assert(s_instance == nullptr); + s_instance = this; + } + ~ImageRemoveRequest() { + s_instance = nullptr; + } + + MOCK_METHOD0(send, void()); +}; + +ImageRemoveRequest* ImageRemoveRequest::s_instance = nullptr; + } // namespace mirror namespace trash { @@ -184,6 +216,7 @@ public: typedef TrashMoveRequest MockTrashMoveRequest; typedef librbd::journal::ResetRequest MockJournalResetRequest; typedef librbd::mirror::GetInfoRequest MockGetMirrorInfoRequest; + typedef librbd::mirror::ImageRemoveRequest MockImageRemoveRequest; typedef librbd::trash::MoveRequest MockLibrbdTrashMoveRequest; typedef librbd::TrashWatcher MockTrashWatcher; @@ -275,11 +308,12 @@ public: .WillOnce(Return(r)); } - void expect_mirror_image_remove(librados::IoCtx &ioctx, int r) { - EXPECT_CALL(get_mock_io_ctx(ioctx), - exec(StrEq("rbd_mirroring"), _, StrEq("rbd"), StrEq("mirror_image_remove"), - _, _, _)) - .WillOnce(Return(r)); + void expect_mirror_image_remove_request( + MockImageRemoveRequest& mock_image_remove_request, int r) { + EXPECT_CALL(mock_image_remove_request, send()) + .WillOnce(Invoke([this, &mock_image_remove_request, r]() { + m_threads->work_queue->queue(mock_image_remove_request.on_finish, r); + })); } void expect_journal_reset(MockJournalResetRequest& mock_journal_reset_request, @@ -357,7 +391,8 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, SuccessJournal) { 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); + MockImageRemoveRequest mock_image_remove_request; + expect_mirror_image_remove_request(mock_image_remove_request, 0); expect_close(mock_image_ctx, 0); expect_destroy(mock_image_ctx); @@ -399,7 +434,8 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, SuccessSnapshot) { 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); + MockImageRemoveRequest mock_image_remove_request; + expect_mirror_image_remove_request(mock_image_remove_request, 0); expect_close(mock_image_ctx, 0); expect_destroy(mock_image_ctx); @@ -750,7 +786,8 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, RemoveMirrorImageError) { 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, -EINVAL); + MockImageRemoveRequest mock_image_remove_request; + expect_mirror_image_remove_request(mock_image_remove_request, -EINVAL); expect_close(mock_image_ctx, 0); expect_destroy(mock_image_ctx); @@ -800,7 +837,8 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, CloseImageError) { 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); + MockImageRemoveRequest mock_image_remove_request; + expect_mirror_image_remove_request(mock_image_remove_request, 0); expect_close(mock_image_ctx, -EINVAL); expect_destroy(mock_image_ctx); @@ -852,7 +890,8 @@ TEST_F(TestMockImageDeleterTrashMoveRequest, DelayedDelation) { expect_trash_move(mock_librbd_trash_move_request, m_image_name, "image id", 600, 0); - expect_mirror_image_remove(m_local_io_ctx, 0); + MockImageRemoveRequest mock_image_remove_request; + expect_mirror_image_remove_request(mock_image_remove_request, 0); expect_close(mock_image_ctx, 0); expect_destroy(mock_image_ctx); diff --git a/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc b/src/tools/rbd_mirror/image_deleter/TrashMoveRequest.cc index 0eadca7bf2a86..5a2dc9c7243a9 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/ImageRemoveRequest.h" #include "librbd/mirror/GetInfoRequest.h" #include "librbd/trash/MoveRequest.h" #include "tools/rbd_mirror/image_deleter/Types.h" @@ -309,15 +310,12 @@ template void TrashMoveRequest::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< + auto ctx = create_context_callback< TrashMoveRequest, &TrashMoveRequest::handle_remove_mirror_image>(this); - int r = m_io_ctx.aio_operate(RBD_MIRRORING, aio_comp, &op); - ceph_assert(r == 0); - aio_comp->release(); + auto req = librbd::mirror::ImageRemoveRequest::create( + m_io_ctx, m_global_image_id, m_image_id, ctx); + req->send(); } template -- 2.39.5