From 6949adba21d7457a3d002b93318eae3f68c1663b Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 29 Aug 2018 13:03:59 -0400 Subject: [PATCH] librbd: replace existing detach parent calls with new state machine Signed-off-by: Jason Dillaman --- src/librbd/deep_copy/SetHeadRequest.cc | 21 ++++----- src/librbd/deep_copy/SetHeadRequest.h | 6 +-- src/librbd/operation/FlattenRequest.cc | 23 +++++----- src/librbd/operation/FlattenRequest.h | 6 +-- .../deep_copy/test_mock_SetHeadRequest.cc | 45 ++++++++++++++++--- 5 files changed, 62 insertions(+), 39 deletions(-) diff --git a/src/librbd/deep_copy/SetHeadRequest.cc b/src/librbd/deep_copy/SetHeadRequest.cc index 35da7471641..16bbb04ad55 100644 --- a/src/librbd/deep_copy/SetHeadRequest.cc +++ b/src/librbd/deep_copy/SetHeadRequest.cc @@ -7,6 +7,7 @@ #include "cls/rbd/cls_rbd_types.h" #include "librbd/ExclusiveLock.h" #include "librbd/Utils.h" +#include "librbd/image/DetachParentRequest.h" #define dout_subsys ceph_subsys_rbd #undef dout_prefix @@ -40,7 +41,7 @@ void SetHeadRequest::send_set_size() { m_image_ctx->snap_lock.get_read(); if (m_image_ctx->size == m_size) { m_image_ctx->snap_lock.put_read(); - send_remove_parent(); + send_detach_parent(); return; } m_image_ctx->snap_lock.put_read(); @@ -95,11 +96,11 @@ void SetHeadRequest::handle_set_size(int r) { m_image_ctx->size = m_size; } - send_remove_parent(); + send_detach_parent(); } template -void SetHeadRequest::send_remove_parent() { +void SetHeadRequest::send_detach_parent() { m_image_ctx->parent_lock.get_read(); if (m_image_ctx->parent_md.spec.pool_id == -1 || (m_image_ctx->parent_md.spec == m_parent_spec && @@ -111,10 +112,6 @@ void SetHeadRequest::send_remove_parent() { m_image_ctx->parent_lock.put_read(); ldout(m_cct, 20) << dendl; - - librados::ObjectWriteOperation op; - librbd::cls_client::remove_parent(&op); - auto finish_op_ctx = start_lock_op(); if (finish_op_ctx == nullptr) { lderr(m_cct) << "lost exclusive lock" << dendl; @@ -123,17 +120,15 @@ void SetHeadRequest::send_remove_parent() { } auto ctx = new FunctionContext([this, finish_op_ctx](int r) { - handle_remove_parent(r); + handle_detach_parent(r); finish_op_ctx->complete(0); }); - librados::AioCompletion *comp = create_rados_callback(ctx); - int r = m_image_ctx->md_ctx.aio_operate(m_image_ctx->header_oid, comp, &op); - ceph_assert(r == 0); - comp->release(); + auto req = image::DetachParentRequest::create(*m_image_ctx, ctx); + req->send(); } template -void SetHeadRequest::handle_remove_parent(int r) { +void SetHeadRequest::handle_detach_parent(int r) { ldout(m_cct, 20) << "r=" << r << dendl; if (r < 0) { diff --git a/src/librbd/deep_copy/SetHeadRequest.h b/src/librbd/deep_copy/SetHeadRequest.h index 7754ff73553..14d8013599a 100644 --- a/src/librbd/deep_copy/SetHeadRequest.h +++ b/src/librbd/deep_copy/SetHeadRequest.h @@ -46,7 +46,7 @@ private: * SET_SIZE * | * v (skip if not needed) - * REMOVE_PARENT + * DETACH_PARENT * | * v (skip if not needed) * SET_PARENT @@ -68,8 +68,8 @@ private: void send_set_size(); void handle_set_size(int r); - void send_remove_parent(); - void handle_remove_parent(int r); + void send_detach_parent(); + void handle_detach_parent(int r); void send_set_parent(); void handle_set_parent(int r); diff --git a/src/librbd/operation/FlattenRequest.cc b/src/librbd/operation/FlattenRequest.cc index 0bfdfcd09fa..e55daf2670b 100644 --- a/src/librbd/operation/FlattenRequest.cc +++ b/src/librbd/operation/FlattenRequest.cc @@ -6,6 +6,7 @@ #include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/image/DetachChildRequest.h" +#include "librbd/image/DetachParentRequest.h" #include "librbd/Types.h" #include "librbd/io/ObjectRequest.h" #include "common/dout.h" @@ -147,7 +148,7 @@ void FlattenRequest::detach_child() { !image_ctx.snaps.empty()) { image_ctx.snap_lock.put_read(); image_ctx.owner_lock.put_read(); - remove_parent(); + detach_parent(); return; } image_ctx.snap_lock.put_read(); @@ -173,11 +174,11 @@ void FlattenRequest::handle_detach_child(int r) { return; } - remove_parent(); + detach_parent(); } template -void FlattenRequest::remove_parent() { +void FlattenRequest::detach_parent() { I &image_ctx = this->m_image_ctx; CephContext *cct = image_ctx.cct; ldout(cct, 5) << dendl; @@ -200,25 +201,21 @@ void FlattenRequest::remove_parent() { image_ctx.parent_lock.put_read(); // remove parent from this (base) image - librados::ObjectWriteOperation op; - cls_client::remove_parent(&op); - - auto aio_comp = create_rados_callback< + auto ctx = create_context_callback< FlattenRequest, - &FlattenRequest::handle_remove_parent>(this); - int r = image_ctx.md_ctx.aio_operate(image_ctx.header_oid, aio_comp, &op); - ceph_assert(r == 0); - aio_comp->release(); + &FlattenRequest::handle_detach_parent>(this); + auto req = image::DetachParentRequest::create(image_ctx, ctx); + req->send(); image_ctx.owner_lock.put_read(); } template -void FlattenRequest::handle_remove_parent(int r) { +void FlattenRequest::handle_detach_parent(int r) { I &image_ctx = this->m_image_ctx; CephContext *cct = image_ctx.cct; ldout(cct, 5) << "r=" << r << dendl; - if (r < 0 && r != -ENOENT) { + if (r < 0) { lderr(cct) << "remove parent encountered an error: " << cpp_strerror(r) << dendl; } diff --git a/src/librbd/operation/FlattenRequest.h b/src/librbd/operation/FlattenRequest.h index 171ebae0075..78730963cd6 100644 --- a/src/librbd/operation/FlattenRequest.h +++ b/src/librbd/operation/FlattenRequest.h @@ -46,7 +46,7 @@ private: * DETACH_CHILD * | * v - * REMOVE_PARENT + * DETACH_PARENT * | * v * @@ -64,8 +64,8 @@ private: void detach_child(); void handle_detach_child(int r); - void remove_parent(); - void handle_remove_parent(int r); + void detach_parent(); + void handle_detach_parent(int r); }; diff --git a/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc b/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc index 4477fa0f4c1..c94ccf4214e 100644 --- a/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc +++ b/src/test/librbd/deep_copy/test_mock_SetHeadRequest.cc @@ -10,6 +10,7 @@ #include "test/librados_test_stub/MockTestMemIoCtxImpl.h" #include "test/librbd/mock/MockImageCtx.h" #include "librbd/deep_copy/SetHeadRequest.h" +#include "librbd/image/DetachParentRequest.h" namespace librbd { namespace { @@ -21,6 +22,32 @@ struct MockTestImageCtx : public librbd::MockImageCtx { }; } // anonymous namespace + +namespace image { + +template <> +class DetachParentRequest { +public: + static DetachParentRequest *s_instance; + static DetachParentRequest *create(MockTestImageCtx &image_ctx, + Context *on_finish) { + ceph_assert(s_instance != nullptr); + s_instance->on_finish = on_finish; + return s_instance; + } + + Context *on_finish = nullptr; + + DetachParentRequest() { + s_instance = this; + } + + MOCK_METHOD0(send, void()); +}; + +DetachParentRequest *DetachParentRequest::s_instance; + +} // namespace image } // namespace librbd // template definitions @@ -43,6 +70,7 @@ using ::testing::WithArg; class TestMockDeepCopySetHeadRequest : public TestMockFixture { public: typedef SetHeadRequest MockSetHeadRequest; + typedef image::DetachParentRequest MockDetachParentRequest; librbd::ImageCtx *m_image_ctx; ThreadPool *m_thread_pool; @@ -74,10 +102,10 @@ public: .WillOnce(Return(r)); } - void expect_remove_parent(librbd::MockTestImageCtx &mock_image_ctx, int r) { - EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), - exec(mock_image_ctx.header_oid, _, StrEq("rbd"), StrEq("remove_parent"), _, _, _)) - .WillOnce(Return(r)); + void expect_detach_parent(MockImageCtx &mock_image_ctx, + MockDetachParentRequest& mock_request, int r) { + EXPECT_CALL(mock_request, send()) + .WillOnce(FinishRequest(&mock_request, r, &mock_image_ctx)); } void expect_set_parent(librbd::MockTestImageCtx &mock_image_ctx, int r) { @@ -134,7 +162,8 @@ TEST_F(TestMockDeepCopySetHeadRequest, RemoveParent) { InSequence seq; expect_start_op(mock_exclusive_lock); - expect_remove_parent(mock_image_ctx, 0); + MockDetachParentRequest mock_detach_parent; + expect_detach_parent(mock_image_ctx, mock_detach_parent, 0); C_SaferCond ctx; auto request = create_request(mock_image_ctx, m_image_ctx->size, {}, 0, &ctx); @@ -151,7 +180,8 @@ TEST_F(TestMockDeepCopySetHeadRequest, RemoveParentError) { InSequence seq; expect_start_op(mock_exclusive_lock); - expect_remove_parent(mock_image_ctx, -EINVAL); + MockDetachParentRequest mock_detach_parent; + expect_detach_parent(mock_image_ctx, mock_detach_parent, -EINVAL); C_SaferCond ctx; auto request = create_request(mock_image_ctx, m_image_ctx->size, {}, 0, &ctx); @@ -168,7 +198,8 @@ TEST_F(TestMockDeepCopySetHeadRequest, RemoveSetParent) { InSequence seq; expect_start_op(mock_exclusive_lock); - expect_remove_parent(mock_image_ctx, 0); + MockDetachParentRequest mock_detach_parent; + expect_detach_parent(mock_image_ctx, mock_detach_parent, 0); expect_start_op(mock_exclusive_lock); expect_set_parent(mock_image_ctx, 0); -- 2.39.5