From 04c33f6bd61899ed6573fa525da26fa356a1977d Mon Sep 17 00:00:00 2001 From: Mykola Golub Date: Sat, 9 Jun 2018 18:07:42 +0300 Subject: [PATCH] librbd: use newly added method to list watchers in remove request Signed-off-by: Mykola Golub --- src/librbd/image/RemoveRequest.cc | 96 ++----------------- src/librbd/image/RemoveRequest.h | 9 +- .../librbd/image/test_mock_RemoveRequest.cc | 64 ++++++++++--- 3 files changed, 62 insertions(+), 107 deletions(-) diff --git a/src/librbd/image/RemoveRequest.cc b/src/librbd/image/RemoveRequest.cc index 09564cca81a..76fbf8d0df5 100644 --- a/src/librbd/image/RemoveRequest.cc +++ b/src/librbd/image/RemoveRequest.cc @@ -11,6 +11,7 @@ #include "librbd/ExclusiveLock.h" #include "librbd/MirroringWatcher.h" #include "librbd/image/DetachChildRequest.h" +#include "librbd/image/ListWatchersRequest.h" #include "librbd/journal/DisabledPolicy.h" #include "librbd/journal/RemoveRequest.h" #include "librbd/mirror/DisableRequest.h" @@ -229,26 +230,21 @@ template void RemoveRequest::list_image_watchers() { ldout(m_cct, 20) << dendl; - librados::ObjectReadOperation op; - op.list_watchers(&m_watchers, &m_ret_val); - + int flags = LIST_WATCHERS_FILTER_OUT_MY_INSTANCE | + LIST_WATCHERS_FILTER_OUT_MIRROR_INSTANCES; using klass = RemoveRequest; - librados::AioCompletion *rados_completion = - create_rados_callback(this); + Context *ctx = create_context_callback< + klass, &klass::handle_list_image_watchers>(this); - int r = m_image_ctx->md_ctx.aio_operate(m_header_oid, rados_completion, - &op, &m_out_bl); - assert(r == 0); - rados_completion->release(); + auto req = ListWatchersRequest::create(*m_image_ctx, flags, &m_watchers, + ctx); + req->send(); } template void RemoveRequest::handle_list_image_watchers(int r) { ldout(m_cct, 20) << "r=" << r << dendl; - if (r == 0 && m_ret_val < 0) { - r = m_ret_val; - } if (r < 0) { lderr(m_cct) << "error listing image watchers: " << cpp_strerror(r) << dendl; @@ -256,86 +252,12 @@ void RemoveRequest::handle_list_image_watchers(int r) { return; } - get_mirror_image(); -} - -template -void RemoveRequest::get_mirror_image() { - ldout(m_cct, 20) << dendl; - if ((m_watchers.empty()) || - ((m_image_ctx->features & RBD_FEATURE_JOURNALING) == 0)) { - check_image_watchers(); - return; - } - - librados::ObjectReadOperation op; - cls_client::mirror_image_get_start(&op, m_image_id); - - using klass = RemoveRequest; - librados::AioCompletion *comp = - create_rados_callback(this); - m_out_bl.clear(); - int r = m_image_ctx->md_ctx.aio_operate(RBD_MIRRORING, comp, &op, &m_out_bl); - assert(r == 0); - comp->release(); -} - -template -void RemoveRequest::handle_get_mirror_image(int r) { - ldout(m_cct, 20) << "r=" << r << dendl; - - if (r == -ENOENT || r == -EOPNOTSUPP) { - check_image_watchers(); - return; - } else if (r < 0) { - ldout(m_cct, 5) << "error retrieving mirror image: " << cpp_strerror(r) - << dendl; - } - - list_mirror_watchers(); -} - -template -void RemoveRequest::list_mirror_watchers() { - ldout(m_cct, 20) << dendl; - - librados::ObjectReadOperation op; - op.list_watchers(&m_mirror_watchers, &m_ret_val); - - using klass = RemoveRequest; - librados::AioCompletion *rados_completion = - create_rados_callback(this); - m_out_bl.clear(); - int r = m_image_ctx->md_ctx.aio_operate(RBD_MIRRORING, rados_completion, - &op, &m_out_bl); - assert(r == 0); - rados_completion->release(); -} - -template -void RemoveRequest::handle_list_mirror_watchers(int r) { - ldout(m_cct, 20) << "r=" << r << dendl; - - if (r == 0 && m_ret_val < 0) { - r = m_ret_val; - } - if (r < 0 && r != -ENOENT) { - ldout(m_cct, 5) << "error listing mirror watchers: " << cpp_strerror(r) - << dendl; - } - - for (auto &watcher : m_mirror_watchers) { - m_watchers.remove_if([watcher] (obj_watch_t &w) { - return (strncmp(w.addr, watcher.addr, sizeof(w.addr)) == 0); - }); - } - check_image_watchers(); } template void RemoveRequest::check_image_watchers() { - if (m_watchers.size() > 1) { + if (!m_watchers.empty()) { lderr(m_cct) << "image has watchers - not removing" << dendl; send_close_image(-EBUSY); return; diff --git a/src/librbd/image/RemoveRequest.h b/src/librbd/image/RemoveRequest.h index 803bd440c2d..0bb95aa0999 100644 --- a/src/librbd/image/RemoveRequest.h +++ b/src/librbd/image/RemoveRequest.h @@ -8,6 +8,8 @@ #include "librbd/ImageCtx.h" #include "librbd/image/TypeTraits.h" +#include + class Context; class ContextWQ; class SafeTimer; @@ -142,7 +144,6 @@ private: int m_ret_val = 0; bufferlist m_out_bl; std::list m_watchers; - std::list m_mirror_watchers; std::map m_snap_infos; @@ -170,12 +171,6 @@ private: void list_image_watchers(); void handle_list_image_watchers(int r); - void get_mirror_image(); - void handle_get_mirror_image(int r); - - void list_mirror_watchers(); - void handle_list_mirror_watchers(int r); - void check_image_watchers(); void check_group(); diff --git a/src/test/librbd/image/test_mock_RemoveRequest.cc b/src/test/librbd/image/test_mock_RemoveRequest.cc index 41695a80197..47a63774033 100644 --- a/src/test/librbd/image/test_mock_RemoveRequest.cc +++ b/src/test/librbd/image/test_mock_RemoveRequest.cc @@ -12,6 +12,7 @@ #include "librbd/Operations.h" #include "librbd/image/TypeTraits.h" #include "librbd/image/DetachChildRequest.h" +#include "librbd/image/ListWatchersRequest.h" #include "librbd/image/RemoveRequest.h" #include "librbd/image/RefreshParentRequest.h" #include "librbd/journal/RemoveRequest.h" @@ -182,6 +183,33 @@ public: DisableRequest *DisableRequest::s_instance; } // namespace mirror + +namespace image { + +template<> +class ListWatchersRequest { +public: + static ListWatchersRequest *s_instance; + Context *on_finish = nullptr; + + static ListWatchersRequest *create(MockTestImageCtx &image_ctx, int flags, + std::list *watchers, + Context *on_finish) { + assert(s_instance != nullptr); + s_instance->on_finish = on_finish; + return s_instance; + } + + ListWatchersRequest() { + s_instance = this; + } + + MOCK_METHOD0(send, void()); +}; + +ListWatchersRequest *ListWatchersRequest::s_instance; + +} // namespace image } // namespace librbd // template definitions @@ -217,6 +245,7 @@ public: typedef typename TypeTraits::ContextWQ ContextWQ; typedef RemoveRequest MockRemoveRequest; typedef DetachChildRequest MockDetachChildRequest; + typedef ListWatchersRequest MockListWatchersRequest; typedef librbd::operation::SnapshotRemoveRequest MockSnapshotRemoveRequest; typedef librbd::operation::TrimRequest MockTrimRequest; typedef librbd::journal::RemoveRequest MockJournalRemoveRequest; @@ -264,6 +293,13 @@ public: })); } + void expect_list_image_watchers( + MockTestImageCtx &mock_image_ctx, + MockListWatchersRequest &mock_list_watchers_request, int r) { + EXPECT_CALL(mock_list_watchers_request, send()) + .WillOnce(FinishRequest(&mock_list_watchers_request, r, &mock_image_ctx)); + } + void expect_get_group(MockTestImageCtx &mock_image_ctx, int r) { auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(mock_image_ctx.header_oid, _, StrEq("rbd"), @@ -307,15 +343,6 @@ public: .WillOnce(Return(r)); } - void expect_mirror_image_get(MockTestImageCtx &mock_image_ctx, int r) { - if ((mock_image_ctx.features & RBD_FEATURE_JOURNALING) != 0ULL) { - EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), - exec(RBD_MIRRORING, _, StrEq("rbd"), - StrEq("mirror_image_get"), _, _, _)) - .WillOnce(Return(r)); - } - } - void expect_dir_remove_image(librados::IoCtx &ioctx, int r) { EXPECT_CALL(get_mock_io_ctx(ioctx), exec(RBD_DIRECTORY, _, StrEq("rbd"), StrEq("dir_remove_image"), @@ -365,6 +392,9 @@ TEST_F(TestMockImageRemoveRequest, SuccessV1) { InSequence seq; expect_state_open(*m_mock_imctx, 0); + MockListWatchersRequest mock_list_watchers_request; + expect_list_image_watchers(*m_mock_imctx, mock_list_watchers_request, 0); + MockTrimRequest mock_trim_request; expect_trim(*m_mock_imctx, mock_trim_request, 0); @@ -421,7 +451,9 @@ TEST_F(TestMockImageRemoveRequest, SuccessV2CloneV1) { expect_set_journal_policy(*m_mock_imctx); expect_shut_down_exclusive_lock(*m_mock_imctx, *mock_exclusive_lock, 0); - expect_mirror_image_get(*m_mock_imctx, 0); + MockListWatchersRequest mock_list_watchers_request; + expect_list_image_watchers(*m_mock_imctx, mock_list_watchers_request, 0); + expect_get_group(*m_mock_imctx, 0); MockTrimRequest mock_trim_request; @@ -472,7 +504,9 @@ TEST_F(TestMockImageRemoveRequest, SuccessV2CloneV2) { expect_set_journal_policy(*m_mock_imctx); expect_shut_down_exclusive_lock(*m_mock_imctx, *mock_exclusive_lock, 0); - expect_mirror_image_get(*m_mock_imctx, 0); + MockListWatchersRequest mock_list_watchers_request; + expect_list_image_watchers(*m_mock_imctx, mock_list_watchers_request, 0); + expect_get_group(*m_mock_imctx, 0); MockTrimRequest mock_trim_request; @@ -523,7 +557,9 @@ TEST_F(TestMockImageRemoveRequest, NotExistsV2) { expect_set_journal_policy(*m_mock_imctx); expect_shut_down_exclusive_lock(*m_mock_imctx, *mock_exclusive_lock, 0); - expect_mirror_image_get(*m_mock_imctx, 0); + MockListWatchersRequest mock_list_watchers_request; + expect_list_image_watchers(*m_mock_imctx, mock_list_watchers_request, 0); + expect_get_group(*m_mock_imctx, 0); MockTrimRequest mock_trim_request; @@ -612,7 +648,9 @@ TEST_F(TestMockImageRemoveRequest, AutoDeleteSnapshots) { expect_set_journal_policy(*m_mock_imctx); expect_shut_down_exclusive_lock(*m_mock_imctx, *mock_exclusive_lock, 0); - expect_mirror_image_get(*m_mock_imctx, 0); + MockListWatchersRequest mock_list_watchers_request; + expect_list_image_watchers(*m_mock_imctx, mock_list_watchers_request, 0); + expect_get_group(*m_mock_imctx, 0); MockSnapshotRemoveRequest mock_snap_remove_request;