From 86a4b33d73cd4a78aef84a8ca473564bc4888cd1 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 21 Nov 2017 22:39:03 -0500 Subject: [PATCH] librbd: async trash move state machine Signed-off-by: Jason Dillaman --- src/cls/rbd/cls_rbd_types.h | 8 +- src/librbd/CMakeLists.txt | 1 + src/librbd/internal.cc | 128 ++++------ src/librbd/trash/MoveRequest.cc | 124 ++++++++++ src/librbd/trash/MoveRequest.h | 88 +++++++ src/test/librbd/CMakeLists.txt | 1 + .../librbd/trash/test_mock_MoveRequest.cc | 230 ++++++++++++++++++ 7 files changed, 492 insertions(+), 88 deletions(-) create mode 100644 src/librbd/trash/MoveRequest.cc create mode 100644 src/librbd/trash/MoveRequest.h create mode 100644 src/test/librbd/trash/test_mock_MoveRequest.cc diff --git a/src/cls/rbd/cls_rbd_types.h b/src/cls/rbd/cls_rbd_types.h index 87c0a397d7605..905f53076ba5e 100644 --- a/src/cls/rbd/cls_rbd_types.h +++ b/src/cls/rbd/cls_rbd_types.h @@ -354,9 +354,11 @@ struct TrashImageSpec { TrashImageSpec() {} TrashImageSpec(TrashImageSource source, const std::string &name, - utime_t deletion_time, utime_t deferment_end_time) : - source(source), name(name), deletion_time(deletion_time), - deferment_end_time(deferment_end_time) {} + const utime_t& deletion_time, + const utime_t& deferment_end_time) + : source(source), name(name), deletion_time(deletion_time), + deferment_end_time(deferment_end_time) { + } void encode(bufferlist &bl) const; void decode(bufferlist::iterator& it); diff --git a/src/librbd/CMakeLists.txt b/src/librbd/CMakeLists.txt index 8025fa4248bdd..9c0d8838d02ce 100644 --- a/src/librbd/CMakeLists.txt +++ b/src/librbd/CMakeLists.txt @@ -107,6 +107,7 @@ set(librbd_internal_srcs operation/SnapshotUnprotectRequest.cc operation/SnapshotLimitRequest.cc operation/TrimRequest.cc + trash/MoveRequest.cc watcher/Notifier.cc watcher/RewatchRequest.cc ${CMAKE_SOURCE_DIR}/src/common/ContextCompletion.cc) diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index 1a34ac8a46fc7..69cfda50b9639 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -45,6 +45,7 @@ #include "librbd/managed_lock/Types.h" #include "librbd/mirror/EnableRequest.h" #include "librbd/operation/TrimRequest.h" +#include "librbd/trash/MoveRequest.h" #include "journal/Journaler.h" @@ -1351,102 +1352,59 @@ bool compare_by_name(const child_info_t& c1, const child_info_t& c2) ldout(cct, 20) << "trash_move " << &io_ctx << " " << image_name << dendl; + // try to get image id from the directory std::string image_id; - ImageCtx *ictx = new ImageCtx(image_name, "", nullptr, io_ctx, false); - int r = ictx->state->open(true); - if (r < 0) { - ictx = nullptr; - - if (r != -ENOENT) { - ldout(cct, 2) << "error opening image: " << cpp_strerror(-r) << dendl; - return r; - } - - // try to get image id from the directory - r = cls_client::dir_get_id(&io_ctx, RBD_DIRECTORY, image_name, &image_id); - if (r < 0) { - if (r != -ENOENT) { - ldout(cct, 2) << "error reading image id from dirctory: " - << cpp_strerror(-r) << dendl; - } - return r; - } - } else { - if (ictx->old_format) { - ictx->state->close(); - return -EOPNOTSUPP; - } + int r = cls_client::dir_get_id(&io_ctx, RBD_DIRECTORY, image_name, + &image_id); + if (r < 0 && r != -ENOENT) { + lderr(cct) << "failed to retrieve image id: " << cpp_strerror(r) << dendl; + return r; + } - image_id = ictx->id; - ictx->owner_lock.get_read(); - if (ictx->exclusive_lock != nullptr) { - r = ictx->operations->prepare_image_update(false); - if (r < 0) { - lderr(cct) << "cannot obtain exclusive lock - not removing" << dendl; - ictx->owner_lock.put_read(); - ictx->state->close(); - return -EBUSY; - } - } + ImageCtx *ictx = new ImageCtx((image_id.empty() ? image_name : ""), + image_id, nullptr, io_ctx, false); + r = ictx->state->open(true); + if (r == -ENOENT) { + return r; + } else if (r < 0) { + lderr(cct) << "failed to open image: " << cpp_strerror(r) << dendl; + return r; + } else if (ictx->old_format) { + ldout(cct, 10) << "cannot move v1 image to trash" << dendl; + ictx->state->close(); + return -EOPNOTSUPP; } - BOOST_SCOPE_EXIT_ALL(ictx, cct) { - if (ictx == nullptr) - return; + image_id = ictx->id; + ictx->owner_lock.get_read(); + if (ictx->exclusive_lock != nullptr) { + ictx->exclusive_lock->block_requests(0); - bool is_locked = ictx->exclusive_lock != nullptr && - ictx->exclusive_lock->is_lock_owner(); - if (is_locked) { - C_SaferCond ctx; - auto exclusive_lock = ictx->exclusive_lock; - exclusive_lock->shut_down(&ctx); - ictx->owner_lock.put_read(); - int r = ctx.wait(); - if (r < 0) { - lderr(cct) << "error shutting down exclusive lock" << dendl; - } - delete exclusive_lock; - } else { + r = ictx->operations->prepare_image_update(false); + if (r < 0) { + lderr(cct) << "cannot obtain exclusive lock - not removing" << dendl; ictx->owner_lock.put_read(); + ictx->state->close(); + return -EBUSY; } - ictx->state->close(); - }; - - ldout(cct, 2) << "adding image entry to rbd_trash" << dendl; - utime_t ts = ceph_clock_now(); - utime_t deferment_end_time = ts; - deferment_end_time += (double)delay; - cls::rbd::TrashImageSource trash_source = - static_cast(source); - cls::rbd::TrashImageSpec trash_spec(trash_source, image_name, ts, - deferment_end_time); - r = cls_client::trash_add(&io_ctx, image_id, trash_spec); - if (r < 0 && r != -EEXIST) { - lderr(cct) << "error adding image " << image_name << " to rbd_trash" - << dendl; - return r; - } else if (r == -EEXIST) { - ldout(cct, 10) << "found previous unfinished deferred remove for image:" - << image_id << dendl; - // continue with removing image from directory } + ictx->owner_lock.put_read(); - ldout(cct, 2) << "removing id object..." << dendl; - r = io_ctx.remove(util::id_obj_name(image_name)); - if (r < 0 && r != -ENOENT) { - lderr(cct) << "error removing id object: " << cpp_strerror(r) - << dendl; - return r; - } + utime_t delete_time{ceph_clock_now()}; + utime_t deferment_end_time{delete_time}; + deferment_end_time += delay; + cls::rbd::TrashImageSpec trash_image_spec{ + static_cast(source), ictx->name, + delete_time, deferment_end_time}; - ldout(cct, 2) << "removing rbd image from v2 directory..." << dendl; - r = cls_client::dir_remove_image(&io_ctx, RBD_DIRECTORY, image_name, - image_id); + C_SaferCond ctx; + auto req = trash::MoveRequest<>::create(io_ctx, ictx->id, trash_image_spec, + &ctx); + req->send(); + + r = ctx.wait(); + ictx->state->close(); if (r < 0) { - if (r != -ENOENT) { - lderr(cct) << "error removing image from v2 directory: " - << cpp_strerror(-r) << dendl; - } return r; } diff --git a/src/librbd/trash/MoveRequest.cc b/src/librbd/trash/MoveRequest.cc new file mode 100644 index 0000000000000..cf80c853b3678 --- /dev/null +++ b/src/librbd/trash/MoveRequest.cc @@ -0,0 +1,124 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#include "librbd/trash/MoveRequest.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" + +#define dout_subsys ceph_subsys_rbd +#undef dout_prefix +#define dout_prefix *_dout << "librbd::trash::MoveRequest: " << this \ + << " " << __func__ << ": " + +namespace librbd { +namespace trash { + +using util::create_context_callback; +using util::create_rados_callback; + +template +void MoveRequest::send() { + trash_add(); +} + +template +void MoveRequest::trash_add() { + ldout(m_cct, 10) << dendl; + + librados::ObjectWriteOperation op; + librbd::cls_client::trash_add(&op, m_image_id, m_trash_image_spec); + + auto aio_comp = create_rados_callback< + MoveRequest, &MoveRequest::handle_trash_add>(this); + int r = m_io_ctx.aio_operate(RBD_TRASH, aio_comp, &op); + assert(r == 0); + aio_comp->release(); +} + +template +void MoveRequest::handle_trash_add(int r) { + ldout(m_cct, 10) << "r=" << r << dendl; + + if (r == -EEXIST) { + ldout(m_cct, 10) << "previous unfinished deferred remove for image: " + << m_image_id << dendl; + } else if (r < 0) { + lderr(m_cct) << "failed to add image to trash: " << cpp_strerror(r) + << dendl; + finish(r); + return; + } + + remove_id(); +} + +template +void MoveRequest::remove_id() { + ldout(m_cct, 10) << dendl; + + auto aio_comp = create_rados_callback< + MoveRequest, &MoveRequest::handle_remove_id>(this); + int r = m_io_ctx.aio_remove(util::id_obj_name(m_trash_image_spec.name), + aio_comp); + assert(r == 0); + aio_comp->release(); +} + +template +void MoveRequest::handle_remove_id(int r) { + ldout(m_cct, 10) << "r=" << r << dendl; + + if (r < 0 && r != -ENOENT) { + lderr(m_cct) << "failed to remove image id object: " << cpp_strerror(r) + << dendl; + finish(r); + return; + } + + directory_remove(); +} + +template +void MoveRequest::directory_remove() { + ldout(m_cct, 10) << dendl; + + librados::ObjectWriteOperation op; + librbd::cls_client::dir_remove_image(&op, m_trash_image_spec.name, + m_image_id); + + auto aio_comp = create_rados_callback< + MoveRequest, &MoveRequest::handle_directory_remove>(this); + int r = m_io_ctx.aio_operate(RBD_DIRECTORY, aio_comp, &op); + assert(r == 0); + aio_comp->release(); +} + +template +void MoveRequest::handle_directory_remove(int r) { + ldout(m_cct, 10) << "r=" << r << dendl; + + if (r < 0 && r != -ENOENT) { + lderr(m_cct) << "failed to remove image from directory: " << cpp_strerror(r) + << dendl; + } + + finish(r); +} + +template +void MoveRequest::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::MoveRequest; diff --git a/src/librbd/trash/MoveRequest.h b/src/librbd/trash/MoveRequest.h new file mode 100644 index 0000000000000..1d20ecd528f7b --- /dev/null +++ b/src/librbd/trash/MoveRequest.h @@ -0,0 +1,88 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#ifndef CEPH_LIBRBD_TRASH_MOVE_REQUEST_H +#define CEPH_LIBRBD_TRASH_MOVE_REQUEST_H + +#include "include/utime.h" +#include "include/rados/librados.hpp" +#include "cls/rbd/cls_rbd_types.h" +#include + +struct CephContext; +struct Context; +namespace librados { struct IoCtx; } + +namespace librbd { + +struct ImageCtx; + +namespace trash { + +template +class MoveRequest { +public: + static MoveRequest* create(librados::IoCtx& io_ctx, + const std::string& image_id, + const cls::rbd::TrashImageSpec& trash_image_spec, + Context* on_finish) { + return new MoveRequest(io_ctx, image_id, trash_image_spec, on_finish); + } + + MoveRequest(librados::IoCtx& io_ctx, const std::string& image_id, + const cls::rbd::TrashImageSpec& trash_image_spec, + Context* on_finish) + : m_io_ctx(io_ctx), m_image_id(image_id), + m_trash_image_spec(trash_image_spec), m_on_finish(on_finish), + m_cct(reinterpret_cast(io_ctx.cct())) { + } + + void send(); + +private: + /* + * @verbatim + * + * + * | + * v + * TRASH_ADD + * | + * v + * REMOVE_ID + * | + * v + * DIRECTORY_REMOVE + * | + * v + * + * + * @endverbatim + */ + + librados::IoCtx &m_io_ctx; + std::string m_image_id; + cls::rbd::TrashImageSpec m_trash_image_spec; + Context *m_on_finish; + + CephContext *m_cct; + + void trash_add(); + void handle_trash_add(int r); + + void remove_id(); + void handle_remove_id(int r); + + void directory_remove(); + void handle_directory_remove(int r); + + void finish(int r); + +}; + +} // namespace trash +} // namespace librbd + +extern template class librbd::trash::MoveRequest; + +#endif // CEPH_LIBRBD_TRASH_MOVE_REQUEST_H diff --git a/src/test/librbd/CMakeLists.txt b/src/test/librbd/CMakeLists.txt index b85a3592e71d7..4d3761c760a8c 100644 --- a/src/test/librbd/CMakeLists.txt +++ b/src/test/librbd/CMakeLists.txt @@ -75,6 +75,7 @@ set(unittest_librbd_srcs operation/test_mock_SnapshotRollbackRequest.cc operation/test_mock_SnapshotUnprotectRequest.cc operation/test_mock_TrimRequest.cc + trash/test_mock_MoveRequest.cc watcher/test_mock_RewatchRequest.cc ) add_executable(unittest_librbd diff --git a/src/test/librbd/trash/test_mock_MoveRequest.cc b/src/test/librbd/trash/test_mock_MoveRequest.cc new file mode 100644 index 0000000000000..7fdc36e757a9c --- /dev/null +++ b/src/test/librbd/trash/test_mock_MoveRequest.cc @@ -0,0 +1,230 @@ +// -*- 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/trash/MoveRequest.h" + +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); + s_instance->construct(image_name, image_id); + return s_instance; + } + + MOCK_METHOD2(construct, void(const std::string&, const std::string&)); + + MockTestImageCtx(librbd::ImageCtx &image_ctx) + : librbd::MockImageCtx(image_ctx) { + s_instance = this; + } +}; + +MockTestImageCtx *MockTestImageCtx::s_instance = nullptr; + +} // anonymous namespace +} // namespace librbd + +#include "librbd/trash/MoveRequest.cc" + +namespace librbd { +namespace trash { + +using ::testing::_; +using ::testing::InSequence; +using ::testing::Invoke; +using ::testing::Return; +using ::testing::StrEq; +using ::testing::WithArg; + +struct TestMockTrashMoveRequest : public TestMockFixture { + typedef MoveRequest MockMoveRequest; + + void expect_trash_add(MockTestImageCtx &mock_image_ctx, + const std::string& image_id, + cls::rbd::TrashImageSource trash_image_source, + const std::string& name, + const utime_t& end_time, + int r) { + EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), + exec(StrEq("rbd_trash"), _, StrEq("rbd"), StrEq("trash_add"), + _, _, _)) + .WillOnce(WithArg<4>(Invoke([=](bufferlist& in_bl) { + std::string id; + cls::rbd::TrashImageSpec trash_image_spec; + + bufferlist::iterator bl_it = in_bl.begin(); + ::decode(id, bl_it); + ::decode(trash_image_spec, bl_it); + + EXPECT_EQ(id, image_id); + EXPECT_EQ(trash_image_spec.source, + trash_image_source); + EXPECT_EQ(trash_image_spec.name, name); + EXPECT_EQ(trash_image_spec.deferment_end_time, + end_time); + return r; + }))); + } + + void expect_aio_remove(MockTestImageCtx &mock_image_ctx, + const std::string& oid, int r) { + EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), remove(oid, _)) + .WillOnce(Return(r)); + } + + void expect_dir_remove(MockTestImageCtx& mock_image_ctx, + const std::string& name, const std::string& id, + int r) { + bufferlist in_bl; + ::encode(name, in_bl); + ::encode(id, in_bl); + + EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), + exec(StrEq("rbd_directory"), _, StrEq("rbd"), StrEq("dir_remove_image"), + ContentsEqual(in_bl), _, _)) + .WillOnce(Return(r)); + } +}; + +TEST_F(TestMockTrashMoveRequest, Success) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + MockExclusiveLock mock_exclusive_lock; + if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) { + mock_image_ctx.exclusive_lock = &mock_exclusive_lock; + } + + expect_op_work_queue(mock_image_ctx); + + InSequence seq; + utime_t delete_time{ceph_clock_now()}; + expect_trash_add(mock_image_ctx, "image id", + cls::rbd::TRASH_IMAGE_SOURCE_USER, "image name", delete_time, + 0); + expect_aio_remove(mock_image_ctx, util::id_obj_name("image name"), 0); + expect_dir_remove(mock_image_ctx, "image name", "image id", 0); + + C_SaferCond ctx; + cls::rbd::TrashImageSpec trash_image_spec{ + cls::rbd::TRASH_IMAGE_SOURCE_USER, "image name", delete_time, + delete_time}; + auto req = MockMoveRequest::create(mock_image_ctx.md_ctx, "image id", + trash_image_spec, &ctx); + req->send(); + ASSERT_EQ(0, ctx.wait()); +} + +TEST_F(TestMockTrashMoveRequest, TrashAddError) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + MockExclusiveLock mock_exclusive_lock; + if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) { + mock_image_ctx.exclusive_lock = &mock_exclusive_lock; + } + + expect_op_work_queue(mock_image_ctx); + + InSequence seq; + utime_t delete_time{ceph_clock_now()}; + expect_trash_add(mock_image_ctx, "image id", + cls::rbd::TRASH_IMAGE_SOURCE_USER, "image name", delete_time, + -EPERM); + + C_SaferCond ctx; + cls::rbd::TrashImageSpec trash_image_spec{ + cls::rbd::TRASH_IMAGE_SOURCE_USER, "image name", delete_time, + delete_time}; + auto req = MockMoveRequest::create(mock_image_ctx.md_ctx, "image id", + trash_image_spec, &ctx); + req->send(); + ASSERT_EQ(-EPERM, ctx.wait()); +} + +TEST_F(TestMockTrashMoveRequest, RemoveIdError) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + MockExclusiveLock mock_exclusive_lock; + if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) { + mock_image_ctx.exclusive_lock = &mock_exclusive_lock; + } + + expect_op_work_queue(mock_image_ctx); + + InSequence seq; + utime_t delete_time{ceph_clock_now()}; + expect_trash_add(mock_image_ctx, "image id", + cls::rbd::TRASH_IMAGE_SOURCE_USER, "image name", delete_time, + 0); + expect_aio_remove(mock_image_ctx, util::id_obj_name("image name"), -EPERM); + + C_SaferCond ctx; + cls::rbd::TrashImageSpec trash_image_spec{ + cls::rbd::TRASH_IMAGE_SOURCE_USER, "image name", delete_time, + delete_time}; + auto req = MockMoveRequest::create(mock_image_ctx.md_ctx, "image id", + trash_image_spec, &ctx); + req->send(); + ASSERT_EQ(-EPERM, ctx.wait()); +} + +TEST_F(TestMockTrashMoveRequest, DirectoryRemoveError) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockTestImageCtx mock_image_ctx(*ictx); + MockExclusiveLock mock_exclusive_lock; + if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) { + mock_image_ctx.exclusive_lock = &mock_exclusive_lock; + } + + expect_op_work_queue(mock_image_ctx); + + InSequence seq; + utime_t delete_time{ceph_clock_now()}; + expect_trash_add(mock_image_ctx, "image id", + cls::rbd::TRASH_IMAGE_SOURCE_USER, "image name", delete_time, + 0); + expect_aio_remove(mock_image_ctx, util::id_obj_name("image name"), 0); + expect_dir_remove(mock_image_ctx, "image name", "image id", -EPERM); + + C_SaferCond ctx; + cls::rbd::TrashImageSpec trash_image_spec{ + cls::rbd::TRASH_IMAGE_SOURCE_USER, "image name", delete_time, + delete_time}; + auto req = MockMoveRequest::create(mock_image_ctx.md_ctx, "image id", + trash_image_spec, &ctx); + req->send(); + ASSERT_EQ(-EPERM, ctx.wait()); +} + +} // namespace trash +} // namespace librbd -- 2.39.5