From 8455d6611c48ae8a721d307d157b64d8a7041abe Mon Sep 17 00:00:00 2001 From: Mykola Golub Date: Mon, 15 Apr 2019 11:32:15 +0100 Subject: [PATCH] librbd: async trash remove state machine Signed-off-by: Mykola Golub --- src/librbd/CMakeLists.txt | 1 + src/librbd/trash/RemoveRequest.cc | 171 +++++++++++++ src/librbd/trash/RemoveRequest.h | 119 +++++++++ src/test/librbd/CMakeLists.txt | 1 + .../librbd/trash/test_mock_RemoveRequest.cc | 231 ++++++++++++++++++ 5 files changed, 523 insertions(+) create mode 100644 src/librbd/trash/RemoveRequest.cc create mode 100644 src/librbd/trash/RemoveRequest.h create mode 100644 src/test/librbd/trash/test_mock_RemoveRequest.cc diff --git a/src/librbd/CMakeLists.txt b/src/librbd/CMakeLists.txt index 265c95f1871..af6217c06d6 100644 --- a/src/librbd/CMakeLists.txt +++ b/src/librbd/CMakeLists.txt @@ -136,6 +136,7 @@ set(librbd_internal_srcs operation/SparsifyRequest.cc operation/TrimRequest.cc trash/MoveRequest.cc + trash/RemoveRequest.cc watcher/Notifier.cc watcher/RewatchRequest.cc ${CMAKE_SOURCE_DIR}/src/common/ContextCompletion.cc) diff --git a/src/librbd/trash/RemoveRequest.cc b/src/librbd/trash/RemoveRequest.cc new file mode 100644 index 00000000000..77cbfd81b41 --- /dev/null +++ b/src/librbd/trash/RemoveRequest.cc @@ -0,0 +1,171 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "librbd/trash/RemoveRequest.h" +#include "common/WorkQueue.h" +#include "common/dout.h" +#include "common/errno.h" +#include "cls/rbd/cls_rbd_client.h" +#include "librbd/ExclusiveLock.h" +#include "librbd/ImageCtx.h" +#include "librbd/ImageState.h" +#include "librbd/Utils.h" +#include "librbd/image/RemoveRequest.h" + +#define dout_subsys ceph_subsys_rbd +#undef dout_prefix +#define dout_prefix *_dout << "librbd::trash::RemoveRequest: " << this \ + << " " << __func__ << ": " + +namespace librbd { +namespace trash { + +using util::create_context_callback; +using util::create_rados_callback; + +template +void RemoveRequest::send() { + set_state(); +} + +template +void RemoveRequest::set_state() { + ldout(m_cct, 10) << dendl; + + librados::ObjectWriteOperation op; + cls_client::trash_state_set(&op, m_image_id, m_trash_set_state, + m_trash_expect_state); + + auto aio_comp = create_rados_callback< + RemoveRequest, &RemoveRequest::handle_set_state>(this); + int r = m_io_ctx.aio_operate(RBD_TRASH, aio_comp, &op); + ceph_assert(r == 0); + aio_comp->release(); +} + +template +void RemoveRequest::handle_set_state(int r) { + ldout(m_cct, 10) << "r=" << r << dendl; + + if (r < 0 && r != -EOPNOTSUPP) { + lderr(m_cct) << "error setting trash image state: " << cpp_strerror(r) + << dendl; + if (m_ret_val == 0) { + m_ret_val = r; + } + if (m_trash_set_state == cls::rbd::TRASH_IMAGE_STATE_REMOVING) { + close_image(); + } else { + finish(m_ret_val); + } + return; + } + + if (m_trash_set_state == cls::rbd::TRASH_IMAGE_STATE_REMOVING) { + remove_image(); + } else { + ceph_assert(m_trash_set_state == cls::rbd::TRASH_IMAGE_STATE_NORMAL); + finish(m_ret_val < 0 ? m_ret_val : r); + }; +} + +template +void RemoveRequest::close_image() { + if (m_image_ctx == nullptr) { + finish(m_ret_val); + return; + } + + ldout(m_cct, 10) << dendl; + + auto ctx = create_context_callback< + RemoveRequest, &RemoveRequest::handle_close_image>(this); + m_image_ctx->state->close(ctx); +} + +template +void RemoveRequest::handle_close_image(int r) { + ldout(m_cct, 10) << "r=" << r << dendl; + + if (r < 0) { + ldout(m_cct, 5) << "failed to close image:" << cpp_strerror(r) << dendl; + } + + m_image_ctx->destroy(); + m_image_ctx = nullptr; + finish(m_ret_val); +} + +template +void RemoveRequest::remove_image() { + ldout(m_cct, 10) << dendl; + + auto ctx = create_context_callback< + RemoveRequest, &RemoveRequest::handle_remove_image>(this); + if (m_image_ctx != nullptr) { + auto req = librbd::image::RemoveRequest::create( + m_io_ctx, m_image_ctx, m_force, true, m_prog_ctx, m_op_work_queue, ctx); + req->send(); + } else { + auto req = librbd::image::RemoveRequest::create( + m_io_ctx, "", m_image_id, m_force, true, m_prog_ctx, m_op_work_queue, + ctx); + req->send(); + } +} + +template +void RemoveRequest::handle_remove_image(int r) { + ldout(m_cct, 10) << "r=" << r << dendl; + + if (r < 0) { + ldout(m_cct, 5) << "failed to remove image:" << cpp_strerror(r) << dendl; + + m_ret_val = r; + m_trash_set_state = cls::rbd::TRASH_IMAGE_STATE_NORMAL; + m_trash_expect_state = cls::rbd::TRASH_IMAGE_STATE_REMOVING; + set_state(); + return; + } + + m_image_ctx = nullptr; + remove_trash_entry(); +} + +template +void RemoveRequest::remove_trash_entry() { + ldout(m_cct, 10) << dendl; + + librados::ObjectWriteOperation op; + cls_client::trash_remove(&op, m_image_id); + + auto aio_comp = create_rados_callback< + RemoveRequest, &RemoveRequest::handle_remove_trash_entry>(this); + int r = m_io_ctx.aio_operate(RBD_TRASH, aio_comp, &op); + ceph_assert(r == 0); + aio_comp->release(); +} + +template +void RemoveRequest::handle_remove_trash_entry(int r) { + ldout(m_cct, 10) << "r=" << r << dendl; + + if (r < 0 && r != -ENOENT) { + lderr(m_cct) << "error removing trash entry: " << cpp_strerror(r) << dendl; + } + + finish(0); +} + +template +void RemoveRequest::finish(int r) { + ldout(m_cct, 10) << "r=" << r << dendl; + + m_on_finish->complete(r); + delete this; +} + +} // namespace trash +} // namespace librbd + +template class librbd::trash::RemoveRequest; diff --git a/src/librbd/trash/RemoveRequest.h b/src/librbd/trash/RemoveRequest.h new file mode 100644 index 00000000000..5f65a57ce9e --- /dev/null +++ b/src/librbd/trash/RemoveRequest.h @@ -0,0 +1,119 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#ifndef CEPH_LIBRBD_TRASH_REMOVE_REQUEST_H +#define CEPH_LIBRBD_TRASH_REMOVE_REQUEST_H + +#include "include/utime.h" +#include "include/rados/librados.hpp" +#include "cls/rbd/cls_rbd_types.h" +#include + +class CephContext; +class Context; +class ContextWQ; + +namespace librbd { + +class ProgressContext; + +struct ImageCtx; + +namespace trash { + +template +class RemoveRequest { +public: + static RemoveRequest* create(librados::IoCtx &io_ctx, + const std::string &image_id, + ContextWQ *op_work_queue, bool force, + ProgressContext &prog_ctx, Context *on_finish) { + return new RemoveRequest(io_ctx, image_id, op_work_queue, force, prog_ctx, + on_finish); + } + + static RemoveRequest* create(librados::IoCtx &io_ctx, ImageCtxT *image_ctx, + ContextWQ *op_work_queue, bool force, + ProgressContext &prog_ctx, Context *on_finish) { + return new RemoveRequest(io_ctx, image_ctx, op_work_queue, force, prog_ctx, + on_finish); + } + + + RemoveRequest(librados::IoCtx &io_ctx, const std::string &image_id, + ContextWQ *op_work_queue, bool force, ProgressContext &prog_ctx, + Context *on_finish) + : m_io_ctx(io_ctx), m_image_id(image_id), m_op_work_queue(op_work_queue), + m_force(force), m_prog_ctx(prog_ctx), m_on_finish(on_finish), + m_cct(reinterpret_cast(io_ctx.cct())) { + } + + RemoveRequest(librados::IoCtx &io_ctx, ImageCtxT *image_ctx, + ContextWQ *op_work_queue, bool force, ProgressContext &prog_ctx, + Context *on_finish) + : m_io_ctx(io_ctx), m_image_ctx(image_ctx), m_image_id(m_image_ctx->id), + m_op_work_queue(op_work_queue), m_force(force), m_prog_ctx(prog_ctx), + m_on_finish(on_finish), + m_cct(reinterpret_cast(io_ctx.cct())) { + } + + void send(); + +private: + /* + * @verbatim + * + * + * | + * v + * SET_STATE (removing) * * * * * * *> CLOSE_IMAGE + * | | + * v | + * REMOVE_IMAGE * * *> SET_STATE (normal) | + * | | | + * v | | + * REMOVE_TRASH_ENTRY | | + * | | | + * v | | + * <-------------/<---------------/ + * + * @endverbatim + */ + + librados::IoCtx &m_io_ctx; + ImageCtxT *m_image_ctx = nullptr; + std::string m_image_id; + ContextWQ *m_op_work_queue; + bool m_force; + ProgressContext &m_prog_ctx; + Context *m_on_finish; + + CephContext *m_cct; + + cls::rbd::TrashImageState m_trash_set_state = + cls::rbd::TRASH_IMAGE_STATE_REMOVING; + cls::rbd::TrashImageState m_trash_expect_state = + cls::rbd::TRASH_IMAGE_STATE_NORMAL; + int m_ret_val = 0; + + void set_state(); + void handle_set_state(int r); + + void close_image(); + void handle_close_image(int r); + + void remove_image(); + void handle_remove_image(int r); + + void remove_trash_entry(); + void handle_remove_trash_entry(int r); + + void finish(int r); +}; + +} // namespace trash +} // namespace librbd + +extern template class librbd::trash::RemoveRequest; + +#endif // CEPH_LIBRBD_TRASH_REMOVE_REQUEST_H diff --git a/src/test/librbd/CMakeLists.txt b/src/test/librbd/CMakeLists.txt index b0fb77f464a..0c1b7fbd235 100644 --- a/src/test/librbd/CMakeLists.txt +++ b/src/test/librbd/CMakeLists.txt @@ -105,6 +105,7 @@ set(unittest_librbd_srcs operation/test_mock_SnapshotUnprotectRequest.cc operation/test_mock_TrimRequest.cc trash/test_mock_MoveRequest.cc + trash/test_mock_RemoveRequest.cc watcher/test_mock_RewatchRequest.cc ) add_executable(unittest_librbd diff --git a/src/test/librbd/trash/test_mock_RemoveRequest.cc b/src/test/librbd/trash/test_mock_RemoveRequest.cc new file mode 100644 index 00000000000..407855ae77c --- /dev/null +++ b/src/test/librbd/trash/test_mock_RemoveRequest.cc @@ -0,0 +1,231 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "test/librbd/test_mock_fixture.h" +#include "test/librbd/test_support.h" +#include "test/librbd/mock/MockExclusiveLock.h" +#include "test/librbd/mock/MockImageCtx.h" +#include "test/librbd/mock/MockImageState.h" +#include "test/librados_test_stub/MockTestMemIoCtxImpl.h" +#include "test/librados_test_stub/MockTestMemRadosClient.h" +#include "include/rbd/librbd.hpp" +#include "librbd/Utils.h" +#include "librbd/image/TypeTraits.h" +#include "librbd/image/RemoveRequest.h" +#include "librbd/internal.h" +#include "librbd/trash/RemoveRequest.h" + +namespace librbd { +namespace { + +struct MockTestImageCtx : public MockImageCtx { + MockTestImageCtx(ImageCtx &image_ctx) : MockImageCtx(image_ctx) { + } +}; + +} // anonymous namespace + +namespace image { + +// template <> +// struct TypeTraits { +// typedef librbd::MockContextWQ ContextWQ; +// }; + +template <> +class RemoveRequest { +private: + typedef ::librbd::image::TypeTraits TypeTraits; + typedef typename TypeTraits::ContextWQ ContextWQ; +public: + static RemoveRequest *s_instance; + static RemoveRequest *create(librados::IoCtx &ioctx, + const std::string &image_name, + const std::string &image_id, + bool force, bool from_trash_remove, + ProgressContext &prog_ctx, + ContextWQ *op_work_queue, + Context *on_finish) { + ceph_assert(s_instance != nullptr); + s_instance->on_finish = on_finish; + return s_instance; + } + + static RemoveRequest *create(librados::IoCtx &ioctx, MockTestImageCtx *image_ctx, + bool force, bool from_trash_remove, + ProgressContext &prog_ctx, + ContextWQ *op_work_queue, + Context *on_finish) { + ceph_assert(s_instance != nullptr); + s_instance->on_finish = on_finish; + return s_instance; + } + + Context *on_finish = nullptr; + + RemoveRequest() { + s_instance = this; + } + + MOCK_METHOD0(send, void()); +}; + +RemoveRequest *RemoveRequest::s_instance; + +} // namespace image +} // namespace librbd + +#include "librbd/trash/RemoveRequest.cc" + +namespace librbd { +namespace trash { + +using ::testing::_; +using ::testing::InSequence; +using ::testing::Invoke; +using ::testing::Return; +using ::testing::StrEq; + +struct TestMockTrashRemoveRequest : public TestMockFixture { + typedef RemoveRequest MockRemoveRequest; + typedef image::RemoveRequest MockImageRemoveRequest; + + NoOpProgressContext m_prog_ctx; + + void expect_set_state(MockTestImageCtx& mock_image_ctx, + cls::rbd::TrashImageState trash_set_state, + cls::rbd::TrashImageState trash_expect_state, + int r) { + bufferlist in_bl; + encode(mock_image_ctx.id, in_bl); + encode(trash_set_state, in_bl); + encode(trash_expect_state, in_bl); + + EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), + exec(StrEq("rbd_trash"), _, StrEq("rbd"), StrEq("trash_state_set"), + ContentsEqual(in_bl), _, _)) + .WillOnce(Return(r)); + } + + void expect_set_deleting_state(MockTestImageCtx& mock_image_ctx, int r) { + expect_set_state(mock_image_ctx, cls::rbd::TRASH_IMAGE_STATE_REMOVING, + cls::rbd::TRASH_IMAGE_STATE_NORMAL, r); + } + + void expect_restore_normal_state(MockTestImageCtx& mock_image_ctx, int r) { + expect_set_state(mock_image_ctx, cls::rbd::TRASH_IMAGE_STATE_NORMAL, + cls::rbd::TRASH_IMAGE_STATE_REMOVING, r); + } + + void expect_close_image(MockTestImageCtx &mock_image_ctx, int r) { + EXPECT_CALL(*mock_image_ctx.state, close(_)) + .WillOnce(Invoke([r](Context *on_finish) { + on_finish->complete(r); + })); + EXPECT_CALL(mock_image_ctx, destroy()); + } + + void expect_remove_image(MockImageRemoveRequest& mock_image_remove_request, + int r) { + EXPECT_CALL(mock_image_remove_request, send()) + .WillOnce(Invoke([&mock_image_remove_request, r]() { + mock_image_remove_request.on_finish->complete(r); + })); + } + + void expect_remove_trash_entry(MockTestImageCtx& mock_image_ctx, + int r) { + bufferlist in_bl; + encode(mock_image_ctx.id, in_bl); + + EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), + exec(StrEq("rbd_trash"), _, StrEq("rbd"), StrEq("trash_remove"), + ContentsEqual(in_bl), _, _)) + .WillOnce(Return(r)); + } +}; + +TEST_F(TestMockTrashRemoveRequest, Success) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + MockImageRemoveRequest mock_image_remove_request; + + InSequence seq; + expect_set_deleting_state(mock_image_ctx, 0); + expect_remove_image(mock_image_remove_request, 0); + expect_remove_trash_entry(mock_image_ctx, 0); + + C_SaferCond ctx; + auto req = MockRemoveRequest::create(mock_image_ctx.md_ctx, &mock_image_ctx, + nullptr, false, m_prog_ctx, &ctx); + req->send(); + ASSERT_EQ(0, ctx.wait()); +} +TEST_F(TestMockTrashRemoveRequest, SetDeletingStateError) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + MockImageRemoveRequest mock_image_remove_request; + + InSequence seq; + expect_set_deleting_state(mock_image_ctx, -EINVAL); + expect_close_image(mock_image_ctx, 0); + + C_SaferCond ctx; + auto req = MockRemoveRequest::create(mock_image_ctx.md_ctx, &mock_image_ctx, + nullptr, false, m_prog_ctx, &ctx); + req->send(); + ASSERT_EQ(-EINVAL, ctx.wait()); +} + +TEST_F(TestMockTrashRemoveRequest, RemoveImageError) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + MockImageRemoveRequest mock_image_remove_request; + + InSequence seq; + expect_set_deleting_state(mock_image_ctx, 0); + expect_remove_image(mock_image_remove_request, -EINVAL); + expect_restore_normal_state(mock_image_ctx, 0); + + C_SaferCond ctx; + auto req = MockRemoveRequest::create(mock_image_ctx.md_ctx, &mock_image_ctx, + nullptr, false, m_prog_ctx, &ctx); + req->send(); + ASSERT_EQ(-EINVAL, ctx.wait()); +} + +TEST_F(TestMockTrashRemoveRequest, RemoveTrashEntryError) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + MockImageRemoveRequest mock_image_remove_request; + + InSequence seq; + expect_set_deleting_state(mock_image_ctx, 0); + expect_remove_image(mock_image_remove_request, 0); + expect_remove_trash_entry(mock_image_ctx, -EINVAL); + + C_SaferCond ctx; + auto req = MockRemoveRequest::create(mock_image_ctx.md_ctx, &mock_image_ctx, + nullptr, false, m_prog_ctx, &ctx); + req->send(); + ASSERT_EQ(0, ctx.wait()); +} + +} // namespace trash +} // namespace librbd -- 2.39.5