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: v14.2.8~20^2~69^2~3 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d6840dc3ce808da8acefeb34550bf225afa39e1b;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/Operations.cc (RWLock::RLocker vs std::shared_lock) src/librbd/api/Migration.cc (RWLock::RLocker vs std::shared_lock) --- diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc index 71d98c5b2e6..4db3f29a8b6 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 81264e5e76f..f73a782dd60 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -662,7 +662,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) { ceph_assert(r == 0); @@ -718,7 +719,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; @@ -744,7 +746,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; @@ -772,7 +775,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; @@ -794,7 +798,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; @@ -815,8 +820,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; @@ -837,7 +847,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; @@ -858,7 +869,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; @@ -879,7 +891,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; @@ -906,7 +919,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; @@ -926,7 +940,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") @@ -949,7 +964,8 @@ bool ImageWatcher::handle_payload(const MigratePayload &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; @@ -975,7 +991,8 @@ bool ImageWatcher::handle_payload(const SparsifyPayload &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; @@ -1002,7 +1019,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 642cd03a7fe..ffb8f5c86e9 100644 --- a/src/librbd/Operations.cc +++ b/src/librbd/Operations.cc @@ -123,7 +123,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; @@ -131,13 +132,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) { } @@ -199,7 +201,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; @@ -265,8 +267,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; @@ -277,8 +278,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(); } @@ -356,7 +356,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, @@ -434,7 +436,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, @@ -484,7 +487,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) { @@ -537,7 +541,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, @@ -634,7 +640,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, @@ -726,7 +734,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, @@ -802,7 +811,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; } @@ -904,8 +914,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), @@ -996,7 +1011,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, @@ -1095,7 +1112,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, @@ -1190,7 +1209,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, @@ -1271,7 +1292,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; } @@ -1375,7 +1397,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; } @@ -1385,7 +1408,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, @@ -1460,7 +1485,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; } @@ -1521,7 +1547,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; } @@ -1583,7 +1610,9 @@ int Operations::migrate(ProgressContext &prog_ctx) { } uint64_t request_id = ++m_async_request_seq; - r = invoke_async_request("migrate", false, + r = invoke_async_request("migrate", + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + false, boost::bind(&Operations::execute_migrate, this, boost::ref(prog_ctx), _1), boost::bind(&ImageWatcher::notify_migrate, @@ -1642,7 +1671,7 @@ template int Operations::sparsify(size_t sparse_size, ProgressContext &prog_ctx) { CephContext *cct = m_image_ctx.cct; ldout(cct, 20) << "sparsify" << dendl; - + if (sparse_size < 4096 || sparse_size > m_image_ctx.get_object_size() || (sparse_size & (sparse_size - 1)) != 0) { lderr(cct) << "sparse size should be power of two not less than 4096" @@ -1651,7 +1680,9 @@ int Operations::sparsify(size_t sparse_size, ProgressContext &prog_ctx) { } uint64_t request_id = ++m_async_request_seq; - int r = invoke_async_request("sparsify", false, + int r = invoke_async_request("sparsify", + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, + false, boost::bind(&Operations::execute_sparsify, this, sparse_size, boost::ref(prog_ctx), _1), @@ -1689,7 +1720,8 @@ void Operations::execute_sparsify(size_t sparse_size, } template -int Operations::prepare_image_update(bool request_lock) { +int Operations::prepare_image_update( + exclusive_lock::OperationRequestType request_type, bool request_lock) { ceph_assert(m_image_ctx.owner_lock.is_locked() && !m_image_ctx.owner_lock.is_wlocked()); if (m_image_ctx.image_watcher == nullptr) { @@ -1704,7 +1736,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); @@ -1741,12 +1773,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 5e1e1711e19..253c9f92673 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 @@ -107,13 +108,15 @@ public: void execute_sparsify(size_t sparse_size, ProgressContext &prog_ctx, 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/api/Migration.cc b/src/librbd/api/Migration.cc index 20c0c417386..04b510d5047 100644 --- a/src/librbd/api/Migration.cc +++ b/src/librbd/api/Migration.cc @@ -19,6 +19,7 @@ #include "librbd/api/Trash.h" #include "librbd/deep_copy/MetadataCopyRequest.h" #include "librbd/deep_copy/SnapshotCopyRequest.h" +#include "librbd/exclusive_lock/Policy.h" #include "librbd/image/AttachChildRequest.h" #include "librbd/image/AttachParentRequest.h" #include "librbd/image/CloneRequest.h" @@ -1259,7 +1260,8 @@ int Migration::create_dst_image() { { RWLock::RLocker owner_locker(dst_image_ctx->owner_lock); - r = dst_image_ctx->operations->prepare_image_update(true); + r = dst_image_ctx->operations->prepare_image_update( + exclusive_lock::OPERATION_REQUEST_TYPE_GENERAL, true); if (r < 0) { lderr(m_cct) << "cannot obtain exclusive lock" << dendl; return r; diff --git a/src/librbd/api/Trash.cc b/src/librbd/api/Trash.cc index 4fcc106b4e3..0664461e863 100644 --- a/src/librbd/api/Trash.cc +++ b/src/librbd/api/Trash.cc @@ -15,6 +15,7 @@ #include "librbd/TrashWatcher.h" #include "librbd/Utils.h" #include "librbd/api/DiffIterate.h" +#include "librbd/exclusive_lock/Policy.h" #include "librbd/image/RemoveRequest.h" #include "librbd/mirror/DisableRequest.h" #include "librbd/mirror/EnableRequest.h" @@ -151,7 +152,8 @@ int Trash::move(librados::IoCtx &io_ctx, rbd_trash_image_source_t source, 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/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/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 b867fcfc4b7..dbdb12e74a9 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); }