From: Mykola Golub Date: Wed, 30 Oct 2019 13:30:35 +0000 (+0200) Subject: librbd: introduce a way to allow proxied trash snap remove X-Git-Tag: v13.2.9~96^2~3 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=3ccf71a3fd6a9a441a0994b44925aac9ed512f48;p=ceph.git librbd: introduce a way to allow proxied trash snap remove (while other operations are still blocked) Signed-off-by: Mykola Golub (cherry picked from commit 42dd83a20538e25cbf55ea9a1f235e9e0d7f5102) Conflicts: src/librbd/ExclusiveLock.cc (Mutex::Locker vs std::lock_guard) src/librbd/ImageWatcher.cc (no migrate and sparsify in mimic) src/librbd/Operations.cc (RWLock::RLocker vs std::shared_lock, assert vs ceph_assert, no migrate and sparsify) src/librbd/Operations.h (no migrate and sparsify) src/librbd/api/Migration.cc (does not exist) src/librbd/api/Trash.cc (does not exist, update internal.cc instead) --- diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc index a70e77e917c..151193a335f 100644 --- a/src/librbd/ExclusiveLock.cc +++ b/src/librbd/ExclusiveLock.cc @@ -37,18 +37,22 @@ ExclusiveLock::ExclusiveLock(I &image_ctx) } template -bool ExclusiveLock::accept_requests(int *ret_val) const { +bool ExclusiveLock::accept_request(OperationRequestType request_type, + int *ret_val) const { Mutex::Locker locker(ML::m_lock); - bool accept_requests = (!ML::is_state_shutdown() && - ML::is_state_locked() && - m_request_blocked_count == 0); + bool accept_request = + (!ML::is_state_shutdown() && ML::is_state_locked() && + (m_request_blocked_count == 0 || + m_image_ctx.get_exclusive_lock_policy()->accept_blocked_request( + request_type))); if (ret_val != nullptr) { - *ret_val = m_request_blocked_ret_val; + *ret_val = accept_request ? 0 : m_request_blocked_ret_val; } - ldout(m_image_ctx.cct, 20) << "=" << accept_requests << dendl; - return accept_requests; + ldout(m_image_ctx.cct, 20) << "=" << accept_request << " (request_type=" + << request_type << ")" << dendl; + return accept_request; } template @@ -74,7 +78,7 @@ void ExclusiveLock::block_requests(int r) { m_request_blocked_ret_val = r; } - ldout(m_image_ctx.cct, 20) << dendl; + ldout(m_image_ctx.cct, 20) << "r=" << r << dendl; } template diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h index 8b2a411f442..824cb00d0b0 100644 --- a/src/librbd/ExclusiveLock.h +++ b/src/librbd/ExclusiveLock.h @@ -4,8 +4,9 @@ #ifndef CEPH_LIBRBD_EXCLUSIVE_LOCK_H #define CEPH_LIBRBD_EXCLUSIVE_LOCK_H -#include "librbd/ManagedLock.h" #include "common/AsyncOpTracker.h" +#include "librbd/ManagedLock.h" +#include "librbd/exclusive_lock/Policy.h" namespace librbd { @@ -18,7 +19,8 @@ public: ExclusiveLock(ImageCtxT &image_ctx); - bool accept_requests(int *ret_val = nullptr) const; + bool accept_request(exclusive_lock::OperationRequestType request_type, + int *ret_val) const; bool accept_ops() const; void block_requests(int r); diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc index f8c525b3c6a..1dfb55447d7 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -632,7 +632,8 @@ bool ImageWatcher::handle_payload(const RequestLockPayload &payload, if (m_image_ctx.exclusive_lock != nullptr && m_image_ctx.exclusive_lock->is_lock_owner()) { int r = 0; - bool accept_request = m_image_ctx.exclusive_lock->accept_requests(&r); + bool accept_request = m_image_ctx.exclusive_lock->accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &r); if (accept_request) { assert(r == 0); @@ -688,7 +689,8 @@ bool ImageWatcher::handle_payload(const FlattenPayload &payload, RWLock::RLocker l(m_image_ctx.owner_lock); if (m_image_ctx.exclusive_lock != nullptr) { int r; - if (m_image_ctx.exclusive_lock->accept_requests(&r)) { + if (m_image_ctx.exclusive_lock->accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &r)) { bool new_request; Context *ctx; ProgressContext *prog_ctx; @@ -714,7 +716,8 @@ bool ImageWatcher::handle_payload(const ResizePayload &payload, RWLock::RLocker l(m_image_ctx.owner_lock); if (m_image_ctx.exclusive_lock != nullptr) { int r; - if (m_image_ctx.exclusive_lock->accept_requests(&r)) { + if (m_image_ctx.exclusive_lock->accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &r)) { bool new_request; Context *ctx; ProgressContext *prog_ctx; @@ -742,7 +745,8 @@ bool ImageWatcher::handle_payload(const SnapCreatePayload &payload, RWLock::RLocker l(m_image_ctx.owner_lock); if (m_image_ctx.exclusive_lock != nullptr) { int r; - if (m_image_ctx.exclusive_lock->accept_requests(&r)) { + if (m_image_ctx.exclusive_lock->accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &r)) { ldout(m_image_ctx.cct, 10) << this << " remote snap_create request: " << payload.snap_name << dendl; @@ -764,7 +768,8 @@ bool ImageWatcher::handle_payload(const SnapRenamePayload &payload, RWLock::RLocker l(m_image_ctx.owner_lock); if (m_image_ctx.exclusive_lock != nullptr) { int r; - if (m_image_ctx.exclusive_lock->accept_requests(&r)) { + if (m_image_ctx.exclusive_lock->accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &r)) { ldout(m_image_ctx.cct, 10) << this << " remote snap_rename request: " << payload.snap_id << " to " << payload.snap_name << dendl; @@ -785,8 +790,13 @@ bool ImageWatcher::handle_payload(const SnapRemovePayload &payload, C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); if (m_image_ctx.exclusive_lock != nullptr) { + auto request_type = exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL; + if (cls::rbd::get_snap_namespace_type(payload.snap_namespace) == + cls::rbd::SNAPSHOT_NAMESPACE_TYPE_TRASH) { + request_type = exclusive_lock::OPERATION_REQUEST_TYPE_TRASH_SNAP_REMOVE; + } int r; - if (m_image_ctx.exclusive_lock->accept_requests(&r)) { + if (m_image_ctx.exclusive_lock->accept_request(request_type, &r)) { ldout(m_image_ctx.cct, 10) << this << " remote snap_remove request: " << payload.snap_name << dendl; @@ -807,7 +817,8 @@ bool ImageWatcher::handle_payload(const SnapProtectPayload& payload, RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_image_ctx.exclusive_lock != nullptr) { int r; - if (m_image_ctx.exclusive_lock->accept_requests(&r)) { + if (m_image_ctx.exclusive_lock->accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &r)) { ldout(m_image_ctx.cct, 10) << this << " remote snap_protect request: " << payload.snap_name << dendl; @@ -828,7 +839,8 @@ bool ImageWatcher::handle_payload(const SnapUnprotectPayload& payload, RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_image_ctx.exclusive_lock != nullptr) { int r; - if (m_image_ctx.exclusive_lock->accept_requests(&r)) { + if (m_image_ctx.exclusive_lock->accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &r)) { ldout(m_image_ctx.cct, 10) << this << " remote snap_unprotect request: " << payload.snap_name << dendl; @@ -849,7 +861,8 @@ bool ImageWatcher::handle_payload(const RebuildObjectMapPayload& payload, RWLock::RLocker l(m_image_ctx.owner_lock); if (m_image_ctx.exclusive_lock != nullptr) { int r; - if (m_image_ctx.exclusive_lock->accept_requests(&r)) { + if (m_image_ctx.exclusive_lock->accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &r)) { bool new_request; Context *ctx; ProgressContext *prog_ctx; @@ -876,7 +889,8 @@ bool ImageWatcher::handle_payload(const RenamePayload& payload, RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_image_ctx.exclusive_lock != nullptr) { int r; - if (m_image_ctx.exclusive_lock->accept_requests(&r)) { + if (m_image_ctx.exclusive_lock->accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &r)) { ldout(m_image_ctx.cct, 10) << this << " remote rename request: " << payload.image_name << dendl; @@ -896,7 +910,8 @@ bool ImageWatcher::handle_payload(const UpdateFeaturesPayload& payload, RWLock::RLocker owner_locker(m_image_ctx.owner_lock); if (m_image_ctx.exclusive_lock != nullptr) { int r; - if (m_image_ctx.exclusive_lock->accept_requests(&r)) { + if (m_image_ctx.exclusive_lock->accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &r)) { ldout(m_image_ctx.cct, 10) << this << " remote update_features request: " << payload.features << " " << (payload.enabled ? "enabled" : "disabled") @@ -918,7 +933,8 @@ bool ImageWatcher::handle_payload(const UnknownPayload &payload, RWLock::RLocker l(m_image_ctx.owner_lock); if (m_image_ctx.exclusive_lock != nullptr) { int r; - if (m_image_ctx.exclusive_lock->accept_requests(&r) || r < 0) { + if (m_image_ctx.exclusive_lock->accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &r) || r < 0) { encode(ResponseMessage(-EOPNOTSUPP), ack_ctx->out); } } diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc index 8c467b61f09..b268113a99d 100644 --- a/src/librbd/Operations.cc +++ b/src/librbd/Operations.cc @@ -120,7 +120,8 @@ struct C_InvokeAsyncRequest : public Context { */ I &image_ctx; - std::string request_type; + std::string name; + exclusive_lock::OperationRequestType request_type; bool permit_snapshot; boost::function local; boost::function remote; @@ -128,13 +129,14 @@ struct C_InvokeAsyncRequest : public Context { Context *on_finish; bool request_lock = false; - C_InvokeAsyncRequest(I &image_ctx, const std::string& request_type, + C_InvokeAsyncRequest(I &image_ctx, const std::string& name, + exclusive_lock::OperationRequestType request_type, bool permit_snapshot, const boost::function& local, const boost::function& remote, const std::set &filter_error_codes, Context *on_finish) - : image_ctx(image_ctx), request_type(request_type), + : image_ctx(image_ctx), name(name), request_type(request_type), permit_snapshot(permit_snapshot), local(local), remote(remote), filter_error_codes(filter_error_codes), on_finish(on_finish) { } @@ -196,7 +198,7 @@ struct C_InvokeAsyncRequest : public Context { } if (image_ctx.exclusive_lock->is_lock_owner() && - image_ctx.exclusive_lock->accept_requests()) { + image_ctx.exclusive_lock->accept_request(request_type, nullptr)) { send_local_request(); owner_lock.put_read(); return; @@ -261,8 +263,7 @@ struct C_InvokeAsyncRequest : public Context { ldout(cct, 20) << __func__ << ": r=" << r << dendl; if (r == -EOPNOTSUPP) { - ldout(cct, 5) << request_type << " not supported by current lock owner" - << dendl; + ldout(cct, 5) << name << " not supported by current lock owner" << dendl; request_lock = true; send_refresh_image(); return; @@ -273,8 +274,7 @@ struct C_InvokeAsyncRequest : public Context { return; } - ldout(cct, 5) << request_type << " timed out notifying lock owner" - << dendl; + ldout(cct, 5) << name << " timed out notifying lock owner" << dendl; send_refresh_image(); } @@ -352,7 +352,9 @@ int Operations::flatten(ProgressContext &prog_ctx) { } uint64_t request_id = ++m_async_request_seq; - r = invoke_async_request("flatten", false, + r = invoke_async_request("flatten", + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + false, boost::bind(&Operations::execute_flatten, this, boost::ref(prog_ctx), _1), boost::bind(&ImageWatcher::notify_flatten, @@ -430,7 +432,8 @@ int Operations::rebuild_object_map(ProgressContext &prog_ctx) { } uint64_t request_id = ++m_async_request_seq; - r = invoke_async_request("rebuild object map", true, + r = invoke_async_request("rebuild object map", + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, true, boost::bind(&Operations::execute_rebuild_object_map, this, boost::ref(prog_ctx), _1), boost::bind(&ImageWatcher::notify_rebuild_object_map, @@ -480,7 +483,8 @@ int Operations::check_object_map(ProgressContext &prog_ctx) { return r; } - r = invoke_async_request("check object map", true, + r = invoke_async_request("check object map", + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, true, boost::bind(&Operations::check_object_map, this, boost::ref(prog_ctx), _1), [this](Context *c) { @@ -533,7 +537,9 @@ int Operations::rename(const char *dstname) { } if (m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) { - r = invoke_async_request("rename", true, + r = invoke_async_request("rename", + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + true, boost::bind(&Operations::execute_rename, this, dstname, _1), boost::bind(&ImageWatcher::notify_rename, @@ -630,7 +636,9 @@ int Operations::resize(uint64_t size, bool allow_shrink, ProgressContext& pro } uint64_t request_id = ++m_async_request_seq; - r = invoke_async_request("resize", false, + r = invoke_async_request("resize", + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + false, boost::bind(&Operations::execute_resize, this, size, allow_shrink, boost::ref(prog_ctx), _1, 0), boost::bind(&ImageWatcher::notify_resize, @@ -722,7 +730,8 @@ void Operations::snap_create(const cls::rbd::SnapshotNamespace &snap_namespac m_image_ctx.snap_lock.put_read(); C_InvokeAsyncRequest *req = new C_InvokeAsyncRequest( - m_image_ctx, "snap_create", true, + m_image_ctx, "snap_create", exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + true, boost::bind(&Operations::execute_snap_create, this, snap_namespace, snap_name, _1, 0, false), boost::bind(&ImageWatcher::notify_snap_create, m_image_ctx.image_watcher, @@ -798,7 +807,8 @@ int Operations::snap_rollback(const cls::rbd::SnapshotNamespace& snap_namespa } } - r = prepare_image_update(false); + r = prepare_image_update(exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + false); if (r < 0) { return r; } @@ -900,8 +910,13 @@ void Operations::snap_remove(const cls::rbd::SnapshotNamespace& snap_namespac m_image_ctx.snap_lock.put_read(); if (proxy_op) { + auto request_type = exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL; + if (cls::rbd::get_snap_namespace_type(snap_namespace) == + cls::rbd::SNAPSHOT_NAMESPACE_TYPE_TRASH) { + request_type = exclusive_lock::OPERATION_REQUEST_TYPE_TRASH_SNAP_REMOVE; + } C_InvokeAsyncRequest *req = new C_InvokeAsyncRequest( - m_image_ctx, "snap_remove", true, + m_image_ctx, "snap_remove", request_type, true, boost::bind(&Operations::execute_snap_remove, this, snap_namespace, snap_name, _1), boost::bind(&ImageWatcher::notify_snap_remove, m_image_ctx.image_watcher, snap_namespace, snap_name, _1), @@ -992,7 +1007,9 @@ int Operations::snap_rename(const char *srcname, const char *dstname) { } if (m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) { - r = invoke_async_request("snap_rename", true, + r = invoke_async_request("snap_rename", + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + true, boost::bind(&Operations::execute_snap_rename, this, snap_id, dstname, _1), boost::bind(&ImageWatcher::notify_snap_rename, @@ -1091,7 +1108,9 @@ int Operations::snap_protect(const cls::rbd::SnapshotNamespace& snap_namespac } if (m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) { - r = invoke_async_request("snap_protect", true, + r = invoke_async_request("snap_protect", + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + true, boost::bind(&Operations::execute_snap_protect, this, snap_namespace, snap_name, _1), boost::bind(&ImageWatcher::notify_snap_protect, @@ -1186,7 +1205,9 @@ int Operations::snap_unprotect(const cls::rbd::SnapshotNamespace& snap_namesp } if (m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) { - r = invoke_async_request("snap_unprotect", true, + r = invoke_async_request("snap_unprotect", + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + true, boost::bind(&Operations::execute_snap_unprotect, this, snap_namespace, snap_name, _1), boost::bind(&ImageWatcher::notify_snap_unprotect, @@ -1267,7 +1288,8 @@ int Operations::snap_set_limit(uint64_t limit) { C_SaferCond limit_ctx; { RWLock::RLocker owner_lock(m_image_ctx.owner_lock); - r = prepare_image_update(true); + r = prepare_image_update(exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + true); if (r < 0) { return r; } @@ -1360,7 +1382,8 @@ int Operations::update_features(uint64_t features, bool enabled) { C_SaferCond cond_ctx; { RWLock::RLocker owner_lock(m_image_ctx.owner_lock); - r = prepare_image_update(true); + r = prepare_image_update(exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + true); if (r < 0) { return r; } @@ -1370,7 +1393,9 @@ int Operations::update_features(uint64_t features, bool enabled) { r = cond_ctx.wait(); } else { - r = invoke_async_request("update_features", false, + r = invoke_async_request("update_features", + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + false, boost::bind(&Operations::execute_update_features, this, features, enabled, _1, 0), boost::bind(&ImageWatcher::notify_update_features, @@ -1440,7 +1465,8 @@ int Operations::metadata_set(const std::string &key, C_SaferCond metadata_ctx; { RWLock::RLocker owner_lock(m_image_ctx.owner_lock); - r = prepare_image_update(true); + r = prepare_image_update(exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + true); if (r < 0) { return r; } @@ -1501,7 +1527,8 @@ int Operations::metadata_remove(const std::string &key) { C_SaferCond metadata_ctx; { RWLock::RLocker owner_lock(m_image_ctx.owner_lock); - r = prepare_image_update(true); + r = prepare_image_update(exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + true); if (r < 0) { return r; } @@ -1541,7 +1568,8 @@ void Operations::execute_metadata_remove(const std::string &key, } template -int Operations::prepare_image_update(bool request_lock) { +int Operations::prepare_image_update( + exclusive_lock::OperationRequestType request_type, bool request_lock) { assert(m_image_ctx.owner_lock.is_locked() && !m_image_ctx.owner_lock.is_wlocked()); if (m_image_ctx.image_watcher == nullptr) { @@ -1556,7 +1584,7 @@ int Operations::prepare_image_update(bool request_lock) { RWLock::WLocker owner_locker(m_image_ctx.owner_lock); if (m_image_ctx.exclusive_lock != nullptr && (!m_image_ctx.exclusive_lock->is_lock_owner() || - !m_image_ctx.exclusive_lock->accept_requests())) { + !m_image_ctx.exclusive_lock->accept_request(request_type, nullptr))) { attempting_lock = true; m_image_ctx.exclusive_lock->block_requests(0); @@ -1593,12 +1621,12 @@ int Operations::prepare_image_update(bool request_lock) { } template -int Operations::invoke_async_request(const std::string& request_type, - bool permit_snapshot, - const boost::function& local_request, - const boost::function& remote_request) { +int Operations::invoke_async_request( + const std::string& name, exclusive_lock::OperationRequestType request_type, + bool permit_snapshot, const boost::function& local_request, + const boost::function& remote_request) { C_SaferCond ctx; - C_InvokeAsyncRequest *req = new C_InvokeAsyncRequest(m_image_ctx, + C_InvokeAsyncRequest *req = new C_InvokeAsyncRequest(m_image_ctx, name, request_type, permit_snapshot, local_request, diff --git a/src/librbd/Operations.h b/src/librbd/Operations.h index ff1238ff50e..e4f2cb08492 100644 --- a/src/librbd/Operations.h +++ b/src/librbd/Operations.h @@ -6,6 +6,7 @@ #include "cls/rbd/cls_rbd_types.h" #include "include/int_types.h" +#include "librbd/exclusive_lock/Policy.h" #include "librbd/operation/ObjectMapIterate.h" #include #include @@ -100,13 +101,15 @@ public: int metadata_remove(const std::string &key); void execute_metadata_remove(const std::string &key, Context *on_finish); - int prepare_image_update(bool request_lock); + int prepare_image_update(exclusive_lock::OperationRequestType request_type, + bool request_lock); private: ImageCtxT &m_image_ctx; std::atomic m_async_request_seq; - int invoke_async_request(const std::string& request_type, + int invoke_async_request(const std::string& name, + exclusive_lock::OperationRequestType request_type, bool permit_snapshot, const boost::function& local, const boost::function& remote); diff --git a/src/librbd/exclusive_lock/Policy.h b/src/librbd/exclusive_lock/Policy.h index 72b2945f58e..8dcbf444441 100644 --- a/src/librbd/exclusive_lock/Policy.h +++ b/src/librbd/exclusive_lock/Policy.h @@ -7,12 +7,21 @@ namespace librbd { namespace exclusive_lock { +enum OperationRequestType { + OPERATION_REQUEST_TYPE_GENERAL = 0, + OPERATION_REQUEST_TYPE_TRASH_SNAP_REMOVE = 1, +}; + struct Policy { virtual ~Policy() { } virtual bool may_auto_request_lock() = 0; virtual int lock_requested(bool force) = 0; + + virtual bool accept_blocked_request(OperationRequestType) { + return false; + } }; } // namespace exclusive_lock diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index 3ae697d580d..3db3c4d4ab7 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -1489,7 +1489,8 @@ int enable_mirroring(IoCtx &io_ctx, const std::string &image_id) { if (ictx->exclusive_lock != nullptr) { ictx->exclusive_lock->block_requests(0); - r = ictx->operations->prepare_image_update(false); + r = ictx->operations->prepare_image_update( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, false); if (r < 0) { lderr(cct) << "cannot obtain exclusive lock - not removing" << dendl; ictx->owner_lock.put_read(); diff --git a/src/test/librbd/mock/MockExclusiveLock.h b/src/test/librbd/mock/MockExclusiveLock.h index 1b49fa6a75d..efb4fa4e33e 100644 --- a/src/test/librbd/mock/MockExclusiveLock.h +++ b/src/test/librbd/mock/MockExclusiveLock.h @@ -6,6 +6,7 @@ #include "include/int_types.h" #include "include/rados/librados.hpp" +#include "librbd/exclusive_lock/Policy.h" #include "gmock/gmock.h" class Context; @@ -27,7 +28,8 @@ struct MockExclusiveLock { MOCK_METHOD1(acquire_lock, void(Context *)); MOCK_METHOD1(release_lock, void(Context *)); - MOCK_METHOD0(accept_requests, bool()); + MOCK_METHOD2(accept_request, bool(exclusive_lock::OperationRequestType, + int *)); MOCK_METHOD0(accept_ops, bool()); MOCK_METHOD0(get_unlocked_op_error, int()); diff --git a/src/test/librbd/mock/exclusive_lock/MockPolicy.h b/src/test/librbd/mock/exclusive_lock/MockPolicy.h index d403568e365..f49eeb23f20 100644 --- a/src/test/librbd/mock/exclusive_lock/MockPolicy.h +++ b/src/test/librbd/mock/exclusive_lock/MockPolicy.h @@ -14,7 +14,7 @@ struct MockPolicy : public Policy { MOCK_METHOD0(may_auto_request_lock, bool()); MOCK_METHOD1(lock_requested, int(bool)); - + MOCK_METHOD1(accept_blocked_request, bool(OperationRequestType)); }; } // namespace exclusive_lock diff --git a/src/test/librbd/test_mock_ExclusiveLock.cc b/src/test/librbd/test_mock_ExclusiveLock.cc index 3aa26f092e6..ad8b273aed4 100644 --- a/src/test/librbd/test_mock_ExclusiveLock.cc +++ b/src/test/librbd/test_mock_ExclusiveLock.cc @@ -4,6 +4,7 @@ #include "test/librbd/test_mock_fixture.h" #include "test/librbd/test_support.h" #include "test/librbd/mock/MockImageCtx.h" +#include "test/librbd/mock/exclusive_lock/MockPolicy.h" #include "librbd/ExclusiveLock.h" #include "librbd/ManagedLock.h" #include "librbd/exclusive_lock/PreAcquireRequest.h" @@ -290,6 +291,16 @@ public: .WillOnce(CompleteContext(0, static_cast(nullptr))); } + void expect_accept_blocked_request( + MockExclusiveLockImageCtx &mock_image_ctx, + exclusive_lock::MockPolicy &policy, + exclusive_lock::OperationRequestType request_type, bool value) { + EXPECT_CALL(mock_image_ctx, get_exclusive_lock_policy()) + .WillOnce(Return(&policy)); + EXPECT_CALL(policy, accept_blocked_request(request_type)) + .WillOnce(Return(value)); + } + int when_init(MockExclusiveLockImageCtx &mock_image_ctx, MockExclusiveLock &exclusive_lock) { C_SaferCond ctx; @@ -663,6 +674,7 @@ TEST_F(TestMockExclusiveLock, BlockRequests) { MockExclusiveLockImageCtx mock_image_ctx(*ictx); MockExclusiveLock exclusive_lock(mock_image_ctx); + exclusive_lock::MockPolicy mock_exclusive_lock_policy; expect_op_work_queue(mock_image_ctx); @@ -675,19 +687,35 @@ TEST_F(TestMockExclusiveLock, BlockRequests) { int ret_val; expect_is_state_shutdown(exclusive_lock, false); expect_is_state_locked(exclusive_lock, true); - ASSERT_TRUE(exclusive_lock.accept_requests(&ret_val)); + ASSERT_TRUE(exclusive_lock.accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &ret_val)); ASSERT_EQ(0, ret_val); exclusive_lock.block_requests(-EROFS); expect_is_state_shutdown(exclusive_lock, false); expect_is_state_locked(exclusive_lock, true); - ASSERT_FALSE(exclusive_lock.accept_requests(&ret_val)); + expect_accept_blocked_request(mock_image_ctx, mock_exclusive_lock_policy, + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + false); + ASSERT_FALSE(exclusive_lock.accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &ret_val)); ASSERT_EQ(-EROFS, ret_val); + expect_is_state_shutdown(exclusive_lock, false); + expect_is_state_locked(exclusive_lock, true); + expect_accept_blocked_request( + mock_image_ctx, mock_exclusive_lock_policy, + exclusive_lock::OPERATION_REQUEST_TYPE_TRASH_SNAP_REMOVE, true); + ASSERT_TRUE(exclusive_lock.accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_TRASH_SNAP_REMOVE, + &ret_val)); + ASSERT_EQ(0, ret_val); + exclusive_lock.unblock_requests(); expect_is_state_shutdown(exclusive_lock, false); expect_is_state_locked(exclusive_lock, true); - ASSERT_TRUE(exclusive_lock.accept_requests(&ret_val)); + ASSERT_TRUE(exclusive_lock.accept_request( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, &ret_val)); ASSERT_EQ(0, ret_val); }