From: Jason Dillaman Date: Wed, 24 Jan 2018 17:31:46 +0000 (-0500) Subject: librbd: remove request now handles clone v2 X-Git-Tag: v13.0.2~327^2~10 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=380d7bce50700b2e6d6540c95397e48d0fe1d791;p=ceph.git librbd: remove request now handles clone v2 Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/image/RemoveRequest.cc b/src/librbd/image/RemoveRequest.cc index 65cc9b72cca..0013bee5c0f 100644 --- a/src/librbd/image/RemoveRequest.cc +++ b/src/librbd/image/RemoveRequest.cc @@ -1,6 +1,7 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab +#include "librbd/image/RemoveRequest.h" #include "common/dout.h" #include "common/errno.h" #include "librbd/internal.h" @@ -9,10 +10,10 @@ #include "librbd/ObjectMap.h" #include "librbd/ExclusiveLock.h" #include "librbd/MirroringWatcher.h" +#include "librbd/image/DetachChildRequest.h" #include "librbd/journal/RemoveRequest.h" -#include "librbd/image/RemoveRequest.h" -#include "librbd/operation/TrimRequest.h" #include "librbd/mirror/DisableRequest.h" +#include "librbd/operation/TrimRequest.h" #define dout_subsys ceph_subsys_rbd #undef dout_prefix @@ -364,42 +365,30 @@ void RemoveRequest::handle_trim_image(int r) { return; } - remove_child(); + detach_child(); } template -void RemoveRequest::remove_child() { +void RemoveRequest::detach_child() { ldout(m_cct, 20) << dendl; - m_image_ctx->parent_lock.get_read(); - ParentInfo parent_info = m_image_ctx->parent_md; - m_image_ctx->parent_lock.put_read(); - - librados::ObjectWriteOperation op; - librbd::cls_client::remove_child(&op, parent_info.spec, m_image_id); - - using klass = RemoveRequest; - librados::AioCompletion *rados_completion = - create_rados_callback(this); - int r = m_image_ctx->md_ctx.aio_operate(RBD_CHILDREN, rados_completion, &op); - assert(r == 0); - rados_completion->release(); + auto ctx = create_context_callback< + RemoveRequest, &RemoveRequest::handle_detach_child>(this); + auto req = DetachChildRequest::create(*m_image_ctx, ctx); + req->send(); } template -void RemoveRequest::handle_remove_child(int r) { +void RemoveRequest::handle_detach_child(int r) { ldout(m_cct, 20) << "r=" << r << dendl; - if (r == -ENOENT) { - r = 0; - } else if (r < 0) { - lderr(m_cct) << "error removing child from children list: " + if (r < 0) { + lderr(m_cct) << "failed to detach child from parent: " << cpp_strerror(r) << dendl; send_close_image(r); return; } - send_disable_mirror(); } diff --git a/src/librbd/image/RemoveRequest.h b/src/librbd/image/RemoveRequest.h index 05adeaa18fd..ab16ed72d15 100644 --- a/src/librbd/image/RemoveRequest.h +++ b/src/librbd/image/RemoveRequest.h @@ -4,6 +4,8 @@ #ifndef CEPH_LIBRBD_IMAGE_REMOVE_REQUEST_H #define CEPH_LIBRBD_IMAGE_REMOVE_REQUEST_H +#include "include/rados/librados.hpp" +#include "librbd/ImageCtx.h" #include "librbd/image/TypeTraits.h" class Context; @@ -12,7 +14,6 @@ class SafeTimer; namespace librbd { -class ImageCtx; class ProgressContext; namespace image { @@ -63,7 +64,7 @@ private: * | TRIM IMAGE | * | | | * v v | - * | REMOVE CHILD | + * | DETACH CHILD | * | | | * | v v * \--------->------------------>CLOSE IMAGE | @@ -118,6 +119,8 @@ private: bool m_unknown_format = true; ImageCtxT *m_image_ctx; + librados::IoCtx m_parent_io_ctx; + decltype(m_image_ctx->exclusive_lock) m_exclusive_lock = nullptr; int m_ret_val = 0; @@ -163,8 +166,8 @@ private: void trim_image(); void handle_trim_image(int r); - void remove_child(); - void handle_remove_child(int r); + void detach_child(); + void handle_detach_child(int r); void send_disable_mirror(); void handle_disable_mirror(int r); diff --git a/src/test/librbd/image/test_mock_RemoveRequest.cc b/src/test/librbd/image/test_mock_RemoveRequest.cc index a4cb4c76251..3b565d594b9 100644 --- a/src/test/librbd/image/test_mock_RemoveRequest.cc +++ b/src/test/librbd/image/test_mock_RemoveRequest.cc @@ -13,6 +13,7 @@ #include "librbd/Operations.h" #include "librbd/operation/TrimRequest.h" #include "librbd/image/TypeTraits.h" +#include "librbd/image/DetachChildRequest.h" #include "librbd/image/RemoveRequest.h" #include "librbd/image/RefreshParentRequest.h" #include "librbd/mirror/DisableRequest.h" @@ -23,20 +24,64 @@ #include namespace librbd { +namespace { + +struct MockTestImageCtx : public MockImageCtx { + static MockTestImageCtx* s_instance; + static MockTestImageCtx* create(const std::string &image_name, + const std::string &image_id, + const char *snap, librados::IoCtx& p, + bool read_only) { + assert(s_instance != nullptr); + return s_instance; + } + + MockTestImageCtx(ImageCtx &image_ctx) : MockImageCtx(image_ctx) { + s_instance = this; + } +}; + +MockTestImageCtx* MockTestImageCtx::s_instance = nullptr; + +} // anonymous namespace namespace image { + template <> -struct TypeTraits { +struct TypeTraits { typedef librbd::MockContextWQ ContextWQ; }; -} + +template <> +class DetachChildRequest { +public: + static DetachChildRequest *s_instance; + static DetachChildRequest *create(MockTestImageCtx &image_ctx, + Context *on_finish) { + assert(s_instance != nullptr); + s_instance->on_finish = on_finish; + return s_instance; + } + + Context *on_finish = nullptr; + + DetachChildRequest() { + s_instance = this; + } + + MOCK_METHOD0(send, void()); +}; + +DetachChildRequest *DetachChildRequest::s_instance; + +} // namespace image namespace operation { template <> -class TrimRequest { +class TrimRequest { public: static TrimRequest *s_instance; - static TrimRequest *create(MockImageCtx &image_ctx, Context *on_finish, + static TrimRequest *create(MockTestImageCtx &image_ctx, Context *on_finish, uint64_t original_size, uint64_t new_size, ProgressContext &prog_ctx) { assert(s_instance != nullptr); @@ -53,13 +98,16 @@ public: MOCK_METHOD0(send, void()); }; +TrimRequest *TrimRequest::s_instance; + } // namespace operation namespace journal { + template <> -class RemoveRequest { +class RemoveRequest { private: - typedef ::librbd::image::TypeTraits TypeTraits; + typedef ::librbd::image::TypeTraits TypeTraits; typedef typename TypeTraits::ContextWQ ContextWQ; public: static RemoveRequest *s_instance; @@ -79,18 +127,20 @@ public: MOCK_METHOD0(send, void()); }; -RemoveRequest *RemoveRequest::s_instance = nullptr; + +RemoveRequest *RemoveRequest::s_instance = nullptr; + } // namespace journal namespace mirror { template<> -class DisableRequest { +class DisableRequest { public: static DisableRequest *s_instance; Context *on_finish = nullptr; - static DisableRequest *create(MockImageCtx *image_ctx, bool force, + static DisableRequest *create(MockTestImageCtx *image_ctx, bool force, bool remove, Context *on_finish) { assert(s_instance != nullptr); s_instance->on_finish = on_finish; @@ -104,14 +154,13 @@ public: MOCK_METHOD0(send, void()); }; -DisableRequest *DisableRequest::s_instance; +DisableRequest *DisableRequest::s_instance; } // namespace mirror } // namespace librbd // template definitions #include "librbd/image/RemoveRequest.cc" -template class librbd::image::RemoveRequest; namespace librbd { namespace image { @@ -128,28 +177,29 @@ using ::testing::StrEq; class TestMockImageRemoveRequest : public TestMockFixture { public: - typedef ::librbd::image::TypeTraits TypeTraits; + typedef ::librbd::image::TypeTraits TypeTraits; typedef typename TypeTraits::ContextWQ ContextWQ; - typedef RemoveRequest MockRemoveRequest; - typedef librbd::operation::TrimRequest MockTrimRequest; - typedef librbd::journal::RemoveRequest MockJournalRemoveRequest; - typedef librbd::mirror::DisableRequest MockMirrorDisableRequest; + typedef RemoveRequest MockRemoveRequest; + typedef DetachChildRequest MockDetachChildRequest; + typedef librbd::operation::TrimRequest MockTrimRequest; + typedef librbd::journal::RemoveRequest MockJournalRemoveRequest; + typedef librbd::mirror::DisableRequest MockMirrorDisableRequest; librbd::ImageCtx *m_test_imctx = NULL; - MockImageCtx *m_mock_imctx = NULL; + MockTestImageCtx *m_mock_imctx = NULL; void TestImageRemoveSetUp() { ASSERT_EQ(0, open_image(m_image_name, &m_test_imctx)); - m_mock_imctx = new MockImageCtx(*m_test_imctx); - librbd::MockImageCtx::s_instance = m_mock_imctx; + m_mock_imctx = new MockTestImageCtx(*m_test_imctx); + librbd::MockTestImageCtx::s_instance = m_mock_imctx; } void TestImageRemoveTearDown() { - librbd::MockImageCtx::s_instance = NULL; + librbd::MockTestImageCtx::s_instance = NULL; delete m_mock_imctx; } - void expect_state_open(MockImageCtx &mock_image_ctx, int r) { + void expect_state_open(MockTestImageCtx &mock_image_ctx, int r) { EXPECT_CALL(*mock_image_ctx.state, open(_, _)) .WillOnce(Invoke([r](bool open_parent, Context *on_ready) { on_ready->complete(r); @@ -159,7 +209,7 @@ public: } } - void expect_state_close(MockImageCtx &mock_image_ctx) { + void expect_state_close(MockTestImageCtx &mock_image_ctx) { EXPECT_CALL(*mock_image_ctx.state, close(_)) .WillOnce(Invoke([](Context *on_ready) { on_ready->complete(0); @@ -174,7 +224,7 @@ public: })); } - void expect_get_group(MockImageCtx &mock_image_ctx, int r) { + 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"), StrEq("image_group_get"), _, _, _)); @@ -185,31 +235,24 @@ public: } } - void expect_trim(MockImageCtx &mock_image_ctx, + void expect_trim(MockTestImageCtx &mock_image_ctx, MockTrimRequest &mock_trim_request, int r) { EXPECT_CALL(mock_trim_request, send()) .WillOnce(FinishRequest(&mock_trim_request, r, &mock_image_ctx)); } - void expect_journal_remove(MockImageCtx &mock_image_ctx, + void expect_journal_remove(MockTestImageCtx &mock_image_ctx, MockJournalRemoveRequest &mock_journal_remove_request, int r) { EXPECT_CALL(mock_journal_remove_request, send()) .WillOnce(FinishRequest(&mock_journal_remove_request, r, &mock_image_ctx)); } - void expect_mirror_disable(MockImageCtx &mock_image_ctx, + void expect_mirror_disable(MockTestImageCtx &mock_image_ctx, MockMirrorDisableRequest &mock_mirror_disable_request, int r) { EXPECT_CALL(mock_mirror_disable_request, send()) .WillOnce(FinishRequest(&mock_mirror_disable_request, r, &mock_image_ctx)); } - void expect_remove_child(MockImageCtx &mock_image_ctx, int r) { - EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), - exec(RBD_CHILDREN, _, StrEq("rbd"), StrEq("remove_child"), _, - _, _)) - .WillOnce(Return(r)); - } - void expect_remove_mirror_image(librados::IoCtx &ioctx, int r) { EXPECT_CALL(get_mock_io_ctx(ioctx), exec(StrEq("rbd_mirroring"), _, StrEq("rbd"), StrEq("mirror_image_remove"), @@ -217,7 +260,7 @@ public: .WillOnce(Return(r)); } - void expect_mirror_image_get(MockImageCtx &mock_image_ctx, int r) { + void expect_mirror_image_get(MockTestImageCtx &mock_image_ctx, int r) { EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(RBD_MIRRORING, _, StrEq("rbd"), StrEq("mirror_image_get"), _, _, _)) @@ -230,6 +273,12 @@ public: _, _, _)) .WillOnce(Return(r)); } + + void expect_detach_child(MockTestImageCtx &mock_image_ctx, + MockDetachChildRequest& mock_request, int r) { + EXPECT_CALL(mock_request, send()) + .WillOnce(FinishRequest(&mock_request, r, &mock_image_ctx)); + } }; TEST_F(TestMockImageRemoveRequest, SuccessV1) { @@ -281,7 +330,48 @@ TEST_F(TestMockImageRemoveRequest, OpenFailV1) { TestImageRemoveTearDown(); } -TEST_F(TestMockImageRemoveRequest, SuccessV2) { +TEST_F(TestMockImageRemoveRequest, SuccessV2CloneV1) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + TestImageRemoveSetUp(); + + C_SaferCond ctx; + librbd::NoOpProgressContext no_op; + ContextWQ op_work_queue; + MockTrimRequest mock_trim_request; + MockJournalRemoveRequest mock_journal_remove_request; + MockMirrorDisableRequest mock_mirror_disable_request; + + m_mock_imctx->parent_md.spec.pool_id = m_ioctx.get_id(); + m_mock_imctx->parent_md.spec.image_id = "parent id"; + m_mock_imctx->parent_md.spec.snap_id = 234; + + InSequence seq; + expect_state_open(*m_mock_imctx, 0); + expect_mirror_image_get(*m_mock_imctx, 0); + expect_get_group(*m_mock_imctx, 0); + expect_trim(*m_mock_imctx, mock_trim_request, 0); + expect_op_work_queue(*m_mock_imctx); + + MockDetachChildRequest mock_detach_child_request; + expect_detach_child(*m_mock_imctx, mock_detach_child_request, 0); + + expect_mirror_disable(*m_mock_imctx, mock_mirror_disable_request, 0); + expect_state_close(*m_mock_imctx); + expect_wq_queue(op_work_queue, 0); + expect_journal_remove(*m_mock_imctx, mock_journal_remove_request, 0); + expect_remove_mirror_image(m_ioctx, 0); + expect_dir_remove_image(m_ioctx, 0); + + MockRemoveRequest *req = MockRemoveRequest::create(m_ioctx, m_image_name, "", + true, false, no_op, &op_work_queue, &ctx); + req->send(); + + ASSERT_EQ(0, ctx.wait()); + + TestImageRemoveTearDown(); +} + +TEST_F(TestMockImageRemoveRequest, SuccessV2CloneV2) { REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); TestImageRemoveSetUp(); @@ -292,13 +382,20 @@ TEST_F(TestMockImageRemoveRequest, SuccessV2) { MockJournalRemoveRequest mock_journal_remove_request; MockMirrorDisableRequest mock_mirror_disable_request; + m_mock_imctx->parent_md.spec.pool_id = m_ioctx.get_id(); + m_mock_imctx->parent_md.spec.image_id = "parent id"; + m_mock_imctx->parent_md.spec.snap_id = 234; + InSequence seq; expect_state_open(*m_mock_imctx, 0); expect_mirror_image_get(*m_mock_imctx, 0); expect_get_group(*m_mock_imctx, 0); expect_trim(*m_mock_imctx, mock_trim_request, 0); expect_op_work_queue(*m_mock_imctx); - expect_remove_child(*m_mock_imctx, 0); + + MockDetachChildRequest mock_detach_child_request; + expect_detach_child(*m_mock_imctx, mock_detach_child_request, 0); + expect_mirror_disable(*m_mock_imctx, mock_mirror_disable_request, 0); expect_state_close(*m_mock_imctx); expect_wq_queue(op_work_queue, 0); @@ -326,13 +423,20 @@ TEST_F(TestMockImageRemoveRequest, NotExistsV2) { MockJournalRemoveRequest mock_journal_remove_request; MockMirrorDisableRequest mock_mirror_disable_request; + m_mock_imctx->parent_md.spec.pool_id = m_ioctx.get_id(); + m_mock_imctx->parent_md.spec.image_id = "parent id"; + m_mock_imctx->parent_md.spec.snap_id = 234; + InSequence seq; expect_state_open(*m_mock_imctx, 0); expect_mirror_image_get(*m_mock_imctx, 0); expect_get_group(*m_mock_imctx, 0); expect_trim(*m_mock_imctx, mock_trim_request, 0); expect_op_work_queue(*m_mock_imctx); - expect_remove_child(*m_mock_imctx, 0); + + MockDetachChildRequest mock_detach_child_request; + expect_detach_child(*m_mock_imctx, mock_detach_child_request, 0); + expect_mirror_disable(*m_mock_imctx, mock_mirror_disable_request, 0); expect_state_close(*m_mock_imctx); expect_wq_queue(op_work_queue, 0);