From e22db4ec76819a4adec3694cbb087e278faa5ab0 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Mon, 29 Jan 2018 22:15:11 -0500 Subject: [PATCH] librbd: snapshot remove request now handles clone v2 The state machine was converted to the current style and the final removal of the snapshot was delayed until the end to permit the retry of a previously failed snapshot removal. Signed-off-by: Jason Dillaman --- src/librbd/operation/SnapshotRemoveRequest.cc | 354 ++++++++++------ src/librbd/operation/SnapshotRemoveRequest.h | 84 ++-- .../test_mock_SnapshotRemoveRequest.cc | 393 ++++++++++++++++-- 3 files changed, 640 insertions(+), 191 deletions(-) diff --git a/src/librbd/operation/SnapshotRemoveRequest.cc b/src/librbd/operation/SnapshotRemoveRequest.cc index 07e32b9a703..4622edfd059 100644 --- a/src/librbd/operation/SnapshotRemoveRequest.cc +++ b/src/librbd/operation/SnapshotRemoveRequest.cc @@ -4,131 +4,162 @@ #include "librbd/operation/SnapshotRemoveRequest.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/ObjectMap.h" +#include "librbd/Utils.h" +#include "librbd/image/DetachChildRequest.h" #define dout_subsys ceph_subsys_rbd #undef dout_prefix -#define dout_prefix *_dout << "librbd::SnapshotRemoveRequest: " +#define dout_prefix *_dout << "librbd::SnapshotRemoveRequest: " << this << " " \ + << __func__ << ": " namespace librbd { namespace operation { -namespace { - -template -std::ostream& operator<<(std::ostream& os, - const typename SnapshotRemoveRequest::State& state) { - switch(state) { - case SnapshotRemoveRequest::STATE_REMOVE_OBJECT_MAP: - os << "REMOVE_OBJECT_MAP"; - break; - case SnapshotRemoveRequest::STATE_REMOVE_CHILD: - os << "REMOVE_CHILD"; - break; - case SnapshotRemoveRequest::STATE_REMOVE_SNAP: - os << "REMOVE_SNAP"; - break; - case SnapshotRemoveRequest::STATE_RELEASE_SNAP_ID: - os << "RELEASE_SNAP_ID"; - break; - case SnapshotRemoveRequest::STATE_ERROR: - os << "STATE_ERROR"; - break; - default: - os << "UNKNOWN (" << static_cast(state) << ")"; - break; - } - return os; -} - -} // anonymous namespace - -template -SnapshotRemoveRequest::SnapshotRemoveRequest(I &image_ctx, - Context *on_finish, - const cls::rbd::SnapshotNamespace &snap_namespace, - const std::string &snap_name, - uint64_t snap_id) +using util::create_context_callback; +using util::create_rados_callback; + +template +SnapshotRemoveRequest::SnapshotRemoveRequest( + I &image_ctx, Context *on_finish, + const cls::rbd::SnapshotNamespace &snap_namespace, + const std::string &snap_name, uint64_t snap_id) : Request(image_ctx, on_finish), m_snap_namespace(snap_namespace), - m_snap_name(snap_name), m_snap_id(snap_id), m_state(STATE_REMOVE_OBJECT_MAP) { + m_snap_name(snap_name), m_snap_id(snap_id) { } template void SnapshotRemoveRequest::send_op() { - send_remove_object_map(); + I &image_ctx = this->m_image_ctx; + CephContext *cct = image_ctx.cct; + + assert(image_ctx.owner_lock.is_locked()); + { + RWLock::RLocker snap_locker(image_ctx.snap_lock); + RWLock::RLocker object_map_locker(image_ctx.object_map_lock); + if (image_ctx.snap_info.find(m_snap_id) == image_ctx.snap_info.end()) { + lderr(cct) << "snapshot doesn't exist" << dendl; + this->async_complete(-ENOENT); + return; + } + } + + trash_snap(); } template bool SnapshotRemoveRequest::should_complete(int r) { I &image_ctx = this->m_image_ctx; CephContext *cct = image_ctx.cct; - ldout(cct, 5) << this << " " << __func__ << ": state=" << m_state << ", " - << "r=" << r << dendl; - r = filter_state_return_code(r); + ldout(cct, 5) << "r=" << r << dendl; if (r < 0) { - return true; + lderr(cct) << "encountered error: " << cpp_strerror(r) << dendl; } + return true; +} - RWLock::RLocker owner_lock(image_ctx.owner_lock); - bool finished = false; - switch (m_state) { - case STATE_REMOVE_OBJECT_MAP: - send_remove_child(); - break; - case STATE_REMOVE_CHILD: - send_remove_snap(); - break; - case STATE_REMOVE_SNAP: - remove_snap_context(); - send_release_snap_id(); - break; - case STATE_RELEASE_SNAP_ID: - finished = true; - break; - default: - ceph_abort(); - break; +template +void SnapshotRemoveRequest::trash_snap() { + I &image_ctx = this->m_image_ctx; + if (image_ctx.old_format) { + release_snap_id(); + return; + } else if (cls::rbd::get_snap_namespace_type(m_snap_namespace) == + cls::rbd::SNAPSHOT_NAMESPACE_TYPE_TRASH) { + get_snap(); + return; } - return finished; + CephContext *cct = image_ctx.cct; + ldout(cct, 5) << dendl; + + librados::ObjectWriteOperation op; + cls_client::snapshot_trash_add(&op, m_snap_id); + + auto aio_comp = create_rados_callback< + SnapshotRemoveRequest, + &SnapshotRemoveRequest::handle_trash_snap>(this); + int r = image_ctx.md_ctx.aio_operate(image_ctx.header_oid, aio_comp, &op); + assert(r == 0); + aio_comp->release(); } template -void SnapshotRemoveRequest::send_remove_object_map() { +void SnapshotRemoveRequest::handle_trash_snap(int r) { I &image_ctx = this->m_image_ctx; - assert(image_ctx.owner_lock.is_locked()); + CephContext *cct = image_ctx.cct; + ldout(cct, 5) << "r=" << r << dendl; + + if (r == -EOPNOTSUPP) { + // trash / clone v2 not supported + detach_child(); + return; + } else if (r < 0) { + lderr(cct) << "failed to move snapshot to trash: " << cpp_strerror(r) + << dendl; + this->complete(r); + return; + } - { - CephContext *cct = image_ctx.cct; - RWLock::WLocker snap_locker(image_ctx.snap_lock); - RWLock::RLocker object_map_locker(image_ctx.object_map_lock); - if (image_ctx.snap_info.find(m_snap_id) == image_ctx.snap_info.end()) { - lderr(cct) << this << " " << __func__ << ": snapshot doesn't exist" - << dendl; - m_state = STATE_ERROR; - this->async_complete(-ENOENT); - return; - } + m_trashed_snapshot = true; + get_snap(); +} - if (image_ctx.object_map != nullptr) { - ldout(cct, 5) << this << " " << __func__ << dendl; +template +void SnapshotRemoveRequest::get_snap() { + I &image_ctx = this->m_image_ctx; + CephContext *cct = image_ctx.cct; + ldout(cct, 5) << dendl; - image_ctx.object_map->snapshot_remove( - m_snap_id, this->create_callback_context()); - return; + librados::ObjectReadOperation op; + cls_client::snapshot_get_start(&op, {m_snap_id}); + + auto aio_comp = create_rados_callback< + SnapshotRemoveRequest, + &SnapshotRemoveRequest::handle_get_snap>(this); + int r = image_ctx.md_ctx.aio_operate(image_ctx.header_oid, aio_comp, &op, + &m_out_bl); + assert(r == 0); + aio_comp->release(); +} + +template +void SnapshotRemoveRequest::handle_get_snap(int r) { + I &image_ctx = this->m_image_ctx; + CephContext *cct = image_ctx.cct; + ldout(cct, 5) << "r=" << r << dendl; + + if (r == 0) { + std::vector snap_infos; + std::vector parents; + std::vector protections; + bufferlist::iterator it = m_out_bl.begin(); + r = cls_client::snapshot_get_finish(&it, {m_snap_id}, &snap_infos, + &parents, &protections); + if (r == 0) { + m_child_attached = (snap_infos[0].child_count > 0); } } - send_remove_child(); + + if (r < 0) { + lderr(cct) << "failed to retrieve snapshot: " << cpp_strerror(r) + << dendl; + this->complete(r); + return; + } + + detach_child(); } template -void SnapshotRemoveRequest::send_remove_child() { +void SnapshotRemoveRequest::detach_child() { I &image_ctx = this->m_image_ctx; - assert(image_ctx.owner_lock.is_locked()); - CephContext *cct = image_ctx.cct; + + bool detach_child = false; { RWLock::RLocker snap_locker(image_ctx.snap_lock); RWLock::RLocker parent_locker(image_ctx.parent_lock); @@ -141,7 +172,6 @@ void SnapshotRemoveRequest::send_remove_child() { } else { lderr(cct) << "failed to retrieve parent spec" << dendl; } - m_state = STATE_ERROR; this->async_complete(r); return; @@ -150,32 +180,124 @@ void SnapshotRemoveRequest::send_remove_child() { if (image_ctx.parent_md.spec != our_pspec && (scan_for_parents(our_pspec) == -ENOENT)) { // no other references to the parent image - ldout(cct, 5) << this << " " << __func__ << dendl; - m_state = STATE_REMOVE_CHILD; + detach_child = true; + } + } - librados::ObjectWriteOperation op; - cls_client::remove_child(&op, our_pspec, image_ctx.id); + if (!detach_child) { + // HEAD image or other snapshots still associated with parent + remove_object_map(); + return; + } + + ldout(cct, 5) << dendl; + auto ctx = create_context_callback< + SnapshotRemoveRequest, + &SnapshotRemoveRequest::handle_detach_child>(this); + auto req = image::DetachChildRequest::create(image_ctx, ctx); + req->send(); +} + +template +void SnapshotRemoveRequest::handle_detach_child(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) { + lderr(cct) << "failed to detach child from parent: " << cpp_strerror(r) + << dendl; + this->complete(r); + return; + } + + remove_object_map(); +} + +template +void SnapshotRemoveRequest::remove_object_map() { + I &image_ctx = this->m_image_ctx; + if (m_child_attached) { + // if a clone v2 child is attached to this snapshot, we cannot + // proceed. It's only an error if the snap was already in the trash + this->complete(m_trashed_snapshot ? 0 : -EBUSY); + return; + } + + CephContext *cct = image_ctx.cct; + + { + RWLock::RLocker owner_lock(image_ctx.owner_lock); + RWLock::WLocker snap_locker(image_ctx.snap_lock); + RWLock::RLocker object_map_locker(image_ctx.object_map_lock); + if (image_ctx.object_map != nullptr) { + ldout(cct, 5) << dendl; - librados::AioCompletion *rados_completion = this->create_callback_completion(); - r = image_ctx.md_ctx.aio_operate(RBD_CHILDREN, rados_completion, &op); - assert(r == 0); - rados_completion->release(); + auto ctx = create_context_callback< + SnapshotRemoveRequest, + &SnapshotRemoveRequest::handle_remove_object_map>(this); + image_ctx.object_map->snapshot_remove(m_snap_id, ctx); return; } } - // HEAD image or other snapshots still associated with parent - send_remove_snap(); + // object map disabled + release_snap_id(); } template -void SnapshotRemoveRequest::send_remove_snap() { +void SnapshotRemoveRequest::handle_remove_object_map(int r) { I &image_ctx = this->m_image_ctx; - assert(image_ctx.owner_lock.is_locked()); + CephContext *cct = image_ctx.cct; + ldout(cct, 5) << "r=" << r << dendl; + if (r < 0) { + lderr(cct) << "failed to remove snapshot object map: " << cpp_strerror(r) + << dendl; + this->complete(r); + return; + } + + release_snap_id(); +} + +template +void SnapshotRemoveRequest::release_snap_id() { + I &image_ctx = this->m_image_ctx; + + CephContext *cct = image_ctx.cct; + ldout(cct, 5) << "snap_name=" << m_snap_name << ", " + << "snap_id=" << m_snap_id << dendl; + + + auto aio_comp = create_rados_callback< + SnapshotRemoveRequest, + &SnapshotRemoveRequest::handle_release_snap_id>(this); + image_ctx.data_ctx.aio_selfmanaged_snap_remove(m_snap_id, aio_comp); + aio_comp->release(); +} + +template +void SnapshotRemoveRequest::handle_release_snap_id(int r) { + I &image_ctx = this->m_image_ctx; CephContext *cct = image_ctx.cct; - ldout(cct, 5) << this << " " << __func__ << dendl; - m_state = STATE_REMOVE_SNAP; + ldout(cct, 5) << "r=" << r << dendl; + + if (r < 0 && r != -ENOENT) { + lderr(cct) << "failed to release snap id: " << cpp_strerror(r) << dendl; + this->complete(r); + return; + } + + remove_snap(); +} + +template +void SnapshotRemoveRequest::remove_snap() { + I &image_ctx = this->m_image_ctx; + + CephContext *cct = image_ctx.cct; + ldout(cct, 5) << dendl; librados::ObjectWriteOperation op; if (image_ctx.old_format) { @@ -184,35 +306,35 @@ void SnapshotRemoveRequest::send_remove_snap() { cls_client::snapshot_remove(&op, m_snap_id); } - librados::AioCompletion *rados_completion = this->create_callback_completion(); - int r = image_ctx.md_ctx.aio_operate(image_ctx.header_oid, - rados_completion, &op); + auto aio_comp = create_rados_callback< + SnapshotRemoveRequest, + &SnapshotRemoveRequest::handle_remove_snap>(this); + int r = image_ctx.md_ctx.aio_operate(image_ctx.header_oid, aio_comp, &op); assert(r == 0); - rados_completion->release(); + aio_comp->release(); } template -void SnapshotRemoveRequest::send_release_snap_id() { +void SnapshotRemoveRequest::handle_remove_snap(int r) { I &image_ctx = this->m_image_ctx; - assert(image_ctx.owner_lock.is_locked()); - CephContext *cct = image_ctx.cct; - ldout(cct, 5) << this << " " << __func__ << ": " - << "snap_name=" << m_snap_name << ", " - << "snap_id=" << m_snap_id << dendl; - m_state = STATE_RELEASE_SNAP_ID; + ldout(cct, 5) << "r=" << r << dendl; + + if (r < 0) { + lderr(cct) << "failed to remove snapshot: " << cpp_strerror(r) << dendl; + this->complete(r); + return; + } - librados::AioCompletion *rados_completion = - this->create_callback_completion(); - image_ctx.data_ctx.aio_selfmanaged_snap_remove(m_snap_id, rados_completion); - rados_completion->release(); + remove_snap_context(); + this->complete(0); } template void SnapshotRemoveRequest::remove_snap_context() { I &image_ctx = this->m_image_ctx; CephContext *cct = image_ctx.cct; - ldout(cct, 5) << this << " " << __func__ << dendl; + ldout(cct, 5) << dendl; RWLock::WLocker snap_locker(image_ctx.snap_lock); image_ctx.rm_snap(m_snap_namespace, m_snap_name, m_snap_id); diff --git a/src/librbd/operation/SnapshotRemoveRequest.h b/src/librbd/operation/SnapshotRemoveRequest.h index 71fb8441228..dfb4399f465 100644 --- a/src/librbd/operation/SnapshotRemoveRequest.h +++ b/src/librbd/operation/SnapshotRemoveRequest.h @@ -5,6 +5,7 @@ #define CEPH_LIBRBD_OPERATION_SNAPSHOT_REMOVE_REQUEST_H #include "librbd/operation/Request.h" +#include "include/buffer.h" #include "librbd/Types.h" #include @@ -20,41 +21,33 @@ template class SnapshotRemoveRequest : public Request { public: /** - * Snap Remove goes through the following state machine: - * * @verbatim * - * ------\ - * . | - * . v - * . STATE_REMOVE_OBJECT_MAP - * . | . - * . v . - * . . > STATE_REMOVE_CHILD . - * . | . - * . | . . . . - * . | . - * . v v - * . . > STATE_REMOVE_SNAP - * | - * v - * STATE_RELEASE_SNAP_ID - * | - * v - * + * + * | + * v + * TRASH_SNAP + * | + * v (skip if unsupported) + * GET_SNAP + * | + * v (skip if unnecessary) + * DETACH_CHILD + * | + * v (skip if disabled/in-use) + * REMOVE_OBJECT_MAP + * | + * v (skip if in-use) + * RELEASE_SNAP_ID + * | + * v (skip if in-use) + * REMOVE_SNAP + * | + * v + * * * @endverbatim - * - * The _REMOVE_OBJECT_MAP state is skipped if the object map is not enabled. - * The _REMOVE_CHILD state is skipped if the parent is still in-use. */ - enum State { - STATE_REMOVE_OBJECT_MAP, - STATE_REMOVE_CHILD, - STATE_REMOVE_SNAP, - STATE_RELEASE_SNAP_ID, - STATE_ERROR - }; static SnapshotRemoveRequest *create( ImageCtxT &image_ctx, const cls::rbd::SnapshotNamespace &snap_namespace, @@ -80,19 +73,28 @@ private: cls::rbd::SnapshotNamespace m_snap_namespace; std::string m_snap_name; uint64_t m_snap_id; - State m_state; + bool m_trashed_snapshot = false; + bool m_child_attached = false; - int filter_state_return_code(int r) const { - if (m_state == STATE_REMOVE_CHILD && r == -ENOENT) { - return 0; - } - return r; - } + ceph::bufferlist m_out_bl; + + void trash_snap(); + void handle_trash_snap(int r); + + void get_snap(); + void handle_get_snap(int r); + + void detach_child(); + void handle_detach_child(int r); + + void remove_object_map(); + void handle_remove_object_map(int r); + + void release_snap_id(); + void handle_release_snap_id(int r); - void send_remove_object_map(); - void send_remove_child(); - void send_remove_snap(); - void send_release_snap_id(); + void remove_snap(); + void handle_remove_snap(int r); void remove_snap_context(); int scan_for_parents(ParentSpec &pspec); diff --git a/src/test/librbd/operation/test_mock_SnapshotRemoveRequest.cc b/src/test/librbd/operation/test_mock_SnapshotRemoveRequest.cc index d16e0a4d2dc..4f86b93fec7 100644 --- a/src/test/librbd/operation/test_mock_SnapshotRemoveRequest.cc +++ b/src/test/librbd/operation/test_mock_SnapshotRemoveRequest.cc @@ -8,6 +8,8 @@ #include "common/bit_vector.hpp" #include "librbd/ImageState.h" #include "librbd/internal.h" +#include "librbd/Operations.h" +#include "librbd/image/DetachChildRequest.h" #include "librbd/operation/SnapshotRemoveRequest.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -16,11 +18,38 @@ #include "librbd/operation/SnapshotRemoveRequest.cc" namespace librbd { +namespace image { + +template <> +class DetachChildRequest { +public: + static DetachChildRequest *s_instance; + static DetachChildRequest *create(MockImageCtx &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 { using ::testing::_; using ::testing::DoAll; using ::testing::DoDefault; +using ::testing::Invoke; using ::testing::Return; using ::testing::SetArgPointee; using ::testing::StrEq; @@ -29,6 +58,7 @@ using ::testing::WithArg; class TestMockOperationSnapshotRemoveRequest : public TestMockFixture { public: typedef SnapshotRemoveRequest MockSnapshotRemoveRequest; + typedef image::DetachChildRequest MockDetachChildRequest; int create_snapshot(const char *snap_name) { librbd::ImageCtx *ictx; @@ -50,6 +80,48 @@ public: return 0; } + void expect_snapshot_trash_add(MockImageCtx &mock_image_ctx, int r) { + if (mock_image_ctx.old_format) { + return; + } + + auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), + exec(mock_image_ctx.header_oid, _, StrEq("rbd"), + StrEq("snapshot_trash_add"), + _, _, _)); + if (r < 0) { + expect.WillOnce(Return(r)); + } else { + expect.WillOnce(DoDefault()); + } + } + + void expect_snapshot_get(MockImageCtx &mock_image_ctx, + const cls::rbd::SnapshotInfo& snap_info, int r) { + if (mock_image_ctx.old_format) { + return; + } + + using ceph::encode; + EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), + exec(mock_image_ctx.header_oid, _, StrEq("rbd"), + StrEq("snapshot_get"), _, _, _)) + .WillOnce(WithArg<5>(Invoke([snap_info, r](bufferlist* bl) { + encode(snap_info, *bl); + return r; + }))); + if (r >= 0) { + EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), + exec(mock_image_ctx.header_oid, _, StrEq("rbd"), + StrEq("get_parent"), _, _, _)) + .WillOnce(DoDefault()); + EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), + exec(mock_image_ctx.header_oid, _, StrEq("rbd"), + StrEq("get_protection_status"), _, _, _)) + .WillOnce(DoDefault()); + } + } + void expect_object_map_snap_remove(MockImageCtx &mock_image_ctx, int r) { if (mock_image_ctx.object_map != nullptr) { EXPECT_CALL(*mock_image_ctx.object_map, snapshot_remove(_, _)) @@ -59,6 +131,10 @@ public: } void expect_get_parent_spec(MockImageCtx &mock_image_ctx, int r) { + if (mock_image_ctx.old_format) { + return; + } + auto &expect = EXPECT_CALL(mock_image_ctx, get_parent_spec(_, _)); if (r < 0) { expect.WillOnce(Return(r)); @@ -69,27 +145,10 @@ public: } } - void expect_remove_child(MockImageCtx &mock_image_ctx, int r) { - bool deep_flatten = mock_image_ctx.image_ctx->test_features(RBD_FEATURE_DEEP_FLATTEN); - auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), - exec(RBD_CHILDREN, _, StrEq("rbd"), StrEq("remove_child"), _, - _, _)); - if (deep_flatten) { - expect.Times(0); - } else { - expect.WillOnce(Return(r)); - } - } - - void expect_verify_lock_ownership(MockImageCtx &mock_image_ctx) { - if (mock_image_ctx.old_format) { - return; - } - - if (mock_image_ctx.exclusive_lock != nullptr) { - EXPECT_CALL(*mock_image_ctx.exclusive_lock, is_lock_owner()) - .WillRepeatedly(Return(false)); - } + void expect_detach_child(MockImageCtx &mock_image_ctx, + MockDetachChildRequest& mock_request, int r) { + EXPECT_CALL(mock_request, send()) + .WillOnce(FinishRequest(&mock_request, r, &mock_image_ctx)); } void expect_snap_remove(MockImageCtx &mock_image_ctx, int r) { @@ -138,13 +197,108 @@ TEST_F(TestMockOperationSnapshotRemoveRequest, Success) { expect_op_work_queue(mock_image_ctx); ::testing::InSequence seq; + expect_snapshot_trash_add(mock_image_ctx, 0); + uint64_t snap_id = ictx->snap_info.rbegin()->first; - expect_object_map_snap_remove(mock_image_ctx, 0); + expect_snapshot_get(mock_image_ctx, + {snap_id, {cls::rbd::UserSnapshotNamespace{}}, + "snap1", 123, {}, 0}, 0); + expect_get_parent_spec(mock_image_ctx, 0); - expect_verify_lock_ownership(mock_image_ctx); + expect_object_map_snap_remove(mock_image_ctx, 0); + expect_release_snap_id(mock_image_ctx); expect_snap_remove(mock_image_ctx, 0); expect_rm_snap(mock_image_ctx); + + C_SaferCond cond_ctx; + MockSnapshotRemoveRequest *req = new MockSnapshotRemoveRequest( + mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), "snap1", + snap_id); + { + RWLock::RLocker owner_locker(mock_image_ctx.owner_lock); + req->send(); + } + ASSERT_EQ(0, cond_ctx.wait()); +} + +TEST_F(TestMockOperationSnapshotRemoveRequest, SuccessCloneParent) { + REQUIRE_FEATURE(RBD_FEATURE_LAYERING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, snap_create(*ictx, "snap1")); + ASSERT_EQ(0, ictx->state->refresh_if_required()); + + MockImageCtx mock_image_ctx(*ictx); + + MockExclusiveLock mock_exclusive_lock; + if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) { + mock_image_ctx.exclusive_lock = &mock_exclusive_lock; + } + + MockObjectMap mock_object_map; + if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) { + mock_image_ctx.object_map = &mock_object_map; + } + + expect_op_work_queue(mock_image_ctx); + + ::testing::InSequence seq; + expect_snapshot_trash_add(mock_image_ctx, 0); + + uint64_t snap_id = ictx->snap_info.rbegin()->first; + expect_snapshot_get(mock_image_ctx, + {snap_id, {cls::rbd::UserSnapshotNamespace{}}, + "snap1", 123, {}, 1}, 0); + + expect_get_parent_spec(mock_image_ctx, 0); + + C_SaferCond cond_ctx; + MockSnapshotRemoveRequest *req = new MockSnapshotRemoveRequest( + mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), "snap1", + snap_id); + { + RWLock::RLocker owner_locker(mock_image_ctx.owner_lock); + req->send(); + } + ASSERT_EQ(0, cond_ctx.wait()); +} + +TEST_F(TestMockOperationSnapshotRemoveRequest, SuccessTrash) { + REQUIRE_FEATURE(RBD_FEATURE_LAYERING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, snap_create(*ictx, "snap1")); + ASSERT_EQ(0, ictx->state->refresh_if_required()); + + MockImageCtx mock_image_ctx(*ictx); + + MockExclusiveLock mock_exclusive_lock; + if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) { + mock_image_ctx.exclusive_lock = &mock_exclusive_lock; + } + + MockObjectMap mock_object_map; + if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) { + mock_image_ctx.object_map = &mock_object_map; + } + + expect_op_work_queue(mock_image_ctx); + + ::testing::InSequence seq; + expect_snapshot_trash_add(mock_image_ctx, 0); + + uint64_t snap_id = ictx->snap_info.rbegin()->first; + expect_snapshot_get(mock_image_ctx, + {snap_id, {cls::rbd::TrashSnapshotNamespace{"snap1"}}, + "snap1", 123, {}, 0}, 0); + + expect_get_parent_spec(mock_image_ctx, 0); + expect_object_map_snap_remove(mock_image_ctx, 0); expect_release_snap_id(mock_image_ctx); + expect_snap_remove(mock_image_ctx, 0); + expect_rm_snap(mock_image_ctx); C_SaferCond cond_ctx; MockSnapshotRemoveRequest *req = new MockSnapshotRemoveRequest( @@ -159,6 +313,7 @@ TEST_F(TestMockOperationSnapshotRemoveRequest, Success) { TEST_F(TestMockOperationSnapshotRemoveRequest, FlattenedCloneRemovesChild) { REQUIRE_FEATURE(RBD_FEATURE_LAYERING); + REQUIRE(!is_feature_enabled(RBD_FEATURE_DEEP_FLATTEN)) ASSERT_EQ(0, create_snapshot("snap1")); @@ -191,14 +346,24 @@ TEST_F(TestMockOperationSnapshotRemoveRequest, FlattenedCloneRemovesChild) { expect_op_work_queue(mock_image_ctx); + ::testing::InSequence seq; + expect_snapshot_trash_add(mock_image_ctx, 0); + uint64_t snap_id = ictx->snap_info.rbegin()->first; - expect_object_map_snap_remove(mock_image_ctx, 0); + expect_snapshot_get(mock_image_ctx, + {snap_id, {cls::rbd::UserSnapshotNamespace{}}, + "snap1", 123, {}, 0}, 0); + expect_get_parent_spec(mock_image_ctx, 0); - expect_remove_child(mock_image_ctx, -ENOENT); - expect_verify_lock_ownership(mock_image_ctx); + + MockDetachChildRequest mock_detach_child_request; + expect_detach_child(mock_image_ctx, mock_detach_child_request, -ENOENT); + + expect_object_map_snap_remove(mock_image_ctx, 0); + + expect_release_snap_id(mock_image_ctx); expect_snap_remove(mock_image_ctx, 0); expect_rm_snap(mock_image_ctx); - expect_release_snap_id(mock_image_ctx); C_SaferCond cond_ctx; MockSnapshotRemoveRequest *req = new MockSnapshotRemoveRequest( @@ -211,8 +376,50 @@ TEST_F(TestMockOperationSnapshotRemoveRequest, FlattenedCloneRemovesChild) { ASSERT_EQ(0, cond_ctx.wait()); } -TEST_F(TestMockOperationSnapshotRemoveRequest, ObjectMapSnapRemoveError) { - REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP); +TEST_F(TestMockOperationSnapshotRemoveRequest, TrashCloneParent) { + REQUIRE_FEATURE(RBD_FEATURE_LAYERING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, ictx->operations->snap_create( + {cls::rbd::TrashSnapshotNamespace{}}, "snap1")); + ASSERT_EQ(0, ictx->state->refresh_if_required()); + + MockImageCtx mock_image_ctx(*ictx); + + MockExclusiveLock mock_exclusive_lock; + if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) { + mock_image_ctx.exclusive_lock = &mock_exclusive_lock; + } + + MockObjectMap mock_object_map; + if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) { + mock_image_ctx.object_map = &mock_object_map; + } + + expect_op_work_queue(mock_image_ctx); + + ::testing::InSequence seq; + + uint64_t snap_id = ictx->snap_info.rbegin()->first; + expect_snapshot_get(mock_image_ctx, + {snap_id, {cls::rbd::TrashSnapshotNamespace{}}, + "snap1", 123, {}, 1}, 0); + expect_get_parent_spec(mock_image_ctx, 0); + + C_SaferCond cond_ctx; + MockSnapshotRemoveRequest *req = new MockSnapshotRemoveRequest( + mock_image_ctx, &cond_ctx, cls::rbd::TrashSnapshotNamespace{}, "snap1", + snap_id); + { + RWLock::RLocker owner_locker(mock_image_ctx.owner_lock); + req->send(); + } + ASSERT_EQ(-EBUSY, cond_ctx.wait()); +} + +TEST_F(TestMockOperationSnapshotRemoveRequest, SnapshotTrashAddNotSupported) { + REQUIRE_FORMAT_V2(); librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); @@ -221,6 +428,11 @@ TEST_F(TestMockOperationSnapshotRemoveRequest, ObjectMapSnapRemoveError) { MockImageCtx mock_image_ctx(*ictx); + MockExclusiveLock mock_exclusive_lock; + if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) { + mock_image_ctx.exclusive_lock = &mock_exclusive_lock; + } + MockObjectMap mock_object_map; if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) { mock_image_ctx.object_map = &mock_object_map; @@ -229,7 +441,106 @@ TEST_F(TestMockOperationSnapshotRemoveRequest, ObjectMapSnapRemoveError) { expect_op_work_queue(mock_image_ctx); ::testing::InSequence seq; + expect_snapshot_trash_add(mock_image_ctx, -EOPNOTSUPP); + + uint64_t snap_id = ictx->snap_info.rbegin()->first; + expect_get_parent_spec(mock_image_ctx, 0); + expect_object_map_snap_remove(mock_image_ctx, 0); + expect_release_snap_id(mock_image_ctx); + expect_snap_remove(mock_image_ctx, 0); + expect_rm_snap(mock_image_ctx); + + C_SaferCond cond_ctx; + MockSnapshotRemoveRequest *req = new MockSnapshotRemoveRequest( + mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), "snap1", + snap_id); + { + RWLock::RLocker owner_locker(mock_image_ctx.owner_lock); + req->send(); + } + ASSERT_EQ(0, cond_ctx.wait()); +} + +TEST_F(TestMockOperationSnapshotRemoveRequest, SnapshotTrashAddError) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, snap_create(*ictx, "snap1")); + ASSERT_EQ(0, ictx->state->refresh_if_required()); + + MockImageCtx mock_image_ctx(*ictx); + expect_op_work_queue(mock_image_ctx); + + ::testing::InSequence seq; + uint64_t snap_id = ictx->snap_info.rbegin()->first; + expect_snapshot_trash_add(mock_image_ctx, -EINVAL); + + C_SaferCond cond_ctx; + MockSnapshotRemoveRequest *req = new MockSnapshotRemoveRequest( + mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), "snap1", + snap_id); + { + RWLock::RLocker owner_locker(mock_image_ctx.owner_lock); + req->send(); + } + ASSERT_EQ(-EINVAL, cond_ctx.wait()); +} + +TEST_F(TestMockOperationSnapshotRemoveRequest, SnapshotGetError) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, snap_create(*ictx, "snap1")); + ASSERT_EQ(0, ictx->state->refresh_if_required()); + + MockImageCtx mock_image_ctx(*ictx); + expect_op_work_queue(mock_image_ctx); + + ::testing::InSequence seq; + expect_snapshot_trash_add(mock_image_ctx, 0); + uint64_t snap_id = ictx->snap_info.rbegin()->first; + expect_snapshot_get(mock_image_ctx, + {snap_id, {cls::rbd::UserSnapshotNamespace{}}, + "snap1", 123, {}, 0}, -EOPNOTSUPP); + + C_SaferCond cond_ctx; + MockSnapshotRemoveRequest *req = new MockSnapshotRemoveRequest( + mock_image_ctx, &cond_ctx, cls::rbd::UserSnapshotNamespace(), "snap1", + snap_id); + { + RWLock::RLocker owner_locker(mock_image_ctx.owner_lock); + req->send(); + } + ASSERT_EQ(-EOPNOTSUPP, cond_ctx.wait()); +} + +TEST_F(TestMockOperationSnapshotRemoveRequest, ObjectMapSnapRemoveError) { + REQUIRE_FEATURE(RBD_FEATURE_OBJECT_MAP); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, snap_create(*ictx, "snap1")); + ASSERT_EQ(0, ictx->state->refresh_if_required()); + + MockImageCtx mock_image_ctx(*ictx); + MockObjectMap mock_object_map; + mock_image_ctx.object_map = &mock_object_map; + + expect_op_work_queue(mock_image_ctx); + + ::testing::InSequence seq; + expect_snapshot_trash_add(mock_image_ctx, 0); + + uint64_t snap_id = ictx->snap_info.rbegin()->first; + expect_snapshot_get(mock_image_ctx, + {snap_id, {cls::rbd::UserSnapshotNamespace{}}, + "snap1", 123, {}, 0}, 0); + + expect_get_parent_spec(mock_image_ctx, 0); + expect_object_map_snap_remove(mock_image_ctx, -EINVAL); C_SaferCond cond_ctx; @@ -244,6 +555,8 @@ TEST_F(TestMockOperationSnapshotRemoveRequest, ObjectMapSnapRemoveError) { } TEST_F(TestMockOperationSnapshotRemoveRequest, RemoveChildParentError) { + REQUIRE_FEATURE(RBD_FEATURE_LAYERING); + librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); ASSERT_EQ(0, snap_create(*ictx, "snap1")); @@ -259,8 +572,13 @@ TEST_F(TestMockOperationSnapshotRemoveRequest, RemoveChildParentError) { expect_op_work_queue(mock_image_ctx); ::testing::InSequence seq; + expect_snapshot_trash_add(mock_image_ctx, 0); + uint64_t snap_id = ictx->snap_info.rbegin()->first; - expect_object_map_snap_remove(mock_image_ctx, 0); + expect_snapshot_get(mock_image_ctx, + {snap_id, {cls::rbd::UserSnapshotNamespace{}}, + "snap1", 123, {}, 0}, 0); + expect_get_parent_spec(mock_image_ctx, -ENOENT); C_SaferCond cond_ctx; @@ -309,9 +627,10 @@ TEST_F(TestMockOperationSnapshotRemoveRequest, RemoveChildError) { expect_op_work_queue(mock_image_ctx); uint64_t snap_id = ictx->snap_info.rbegin()->first; - expect_object_map_snap_remove(mock_image_ctx, 0); expect_get_parent_spec(mock_image_ctx, 0); - expect_remove_child(mock_image_ctx, -EINVAL); + + MockDetachChildRequest mock_detach_child_request; + expect_detach_child(mock_image_ctx, mock_detach_child_request, -EINVAL); C_SaferCond cond_ctx; MockSnapshotRemoveRequest *req = new MockSnapshotRemoveRequest( @@ -345,10 +664,16 @@ TEST_F(TestMockOperationSnapshotRemoveRequest, RemoveSnapError) { expect_op_work_queue(mock_image_ctx); ::testing::InSequence seq; + expect_snapshot_trash_add(mock_image_ctx, 0); + uint64_t snap_id = ictx->snap_info.rbegin()->first; - expect_object_map_snap_remove(mock_image_ctx, 0); + expect_snapshot_get(mock_image_ctx, + {snap_id, {cls::rbd::UserSnapshotNamespace{}}, + "snap1", 123, {}, 0}, 0); + expect_get_parent_spec(mock_image_ctx, 0); - expect_verify_lock_ownership(mock_image_ctx); + expect_object_map_snap_remove(mock_image_ctx, 0); + expect_release_snap_id(mock_image_ctx); expect_snap_remove(mock_image_ctx, -ENOENT); C_SaferCond cond_ctx; -- 2.39.5