From: Mykola Golub Date: Fri, 13 Jan 2017 13:51:16 +0000 (+0100) Subject: librbd: managed_lock: make AcquireRequest use GetLockRequest and BreakRequest X-Git-Tag: v12.0.0~159^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=91b72f0a2d8edbbe94a82aafab6f759ac228db73;p=ceph-ci.git librbd: managed_lock: make AcquireRequest use GetLockRequest and BreakRequest Initially this was implemented for exclusive_lock (03533b9, 23f60fe) but was lost when merging managed_lock refactoring. Signed-off-by: Mykola Golub --- diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc index 06795bff255..2b1621ef60d 100644 --- a/src/librbd/ExclusiveLock.cc +++ b/src/librbd/ExclusiveLock.cc @@ -26,7 +26,8 @@ using ML = ManagedLock; template ExclusiveLock::ExclusiveLock(I &image_ctx) : ML(image_ctx.md_ctx, image_ctx.op_work_queue, image_ctx.header_oid, - image_ctx.image_watcher), + image_ctx.image_watcher, image_ctx.blacklist_on_break_lock, + image_ctx.blacklist_expire_seconds), m_image_ctx(image_ctx), m_pre_post_callback(nullptr), m_shutting_down(false) { ML::m_state = ML::STATE_UNINITIALIZED; diff --git a/src/librbd/ManagedLock.cc b/src/librbd/ManagedLock.cc index c5dead59617..ee1303cbea0 100644 --- a/src/librbd/ManagedLock.cc +++ b/src/librbd/ManagedLock.cc @@ -43,13 +43,17 @@ const std::string ManagedLock::WATCHER_LOCK_TAG("internal"); template ManagedLock::ManagedLock(librados::IoCtx &ioctx, ContextWQ *work_queue, - const string& oid, Watcher *watcher) + const string& oid, Watcher *watcher, + bool blacklist_on_break_lock, + uint32_t blacklist_expire_seconds) : m_lock(util::unique_lock_name("librbd::ManagedLock::m_lock", this)), m_state(STATE_UNLOCKED), m_ioctx(ioctx), m_cct(reinterpret_cast(ioctx.cct())), m_work_queue(work_queue), m_oid(oid), - m_watcher(watcher) { + m_watcher(watcher), + m_blacklist_on_break_lock(blacklist_on_break_lock), + m_blacklist_expire_seconds(blacklist_expire_seconds) { } template @@ -376,9 +380,10 @@ void ManagedLock::handle_pre_acquire_lock(int r) { } using managed_lock::AcquireRequest; - AcquireRequest* req = AcquireRequest::create(m_ioctx, m_watcher, - m_work_queue, m_oid, m_cookie, - util::create_context_callback< + AcquireRequest* req = AcquireRequest::create( + m_ioctx, m_watcher, m_work_queue, m_oid, m_cookie, + m_blacklist_on_break_lock, m_blacklist_expire_seconds, + util::create_context_callback< ManagedLock, &ManagedLock::handle_acquire_lock>(this)); m_work_queue->queue(new C_SendLockRequest>(req), 0); } diff --git a/src/librbd/ManagedLock.h b/src/librbd/ManagedLock.h index dccb8b7f057..41b439d9d60 100644 --- a/src/librbd/ManagedLock.h +++ b/src/librbd/ManagedLock.h @@ -30,12 +30,17 @@ public: static const std::string WATCHER_LOCK_TAG; static ManagedLock *create(librados::IoCtx& ioctx, ContextWQ *work_queue, - const std::string& oid, Watcher *watcher) { - return new ManagedLock(ioctx, work_queue, oid, watcher); + const std::string& oid, Watcher *watcher, + bool blacklist_on_break_lock, + uint32_t blacklist_expire_seconds) { + return new ManagedLock(ioctx, work_queue, oid, watcher, + blacklist_on_break_lock, blacklist_expire_seconds); } ManagedLock(librados::IoCtx& ioctx, ContextWQ *work_queue, - const std::string& oid, Watcher *watcher); + const std::string& oid, Watcher *watcher, + bool blacklist_on_break_lock, + uint32_t blacklist_expire_seconds); virtual ~ManagedLock(); bool is_lock_owner() const; @@ -151,6 +156,8 @@ private: ContextWQ *m_work_queue; std::string m_oid; Watcher *m_watcher; + bool m_blacklist_on_break_lock; + uint32_t m_blacklist_expire_seconds; std::string m_cookie; std::string m_new_cookie; diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index 8775cff9919..0f9912843f5 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -1571,7 +1571,7 @@ void filter_out_mirror_watchers(ImageCtx *ictx, managed_lock::Locker locker; C_SaferCond get_owner_ctx; auto get_owner_req = managed_lock::GetLockerRequest<>::create( - *ictx, &locker, &get_owner_ctx); + ictx->md_ctx, ictx->header_oid, &locker, &get_owner_ctx); get_owner_req->send(); int r = get_owner_ctx.wait(); @@ -1604,7 +1604,7 @@ void filter_out_mirror_watchers(ImageCtx *ictx, managed_lock::Locker locker; C_SaferCond get_owner_ctx; auto get_owner_req = managed_lock::GetLockerRequest<>::create( - *ictx, &locker, &get_owner_ctx); + ictx->md_ctx, ictx->header_oid, &locker, &get_owner_ctx); get_owner_req->send(); int r = get_owner_ctx.wait(); @@ -1622,7 +1622,9 @@ void filter_out_mirror_watchers(ImageCtx *ictx, C_SaferCond break_ctx; auto break_req = managed_lock::BreakRequest<>::create( - *ictx, locker, ictx->blacklist_on_break_lock, true, &break_ctx); + ictx->md_ctx, ictx->op_work_queue, ictx->header_oid, locker, + ictx->blacklist_on_break_lock, ictx->blacklist_expire_seconds, true, + &break_ctx); break_req->send(); r = break_ctx.wait(); diff --git a/src/librbd/managed_lock/AcquireRequest.cc b/src/librbd/managed_lock/AcquireRequest.cc index 5084180a517..23b0f044ee0 100644 --- a/src/librbd/managed_lock/AcquireRequest.cc +++ b/src/librbd/managed_lock/AcquireRequest.cc @@ -11,12 +11,15 @@ #include "common/WorkQueue.h" #include "include/stringify.h" #include "librbd/Utils.h" +#include "librbd/managed_lock/BreakRequest.h" +#include "librbd/managed_lock/GetLockerRequest.h" #include "librbd/ImageCtx.h" #define dout_subsys ceph_subsys_rbd #undef dout_prefix -#define dout_prefix *_dout << "librbd::managed_lock::AcquireRequest: " +#define dout_prefix *_dout << "librbd::managed_lock::AcquireRequest: " << this \ + << " " << __func__ << ": " using std::string; @@ -29,50 +32,33 @@ using util::create_rados_ack_callback; namespace managed_lock { -namespace { - -struct C_BlacklistClient : public Context { - librados::IoCtx& ioctx; - std::string locker_address; - Context *on_finish; - - C_BlacklistClient(librados::IoCtx& ioctx, const std::string &locker_address, - Context *on_finish) - : ioctx(ioctx), locker_address(locker_address), - on_finish(on_finish) { - } - - virtual void finish(int r) override { - librados::Rados rados(ioctx); - CephContext *cct = reinterpret_cast(ioctx.cct()); - r = rados.blacklist_add(locker_address, - cct->_conf->rbd_blacklist_expire_seconds); - on_finish->complete(r); - } -}; - -} // anonymous namespace - template AcquireRequest* AcquireRequest::create(librados::IoCtx& ioctx, Watcher *watcher, ContextWQ *work_queue, const string& oid, const string& cookie, + bool blacklist_on_break_lock, + uint32_t blacklist_expire_seconds, Context *on_finish) { return new AcquireRequest(ioctx, watcher, work_queue, oid, cookie, + blacklist_on_break_lock, blacklist_expire_seconds, on_finish); } template AcquireRequest::AcquireRequest(librados::IoCtx& ioctx, Watcher *watcher, ContextWQ *work_queue, const string& oid, - const string& cookie, Context *on_finish) + const string& cookie, + bool blacklist_on_break_lock, + uint32_t blacklist_expire_seconds, + Context *on_finish) : m_ioctx(ioctx), m_watcher(watcher), m_cct(reinterpret_cast(m_ioctx.cct())), m_work_queue(work_queue), m_oid(oid), m_cookie(cookie), - m_on_finish(new C_AsyncCallback(work_queue, on_finish)), - m_error_result(0) { + m_blacklist_on_break_lock(blacklist_on_break_lock), + m_blacklist_expire_seconds(blacklist_expire_seconds), + m_on_finish(new C_AsyncCallback(work_queue, on_finish)) { } template @@ -81,233 +67,107 @@ AcquireRequest::~AcquireRequest() { template void AcquireRequest::send() { - send_lock(); -} - -template -void AcquireRequest::send_lock() { - ldout(m_cct, 10) << __func__ << dendl; - - librados::ObjectWriteOperation op; - rados::cls::lock::lock(&op, RBD_LOCK_NAME, LOCK_EXCLUSIVE, m_cookie, - ManagedLock::WATCHER_LOCK_TAG, "", utime_t(), 0); - - using klass = AcquireRequest; - librados::AioCompletion *rados_completion = - create_rados_safe_callback(this); - int r = m_ioctx.aio_operate(m_oid, rados_completion, &op); - assert(r == 0); - rados_completion->release(); -} - -template -void AcquireRequest::handle_lock(int r) { - ldout(m_cct, 10) << __func__ << ": r=" << r << dendl; - - if (r == 0) { - finish(); - return; - } else if (r != -EBUSY) { - save_result(r); - lderr(m_cct) << "failed to lock: " << cpp_strerror(r) << dendl; - finish(); - return; - } - - send_get_lockers(); + send_get_locker(); } template -void AcquireRequest::send_get_lockers() { - ldout(m_cct, 10) << __func__ << dendl; +void AcquireRequest::send_get_locker() { + ldout(m_cct, 10) << dendl; - librados::ObjectReadOperation op; - rados::cls::lock::get_lock_info_start(&op, RBD_LOCK_NAME); - - using klass = AcquireRequest; - librados::AioCompletion *rados_completion = - create_rados_ack_callback(this); - m_out_bl.clear(); - int r = m_ioctx.aio_operate(m_oid, rados_completion, &op, &m_out_bl); - assert(r == 0); - rados_completion->release(); + Context *ctx = create_context_callback< + AcquireRequest, &AcquireRequest::handle_get_locker>(this); + auto req = GetLockerRequest::create(m_ioctx, m_oid, &m_locker, ctx); + req->send(); } template -void AcquireRequest::handle_get_lockers(int r) { - ldout(m_cct, 10) << __func__ << ": r=" << r << dendl; - - std::map lockers; - ClsLockType lock_type = LOCK_NONE; - std::string lock_tag; - - if (r == 0) { - bufferlist::iterator it = m_out_bl.begin(); - r = rados::cls::lock::get_lock_info_finish(&it, &lockers, - &lock_type, &lock_tag); - } +void AcquireRequest::handle_get_locker(int r) { + ldout(m_cct, 10) << "r=" << r << dendl; - save_result(r); - if (r < 0) { - lderr(m_cct) << "failed to retrieve lockers: " << cpp_strerror(r) << dendl; - finish(); - return; - } - - if (lockers.empty()) { + if (r == -ENOENT) { ldout(m_cct, 20) << "no lockers detected" << dendl; - send_lock(); + m_locker = {}; + } else if (r == -EBUSY) { + ldout(m_cct, 5) << "incompatible lock detected" << dendl; + finish(r); return; - } - - if (lock_tag != ManagedLock::WATCHER_LOCK_TAG) { - ldout(m_cct, 5) <<"locked by external mechanism: tag=" << lock_tag << dendl; - save_result(-EBUSY); - finish(); - return; - } - - if (lock_type == LOCK_SHARED) { - ldout(m_cct, 5) << "shared lock type detected" << dendl; - save_result(-EBUSY); - finish(); - return; - } - - std::map::iterator iter = lockers.begin(); - if (!ManagedLock::decode_lock_cookie(iter->first.cookie, &m_locker_handle)) { - ldout(m_cct, 5) << "locked by external mechanism: " - << "cookie=" << iter->first.cookie << dendl; - save_result(-EBUSY); - finish(); - return; - } - - m_locker_entity = iter->first.locker; - m_locker_cookie = iter->first.cookie; - m_locker_address = stringify(iter->second.addr); - if (m_locker_cookie.empty() || m_locker_address.empty()) { - ldout(m_cct, 20) << "no valid lockers detected" << dendl; - send_lock(); + } else if (r < 0) { + lderr(m_cct) << "failed to retrieve lockers: " << cpp_strerror(r) << dendl; + finish(r); return; } - ldout(m_cct, 10) << "retrieved exclusive locker: " - << m_locker_entity << "@" << m_locker_address << dendl; - send_get_watchers(); + send_lock(); } template -void AcquireRequest::send_get_watchers() { - ldout(m_cct, 10) << __func__ << dendl; +void AcquireRequest::send_lock() { + ldout(m_cct, 10) << dendl; - librados::ObjectReadOperation op; - op.list_watchers(&m_watchers, &m_watchers_ret_val); + librados::ObjectWriteOperation op; + rados::cls::lock::lock(&op, RBD_LOCK_NAME, LOCK_EXCLUSIVE, m_cookie, + ManagedLock::WATCHER_LOCK_TAG, "", utime_t(), 0); using klass = AcquireRequest; librados::AioCompletion *rados_completion = - create_rados_ack_callback(this); - m_out_bl.clear(); - int r = m_ioctx.aio_operate(m_oid, rados_completion, &op, &m_out_bl); + create_rados_safe_callback(this); + int r = m_ioctx.aio_operate(m_oid, rados_completion, &op); assert(r == 0); rados_completion->release(); } template -void AcquireRequest::handle_get_watchers(int r) { - ldout(m_cct, 10) << __func__ << ": r=" << r << dendl; +void AcquireRequest::handle_lock(int r) { + ldout(m_cct, 10) << "r=" << r << dendl; if (r == 0) { - r = m_watchers_ret_val; - } - save_result(r); - if (r < 0) { - lderr(m_cct) << "failed to retrieve watchers: " << cpp_strerror(r) << dendl; - finish(); + finish(0); return; - } - - for (auto &watcher : m_watchers) { - if ((strncmp(m_locker_address.c_str(), - watcher.addr, sizeof(watcher.addr)) == 0) && - (m_locker_handle == watcher.cookie)) { - ldout(m_cct, 10) << "lock owner is still alive" << dendl; - - save_result(-EAGAIN); - finish(); - return; - } - } - - send_blacklist(); -} - -template -void AcquireRequest::send_blacklist() { - if (!m_cct->_conf->rbd_blacklist_on_break_lock) { - send_break_lock(); + } else if (r == -EBUSY && m_locker.cookie.empty()) { + ldout(m_cct, 5) << "already locked, refreshing locker" << dendl; + send_get_locker(); return; - } - ldout(m_cct, 10) << __func__ << dendl; - - // TODO: need async version of RadosClient::blacklist_add - using klass = AcquireRequest; - Context *ctx = create_context_callback( - this); - m_work_queue->queue( - new C_BlacklistClient(m_ioctx, m_locker_address, ctx), 0); -} -template -void AcquireRequest::handle_blacklist(int r) { - ldout(m_cct, 10) << __func__ << ": r=" << r << dendl; - - save_result(r); - if (r < 0) { - lderr(m_cct) << "failed to blacklist lock owner: " << cpp_strerror(r) - << dendl; - finish(); + } else if (r != -EBUSY) { + lderr(m_cct) << "failed to lock: " << cpp_strerror(r) << dendl; + finish(r); return; } + send_break_lock(); } template void AcquireRequest::send_break_lock() { - ldout(m_cct, 10) << __func__ << dendl; - - librados::ObjectWriteOperation op; - rados::cls::lock::break_lock(&op, RBD_LOCK_NAME, m_locker_cookie, - m_locker_entity); - - using klass = AcquireRequest; - librados::AioCompletion *rados_completion = - create_rados_safe_callback(this); - int r = m_ioctx.aio_operate(m_oid, rados_completion, &op); - assert(r == 0); - rados_completion->release(); + ldout(m_cct, 10) << dendl; + + Context *ctx = create_context_callback< + AcquireRequest, &AcquireRequest::handle_break_lock>(this); + auto req = BreakRequest::create( + m_ioctx, m_work_queue, m_oid, m_locker, m_blacklist_on_break_lock, + m_blacklist_expire_seconds, false, ctx); + req->send(); } template void AcquireRequest::handle_break_lock(int r) { - ldout(m_cct, 10) << __func__ << ": r=" << r << dendl; + ldout(m_cct, 10) << "r=" << r << dendl; - if (r == -ENOENT) { - r = 0; + if (r == -EAGAIN) { + ldout(m_cct, 5) << "lock owner is still alive" << dendl; + finish(r); + return; } else if (r < 0) { - lderr(m_cct) << "failed to break lock: " << cpp_strerror(r) << dendl; - save_result(r); - finish(); + lderr(m_cct) << "failed to break lock : " << cpp_strerror(r) << dendl; + finish(r); return; } - send_lock(); + send_get_locker(); } template -void AcquireRequest::finish() { - m_on_finish->complete(m_error_result); +void AcquireRequest::finish(int r) { + m_on_finish->complete(r); delete this; } diff --git a/src/librbd/managed_lock/AcquireRequest.h b/src/librbd/managed_lock/AcquireRequest.h index c8a8a626d36..fe31595df4a 100644 --- a/src/librbd/managed_lock/AcquireRequest.h +++ b/src/librbd/managed_lock/AcquireRequest.h @@ -8,6 +8,7 @@ #include "include/int_types.h" #include "include/buffer.h" #include "msg/msg_types.h" +#include "librbd/managed_lock/Types.h" #include "librbd/watcher/Types.h" #include @@ -29,7 +30,10 @@ private: public: static AcquireRequest* create(librados::IoCtx& ioctx, Watcher *watcher, ContextWQ *work_queue, const std::string& oid, - const std::string& cookie, Context *on_finish); + const std::string& cookie, + bool blacklist_on_break_lock, + uint32_t blacklist_expire_seconds, + Context *on_finish); ~AcquireRequest(); void send(); @@ -41,26 +45,14 @@ private: * * * | - * | - * | - * | /-----------------------------------------------------------\ - * | | | - * | | (no lockers) | - * | | . . . . . . . . . . . . . . . . . . . . . . | - * | | . . | - * | v v (EBUSY) . | - * \--> LOCK_IMAGE * * * * * * * * > GET_LOCKERS . . . . | - * | | | - * | v | - * | GET_WATCHERS | - * | | | - * | v | - * | BLACKLIST (skip if blacklist | - * | | disabled) | - * | v | - * | BREAK_LOCK | - * | | | - * | \-----------------------------/ + * v + * GET_LOCKER <---------------------------------------\ + * | ^ | + * | . (EBUSY && no cached locker) | + * | . | + * | . (EBUSY && cached locker) | + * \--> LOCK_IMAGE * * * * * * * * > BREAK_LOCK ---/ + * | * v * * @@ -69,7 +61,8 @@ private: AcquireRequest(librados::IoCtx& ioctx, Watcher *watcher, ContextWQ *work_queue, const std::string& oid, - const std::string& cookie, Context *on_finish); + const std::string& cookie, bool blacklist_on_break_lock, + uint32_t blacklist_expire_seconds, Context *on_finish); librados::IoCtx& m_ioctx; Watcher *m_watcher; @@ -77,45 +70,24 @@ private: ContextWQ *m_work_queue; std::string m_oid; std::string m_cookie; + bool m_blacklist_on_break_lock; + uint32_t m_blacklist_expire_seconds; Context *m_on_finish; bufferlist m_out_bl; - std::list m_watchers; - int m_watchers_ret_val; + Locker m_locker; - entity_name_t m_locker_entity; - std::string m_locker_cookie; - std::string m_locker_address; - uint64_t m_locker_handle; - - int m_error_result; + void send_get_locker(); + void handle_get_locker(int r); void send_lock(); void handle_lock(int r); - void send_unlock(); - void handle_unlock(int r); - - void send_get_lockers(); - void handle_get_lockers(int r); - - void send_get_watchers(); - void handle_get_watchers(int r); - - void send_blacklist(); - void handle_blacklist(int r); - void send_break_lock(); void handle_break_lock(int r); - void finish(); - - void save_result(int r) { - if (m_error_result == 0 && r < 0) { - m_error_result = r; - } - } + void finish(int r); }; } // namespace managed_lock diff --git a/src/librbd/managed_lock/BreakRequest.cc b/src/librbd/managed_lock/BreakRequest.cc index 579bf793979..d7f65a88930 100644 --- a/src/librbd/managed_lock/BreakRequest.cc +++ b/src/librbd/managed_lock/BreakRequest.cc @@ -8,8 +8,8 @@ #include "include/stringify.h" #include "cls/lock/cls_lock_client.h" #include "cls/lock/cls_lock_types.h" +#include "librbd/ImageCtx.h" #include "librbd/Utils.h" -#include "librbd/managed_lock/Types.h" #define dout_subsys ceph_subsys_rbd #undef dout_prefix @@ -25,28 +25,40 @@ using util::create_rados_safe_callback; namespace { -template struct C_BlacklistClient : public Context { - I &image_ctx; + librados::IoCtx &ioctx; std::string locker_address; + uint32_t expire_seconds; Context *on_finish; - C_BlacklistClient(I &image_ctx, const std::string &locker_address, - Context *on_finish) - : image_ctx(image_ctx), locker_address(locker_address), - on_finish(on_finish) { + C_BlacklistClient(librados::IoCtx &ioctx, const std::string &locker_address, + uint32_t expire_seconds, Context *on_finish) + : ioctx(ioctx), locker_address(locker_address), + expire_seconds(expire_seconds), on_finish(on_finish) { } virtual void finish(int r) override { - librados::Rados rados(image_ctx.md_ctx); - r = rados.blacklist_add(locker_address, - image_ctx.blacklist_expire_seconds); + librados::Rados rados(ioctx); + r = rados.blacklist_add(locker_address, expire_seconds); on_finish->complete(r); } }; } // anonymous namespace +template +BreakRequest::BreakRequest(librados::IoCtx& ioctx, ContextWQ *work_queue, + const std::string& oid, const Locker &locker, + bool blacklist_locker, + uint32_t blacklist_expire_seconds, + bool force_break_lock, Context *on_finish) + : m_ioctx(ioctx), m_cct(reinterpret_cast(m_ioctx.cct())), + m_work_queue(work_queue), m_oid(oid), m_locker(locker), + m_blacklist_locker(blacklist_locker), + m_blacklist_expire_seconds(blacklist_expire_seconds), + m_force_break_lock(force_break_lock), m_on_finish(on_finish) { +} + template void BreakRequest::send() { send_get_watchers(); @@ -54,8 +66,7 @@ void BreakRequest::send() { template void BreakRequest::send_get_watchers() { - CephContext *cct = m_image_ctx.cct; - ldout(cct, 10) << dendl; + ldout(m_cct, 10) << dendl; librados::ObjectReadOperation op; op.list_watchers(&m_watchers, &m_watchers_ret_val); @@ -64,23 +75,21 @@ void BreakRequest::send_get_watchers() { librados::AioCompletion *rados_completion = create_rados_ack_callback(this); m_out_bl.clear(); - int r = m_image_ctx.md_ctx.aio_operate(m_image_ctx.header_oid, - rados_completion, &op, &m_out_bl); + int r = m_ioctx.aio_operate(m_oid, rados_completion, &op, &m_out_bl); assert(r == 0); rados_completion->release(); } template void BreakRequest::handle_get_watchers(int r) { - CephContext *cct = m_image_ctx.cct; - ldout(cct, 10) << "r=" << r << dendl; + ldout(m_cct, 10) << "r=" << r << dendl; if (r == 0) { r = m_watchers_ret_val; } if (r < 0) { - lderr(cct) << "failed to retrieve watchers: " << cpp_strerror(r) - << dendl; + lderr(m_cct) << "failed to retrieve watchers: " << cpp_strerror(r) + << dendl; finish(r); return; } @@ -89,7 +98,7 @@ void BreakRequest::handle_get_watchers(int r) { if ((strncmp(m_locker.address.c_str(), watcher.addr, sizeof(watcher.addr)) == 0) && (m_locker.handle == watcher.cookie)) { - ldout(cct, 10) << "lock owner is still alive" << dendl; + ldout(m_cct, 10) << "lock owner is still alive" << dendl; if (m_force_break_lock) { break; @@ -110,26 +119,24 @@ void BreakRequest::send_blacklist() { return; } - CephContext *cct = m_image_ctx.cct; - ldout(cct, 10) << dendl; + ldout(m_cct, 10) << dendl; // TODO: need async version of RadosClient::blacklist_add using klass = BreakRequest; Context *ctx = create_context_callback( this); - m_image_ctx.op_work_queue->queue(new C_BlacklistClient(m_image_ctx, - m_locker.address, - ctx), 0); + m_work_queue->queue(new C_BlacklistClient(m_ioctx, m_locker.address, + m_blacklist_expire_seconds, ctx), + 0); } template void BreakRequest::handle_blacklist(int r) { - CephContext *cct = m_image_ctx.cct; - ldout(cct, 10) << "r=" << r << dendl; + ldout(m_cct, 10) << "r=" << r << dendl; if (r < 0) { - lderr(cct) << "failed to blacklist lock owner: " << cpp_strerror(r) - << dendl; + lderr(m_cct) << "failed to blacklist lock owner: " << cpp_strerror(r) + << dendl; finish(r); return; } @@ -138,8 +145,7 @@ void BreakRequest::handle_blacklist(int r) { template void BreakRequest::send_break_lock() { - CephContext *cct = m_image_ctx.cct; - ldout(cct, 10) << dendl; + ldout(m_cct, 10) << dendl; librados::ObjectWriteOperation op; rados::cls::lock::break_lock(&op, RBD_LOCK_NAME, m_locker.cookie, @@ -148,19 +154,17 @@ void BreakRequest::send_break_lock() { using klass = BreakRequest; librados::AioCompletion *rados_completion = create_rados_safe_callback(this); - int r = m_image_ctx.md_ctx.aio_operate(m_image_ctx.header_oid, - rados_completion, &op); + int r = m_ioctx.aio_operate(m_oid, rados_completion, &op); assert(r == 0); rados_completion->release(); } template void BreakRequest::handle_break_lock(int r) { - CephContext *cct = m_image_ctx.cct; - ldout(cct, 10) << "r=" << r << dendl; + ldout(m_cct, 10) << "r=" << r << dendl; if (r < 0 && r != -ENOENT) { - lderr(cct) << "failed to break lock: " << cpp_strerror(r) << dendl; + lderr(m_cct) << "failed to break lock: " << cpp_strerror(r) << dendl; finish(r); return; } @@ -170,8 +174,7 @@ void BreakRequest::handle_break_lock(int r) { template void BreakRequest::finish(int r) { - CephContext *cct = m_image_ctx.cct; - ldout(cct, 10) << "r=" << r << dendl; + ldout(m_cct, 10) << "r=" << r << dendl; m_on_finish->complete(r); delete this; diff --git a/src/librbd/managed_lock/BreakRequest.h b/src/librbd/managed_lock/BreakRequest.h index 5e40b2b5c66..277e79eb927 100644 --- a/src/librbd/managed_lock/BreakRequest.h +++ b/src/librbd/managed_lock/BreakRequest.h @@ -7,29 +7,34 @@ #include "include/int_types.h" #include "include/buffer.h" #include "msg/msg_types.h" -#include "librbd/ImageCtx.h" #include #include #include +#include "librbd/managed_lock/Types.h" class Context; +class ContextWQ; +class obj_watch_t; + +namespace librados { class IoCtx; } namespace librbd { +class ImageCtx; template class Journal; namespace managed_lock { -struct Locker; - template class BreakRequest { public: - static BreakRequest* create(ImageCtxT &image_ctx, const Locker &locker, - bool blacklist_locker, bool force_break_lock, - Context *on_finish) { - return new BreakRequest(image_ctx, locker, blacklist_locker, - force_break_lock, on_finish); + static BreakRequest* create(librados::IoCtx& ioctx, ContextWQ *work_queue, + const std::string& oid, const Locker &locker, + bool blacklist_locker, + uint32_t blacklist_expire_seconds, + bool force_break_lock, Context *on_finish) { + return new BreakRequest(ioctx, work_queue, oid, locker, blacklist_locker, + blacklist_expire_seconds, force_break_lock, on_finish); } void send(); @@ -55,9 +60,13 @@ private: * @endvertbatim */ - ImageCtxT &m_image_ctx; - const Locker &m_locker; + librados::IoCtx &m_ioctx; + CephContext *m_cct; + ContextWQ *m_work_queue; + std::string m_oid; + Locker m_locker; bool m_blacklist_locker; + uint32_t m_blacklist_expire_seconds; bool m_force_break_lock; Context *m_on_finish; @@ -66,13 +75,10 @@ private: std::list m_watchers; int m_watchers_ret_val; - BreakRequest(ImageCtxT &image_ctx, const Locker &locker, - bool blacklist_locker, bool force_break_lock, - Context *on_finish) - : m_image_ctx(image_ctx), m_locker(locker), - m_blacklist_locker(blacklist_locker), - m_force_break_lock(force_break_lock), m_on_finish(on_finish) { - } + BreakRequest(librados::IoCtx& ioctx, ContextWQ *work_queue, + const std::string& oid, const Locker &locker, + bool blacklist_locker, uint32_t blacklist_expire_seconds, + bool force_break_lock, Context *on_finish); void send_get_watchers(); void handle_get_watchers(int r); diff --git a/src/librbd/managed_lock/GetLockerRequest.cc b/src/librbd/managed_lock/GetLockerRequest.cc index e6da32766fe..25b182743d1 100644 --- a/src/librbd/managed_lock/GetLockerRequest.cc +++ b/src/librbd/managed_lock/GetLockerRequest.cc @@ -22,6 +22,14 @@ namespace managed_lock { using util::create_rados_ack_callback; +template +GetLockerRequest::GetLockerRequest(librados::IoCtx& ioctx, + const std::string& oid, Locker *locker, + Context *on_finish) + : m_ioctx(ioctx), m_cct(reinterpret_cast(m_ioctx.cct())), + m_oid(oid), m_locker(locker), m_on_finish(on_finish) { +} + template void GetLockerRequest::send() { send_get_lockers(); @@ -29,8 +37,7 @@ void GetLockerRequest::send() { template void GetLockerRequest::send_get_lockers() { - CephContext *cct = m_image_ctx.cct; - ldout(cct, 10) << dendl; + ldout(m_cct, 10) << dendl; librados::ObjectReadOperation op; rados::cls::lock::get_lock_info_start(&op, RBD_LOCK_NAME); @@ -39,16 +46,14 @@ void GetLockerRequest::send_get_lockers() { librados::AioCompletion *rados_completion = create_rados_ack_callback(this); m_out_bl.clear(); - int r = m_image_ctx.md_ctx.aio_operate(m_image_ctx.header_oid, - rados_completion, &op, &m_out_bl); + int r = m_ioctx.aio_operate(m_oid, rados_completion, &op, &m_out_bl); assert(r == 0); rados_completion->release(); } template void GetLockerRequest::handle_get_lockers(int r) { - CephContext *cct = m_image_ctx.cct; - ldout(cct, 10) << "r=" << r << dendl; + ldout(m_cct, 10) << "r=" << r << dendl; std::map lockers; @@ -61,25 +66,25 @@ void GetLockerRequest::handle_get_lockers(int r) { } if (r < 0) { - lderr(cct) << "failed to retrieve lockers: " << cpp_strerror(r) << dendl; + lderr(m_cct) << "failed to retrieve lockers: " << cpp_strerror(r) << dendl; finish(r); return; } if (lockers.empty()) { - ldout(cct, 20) << "no lockers detected" << dendl; + ldout(m_cct, 20) << "no lockers detected" << dendl; finish(-ENOENT); return; } if (lock_tag != ManagedLock<>::WATCHER_LOCK_TAG) { - ldout(cct, 5) <<"locked by external mechanism: tag=" << lock_tag << dendl; + ldout(m_cct, 5) <<"locked by external mechanism: tag=" << lock_tag << dendl; finish(-EBUSY); return; } if (lock_type == LOCK_SHARED) { - ldout(cct, 5) << "shared lock type detected" << dendl; + ldout(m_cct, 5) << "shared lock type detected" << dendl; finish(-EBUSY); return; } @@ -88,8 +93,8 @@ void GetLockerRequest::handle_get_lockers(int r) { rados::cls::lock::locker_info_t>::iterator iter = lockers.begin(); if (!ManagedLock<>::decode_lock_cookie(iter->first.cookie, &m_locker->handle)) { - ldout(cct, 5) << "locked by external mechanism: " - << "cookie=" << iter->first.cookie << dendl; + ldout(m_cct, 5) << "locked by external mechanism: " + << "cookie=" << iter->first.cookie << dendl; finish(-EBUSY); return; } @@ -98,20 +103,19 @@ void GetLockerRequest::handle_get_lockers(int r) { m_locker->cookie = iter->first.cookie; m_locker->address = stringify(iter->second.addr); if (m_locker->cookie.empty() || m_locker->address.empty()) { - ldout(cct, 20) << "no valid lockers detected" << dendl; + ldout(m_cct, 20) << "no valid lockers detected" << dendl; finish(-ENOENT); return; } - ldout(cct, 10) << "retrieved exclusive locker: " + ldout(m_cct, 10) << "retrieved exclusive locker: " << m_locker->entity << "@" << m_locker->address << dendl; finish(0); } template void GetLockerRequest::finish(int r) { - CephContext *cct = m_image_ctx.cct; - ldout(cct, 10) << "r=" << r << dendl; + ldout(m_cct, 10) << "r=" << r << dendl; m_on_finish->complete(r); delete this; @@ -121,4 +125,3 @@ void GetLockerRequest::finish(int r) { } // namespace librbd template class librbd::managed_lock::GetLockerRequest; - diff --git a/src/librbd/managed_lock/GetLockerRequest.h b/src/librbd/managed_lock/GetLockerRequest.h index b0a5788465a..97373c5f6bb 100644 --- a/src/librbd/managed_lock/GetLockerRequest.h +++ b/src/librbd/managed_lock/GetLockerRequest.h @@ -9,6 +9,8 @@ class Context; +namespace librados { class IoCtx; } + namespace librbd { struct ImageCtx; @@ -20,23 +22,25 @@ struct Locker; template class GetLockerRequest { public: - static GetLockerRequest* create(ImageCtxT &image_ctx, Locker *locker, - Context *on_finish) { - return new GetLockerRequest(image_ctx, locker, on_finish); + static GetLockerRequest* create(librados::IoCtx& ioctx, + const std::string& oid, + Locker *locker, Context *on_finish) { + return new GetLockerRequest(ioctx, oid, locker, on_finish); } void send(); private: - ImageCtxT &m_image_ctx; + librados::IoCtx &m_ioctx; + CephContext *m_cct; + std::string m_oid; Locker *m_locker; Context *m_on_finish; bufferlist m_out_bl; - GetLockerRequest(ImageCtxT &image_ctx, Locker *locker, Context *on_finish) - : m_image_ctx(image_ctx), m_locker(locker), m_on_finish(on_finish) { - } + GetLockerRequest(librados::IoCtx& ioctx, const std::string& oid, + Locker *locker, Context *on_finish); void send_get_lockers(); void handle_get_lockers(int r); diff --git a/src/test/librbd/managed_lock/test_mock_AcquireRequest.cc b/src/test/librbd/managed_lock/test_mock_AcquireRequest.cc index d256db42450..b99f202ffeb 100644 --- a/src/test/librbd/managed_lock/test_mock_AcquireRequest.cc +++ b/src/test/librbd/managed_lock/test_mock_AcquireRequest.cc @@ -7,6 +7,8 @@ #include "test/librados_test_stub/MockTestMemRadosClient.h" #include "cls/lock/cls_lock_ops.h" #include "librbd/managed_lock/AcquireRequest.h" +#include "librbd/managed_lock/BreakRequest.h" +#include "librbd/managed_lock/GetLockerRequest.h" #include "gmock/gmock.h" #include "gtest/gtest.h" #include @@ -19,7 +21,61 @@ struct Traits { typedef librbd::MockImageWatcher Watcher; }; } -} + +namespace managed_lock { + +template<> +struct BreakRequest { + Context *on_finish = nullptr; + static BreakRequest *s_instance; + static BreakRequest* create(librados::IoCtx& ioctx, ContextWQ *work_queue, + const std::string& oid, const Locker &locker, + bool blacklist_locker, + uint32_t blacklist_expire_seconds, + bool force_break_lock, Context *on_finish) { + CephContext *cct = reinterpret_cast(ioctx.cct()); + EXPECT_EQ(cct->_conf->rbd_blacklist_on_break_lock, blacklist_locker); + EXPECT_EQ(cct->_conf->rbd_blacklist_expire_seconds, + (int)blacklist_expire_seconds); + EXPECT_FALSE(force_break_lock); + assert(s_instance != nullptr); + s_instance->on_finish = on_finish; + return s_instance; + } + + BreakRequest() { + s_instance = this; + } + MOCK_METHOD0(send, void()); +}; + +template <> +struct GetLockerRequest { + Locker *locker; + Context *on_finish; + + static GetLockerRequest *s_instance; + static GetLockerRequest* create(librados::IoCtx& ioctx, + const std::string& oid, + Locker *locker, Context *on_finish) { + assert(s_instance != nullptr); + s_instance->locker = locker; + s_instance->on_finish = on_finish; + return s_instance; + } + + GetLockerRequest() { + s_instance = this; + } + + MOCK_METHOD0(send, void()); +}; + +BreakRequest *BreakRequest::s_instance = nullptr; +GetLockerRequest *GetLockerRequest::s_instance = nullptr; + +} // namespace managed_lock +} // namespace librbd // template definitions #include "librbd/managed_lock/AcquireRequest.cc" @@ -45,6 +101,8 @@ static const std::string TEST_COOKIE("auto 123"); class TestMockManagedLockAcquireRequest : public TestMockFixture { public: typedef AcquireRequest MockAcquireRequest; + typedef BreakRequest MockBreakRequest; + typedef GetLockerRequest MockGetLockerRequest; typedef ManagedLock MockManagedLock; void expect_lock(MockImageCtx &mock_image_ctx, int r) { @@ -53,74 +111,22 @@ public: .WillOnce(Return(r)); } - void expect_unlock(MockImageCtx &mock_image_ctx, int r) { - EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), - exec(mock_image_ctx.header_oid, _, StrEq("lock"), StrEq("unlock"), _, _, _)) - .WillOnce(Return(r)); - } - - void expect_get_lock_info(MockImageCtx &mock_image_ctx, int r, - const entity_name_t &locker_entity, - const std::string &locker_address, - const std::string &locker_cookie, - const std::string &lock_tag, - ClsLockType lock_type) { - auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), - exec(mock_image_ctx.header_oid, _, StrEq("lock"), - StrEq("get_info"), _, _, _)); - if (r < 0 && r != -ENOENT) { - expect.WillOnce(Return(r)); - } else { - entity_name_t entity(locker_entity); - entity_addr_t entity_addr; - entity_addr.parse(locker_address.c_str(), NULL); - - cls_lock_get_info_reply reply; - if (r != -ENOENT) { - reply.lockers = decltype(reply.lockers){ - {rados::cls::lock::locker_id_t(entity, locker_cookie), - rados::cls::lock::locker_info_t(utime_t(), entity_addr, "")}}; - reply.tag = lock_tag; - reply.lock_type = lock_type; - } - - bufferlist bl; - ::encode(reply, bl, CEPH_FEATURES_SUPPORTED_DEFAULT); - - std::string str(bl.c_str(), bl.length()); - expect.WillOnce(DoAll(WithArg<5>(CopyInBufferlist(str)), Return(0))); - } - } - - void expect_list_watchers(MockImageCtx &mock_image_ctx, int r, - const std::string &address, uint64_t watch_handle) { - auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), - list_watchers(mock_image_ctx.header_oid, _)); - if (r < 0) { - expect.WillOnce(Return(r)); - } else { - obj_watch_t watcher; - strcpy(watcher.addr, (address + ":0/0").c_str()); - watcher.cookie = watch_handle; - - std::list watchers; - watchers.push_back(watcher); - - expect.WillOnce(DoAll(SetArgPointee<1>(watchers), Return(0))); - } + void expect_get_locker(MockImageCtx &mock_image_ctx, + MockGetLockerRequest &mock_get_locker_request, + const Locker &locker, int r) { + EXPECT_CALL(mock_get_locker_request, send()) + .WillOnce(Invoke([&mock_image_ctx, &mock_get_locker_request, locker, r]() { + *mock_get_locker_request.locker = locker; + mock_image_ctx.image_ctx->op_work_queue->queue( + mock_get_locker_request.on_finish, r); + })); } - void expect_blacklist_add(MockImageCtx &mock_image_ctx, int r) { - EXPECT_CALL(get_mock_rados_client(), blacklist_add(_, _)) - .WillOnce(Return(r)); - } - - void expect_break_lock(MockImageCtx &mock_image_ctx, int r) { - EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), - exec(mock_image_ctx.header_oid, _, StrEq("lock"), - StrEq("break_lock"), _, _, _)).WillOnce(Return(r)); + void expect_break_lock(MockImageCtx &mock_image_ctx, + MockBreakRequest &mock_break_request, int r) { + EXPECT_CALL(mock_break_request, send()) + .WillOnce(FinishRequest(&mock_break_request, r, &mock_image_ctx)); } - }; TEST_F(TestMockManagedLockAcquireRequest, Success) { @@ -129,14 +135,16 @@ TEST_F(TestMockManagedLockAcquireRequest, Success) { ASSERT_EQ(0, open_image(m_image_name, &ictx)); MockImageCtx mock_image_ctx(*ictx); + MockGetLockerRequest mock_get_locker_request; InSequence seq; + expect_get_locker(mock_image_ctx, mock_get_locker_request, {}, 0); expect_lock(mock_image_ctx, 0); C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx.md_ctx, mock_image_ctx.image_watcher, ictx->op_work_queue, mock_image_ctx.header_oid, - TEST_COOKIE, &ctx); + TEST_COOKIE, true, 0, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -146,22 +154,23 @@ TEST_F(TestMockManagedLockAcquireRequest, LockBusy) { ASSERT_EQ(0, open_image(m_image_name, &ictx)); MockImageCtx mock_image_ctx(*ictx); + MockGetLockerRequest mock_get_locker_request; + MockBreakRequest mock_break_request; expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_get_locker(mock_image_ctx, mock_get_locker_request, + {entity_name_t::CLIENT(1), "auto 123", "1.2.3.4:0/0", 123}, + 0); expect_lock(mock_image_ctx, -EBUSY); - expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockManagedLock::WATCHER_LOCK_TAG, - LOCK_EXCLUSIVE); - expect_list_watchers(mock_image_ctx, 0, "dead client", 123); - expect_blacklist_add(mock_image_ctx, 0); - expect_break_lock(mock_image_ctx, 0); + expect_break_lock(mock_image_ctx, mock_break_request, 0); + expect_get_locker(mock_image_ctx, mock_get_locker_request, {}, 0); expect_lock(mock_image_ctx, -ENOENT); C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx.md_ctx, mock_image_ctx.image_watcher, ictx->op_work_queue, mock_image_ctx.header_oid, - TEST_COOKIE, &ctx); + TEST_COOKIE, true, 0, &ctx); req->send(); ASSERT_EQ(-ENOENT, ctx.wait()); } @@ -171,16 +180,15 @@ TEST_F(TestMockManagedLockAcquireRequest, GetLockInfoError) { ASSERT_EQ(0, open_image(m_image_name, &ictx)); MockImageCtx mock_image_ctx(*ictx); + MockGetLockerRequest mock_get_locker_request; InSequence seq; - expect_lock(mock_image_ctx, -EBUSY); - expect_get_lock_info(mock_image_ctx, -EINVAL, entity_name_t::CLIENT(1), "", - "", "", LOCK_EXCLUSIVE); + expect_get_locker(mock_image_ctx, mock_get_locker_request, {}, -EINVAL); C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx.md_ctx, mock_image_ctx.image_watcher, ictx->op_work_queue, mock_image_ctx.header_oid, - TEST_COOKIE, &ctx); + TEST_COOKIE, true, 0, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -190,191 +198,16 @@ TEST_F(TestMockManagedLockAcquireRequest, GetLockInfoEmpty) { ASSERT_EQ(0, open_image(m_image_name, &ictx)); MockImageCtx mock_image_ctx(*ictx); + MockGetLockerRequest mock_get_locker_request; InSequence seq; - expect_lock(mock_image_ctx, -EBUSY); - expect_get_lock_info(mock_image_ctx, -ENOENT, entity_name_t::CLIENT(1), "", - "", "", LOCK_EXCLUSIVE); - expect_lock(mock_image_ctx, -EINVAL); - - C_SaferCond ctx; - MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx.md_ctx, - mock_image_ctx.image_watcher, ictx->op_work_queue, mock_image_ctx.header_oid, - TEST_COOKIE, &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - -TEST_F(TestMockManagedLockAcquireRequest, GetLockInfoExternalTag) { - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockImageCtx mock_image_ctx(*ictx); - - InSequence seq; - expect_lock(mock_image_ctx, -EBUSY); - expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", "external tag", LOCK_EXCLUSIVE); - - C_SaferCond ctx; - MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx.md_ctx, - mock_image_ctx.image_watcher, ictx->op_work_queue, mock_image_ctx.header_oid, - TEST_COOKIE, &ctx); - req->send(); - ASSERT_EQ(-EBUSY, ctx.wait()); -} - -TEST_F(TestMockManagedLockAcquireRequest, GetLockInfoShared) { - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockImageCtx mock_image_ctx(*ictx); - - InSequence seq; - expect_lock(mock_image_ctx, -EBUSY); - expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockManagedLock::WATCHER_LOCK_TAG, - LOCK_SHARED); - - C_SaferCond ctx; - MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx.md_ctx, - mock_image_ctx.image_watcher, ictx->op_work_queue, mock_image_ctx.header_oid, - TEST_COOKIE, &ctx); - req->send(); - ASSERT_EQ(-EBUSY, ctx.wait()); -} - -TEST_F(TestMockManagedLockAcquireRequest, GetLockInfoExternalCookie) { - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockImageCtx mock_image_ctx(*ictx); - - InSequence seq; - expect_lock(mock_image_ctx, -EBUSY); - expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "external cookie", MockManagedLock::WATCHER_LOCK_TAG, - LOCK_EXCLUSIVE); - - C_SaferCond ctx; - MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx.md_ctx, - mock_image_ctx.image_watcher, ictx->op_work_queue, mock_image_ctx.header_oid, - TEST_COOKIE, &ctx); - req->send(); - ASSERT_EQ(-EBUSY, ctx.wait()); -} - -TEST_F(TestMockManagedLockAcquireRequest, GetWatchersError) { - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockImageCtx mock_image_ctx(*ictx); - - InSequence seq; - expect_lock(mock_image_ctx, -EBUSY); - expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockManagedLock::WATCHER_LOCK_TAG, - LOCK_EXCLUSIVE); - expect_list_watchers(mock_image_ctx, -EINVAL, "dead client", 123); - - C_SaferCond ctx; - MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx.md_ctx, - mock_image_ctx.image_watcher, ictx->op_work_queue, mock_image_ctx.header_oid, - TEST_COOKIE, &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - -TEST_F(TestMockManagedLockAcquireRequest, GetWatchersAlive) { - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockImageCtx mock_image_ctx(*ictx); - - InSequence seq; - expect_lock(mock_image_ctx, -EBUSY); - expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockManagedLock::WATCHER_LOCK_TAG, - LOCK_EXCLUSIVE); - expect_list_watchers(mock_image_ctx, 0, "1.2.3.4", 123); - - C_SaferCond ctx; - MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx.md_ctx, - mock_image_ctx.image_watcher, ictx->op_work_queue, mock_image_ctx.header_oid, - TEST_COOKIE, &ctx); - req->send(); - ASSERT_EQ(-EAGAIN, ctx.wait()); -} - -TEST_F(TestMockManagedLockAcquireRequest, BlacklistDisabled) { - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockImageCtx mock_image_ctx(*ictx); - CephContext *cct = reinterpret_cast(mock_image_ctx.md_ctx.cct()); - cct->_conf->set_val("rbd_blacklist_on_break_lock", "false"); - - InSequence seq; - expect_lock(mock_image_ctx, -EBUSY); - expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockManagedLock::WATCHER_LOCK_TAG, - LOCK_EXCLUSIVE); - expect_list_watchers(mock_image_ctx, 0, "dead client", 123); - expect_break_lock(mock_image_ctx, 0); - expect_lock(mock_image_ctx, -ENOENT); - - C_SaferCond ctx; - MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx.md_ctx, - mock_image_ctx.image_watcher, ictx->op_work_queue, mock_image_ctx.header_oid, - TEST_COOKIE, &ctx); - req->send(); - ASSERT_EQ(-ENOENT, ctx.wait()); - - cct->_conf->set_val("rbd_blacklist_on_break_lock", "true"); -} - -TEST_F(TestMockManagedLockAcquireRequest, BlacklistError) { - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockImageCtx mock_image_ctx(*ictx); - - InSequence seq; - expect_lock(mock_image_ctx, -EBUSY); - expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockManagedLock::WATCHER_LOCK_TAG, - LOCK_EXCLUSIVE); - expect_list_watchers(mock_image_ctx, 0, "dead client", 123); - expect_blacklist_add(mock_image_ctx, -EINVAL); - - C_SaferCond ctx; - MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx.md_ctx, - mock_image_ctx.image_watcher, ictx->op_work_queue, mock_image_ctx.header_oid, - TEST_COOKIE, &ctx); - req->send(); - ASSERT_EQ(-EINVAL, ctx.wait()); -} - -TEST_F(TestMockManagedLockAcquireRequest, BreakLockMissing) { - librbd::ImageCtx *ictx; - ASSERT_EQ(0, open_image(m_image_name, &ictx)); - - MockImageCtx mock_image_ctx(*ictx); - - InSequence seq; - expect_lock(mock_image_ctx, -EBUSY); - expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockManagedLock::WATCHER_LOCK_TAG, - LOCK_EXCLUSIVE); - expect_list_watchers(mock_image_ctx, 0, "dead client", 123); - expect_blacklist_add(mock_image_ctx, 0); - expect_break_lock(mock_image_ctx, -ENOENT); + expect_get_locker(mock_image_ctx, mock_get_locker_request, {}, -ENOENT); expect_lock(mock_image_ctx, -EINVAL); C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx.md_ctx, mock_image_ctx.image_watcher, ictx->op_work_queue, mock_image_ctx.header_oid, - TEST_COOKIE, &ctx); + TEST_COOKIE, true, 0, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -384,20 +217,20 @@ TEST_F(TestMockManagedLockAcquireRequest, BreakLockError) { ASSERT_EQ(0, open_image(m_image_name, &ictx)); MockImageCtx mock_image_ctx(*ictx); + MockGetLockerRequest mock_get_locker_request; + MockBreakRequest mock_break_request; InSequence seq; + expect_get_locker(mock_image_ctx, mock_get_locker_request, + {entity_name_t::CLIENT(1), "auto 123", "1.2.3.4:0/0", 123}, + 0); expect_lock(mock_image_ctx, -EBUSY); - expect_get_lock_info(mock_image_ctx, 0, entity_name_t::CLIENT(1), "1.2.3.4", - "auto 123", MockManagedLock::WATCHER_LOCK_TAG, - LOCK_EXCLUSIVE); - expect_list_watchers(mock_image_ctx, 0, "dead client", 123); - expect_blacklist_add(mock_image_ctx, 0); - expect_break_lock(mock_image_ctx, -EINVAL); + expect_break_lock(mock_image_ctx, mock_break_request, -EINVAL); C_SaferCond ctx; MockAcquireRequest *req = MockAcquireRequest::create(mock_image_ctx.md_ctx, mock_image_ctx.image_watcher, ictx->op_work_queue, mock_image_ctx.header_oid, - TEST_COOKIE, &ctx); + TEST_COOKIE, true, 0, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } diff --git a/src/test/librbd/managed_lock/test_mock_BreakRequest.cc b/src/test/librbd/managed_lock/test_mock_BreakRequest.cc index 398063069a3..df6b04e5d48 100644 --- a/src/test/librbd/managed_lock/test_mock_BreakRequest.cc +++ b/src/test/librbd/managed_lock/test_mock_BreakRequest.cc @@ -90,8 +90,9 @@ TEST_F(TestMockManagedLockBreakRequest, DeadLockOwner) { C_SaferCond ctx; Locker locker{entity_name_t::CLIENT(1), "auto 123", "1.2.3.4:0/0", 123}; - MockBreakRequest *req = MockBreakRequest::create(mock_image_ctx, locker, - true, false, &ctx); + MockBreakRequest *req = MockBreakRequest::create( + mock_image_ctx.md_ctx, ictx->op_work_queue, mock_image_ctx.header_oid, + locker, true, 0, false, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -112,8 +113,9 @@ TEST_F(TestMockManagedLockBreakRequest, ForceBreak) { C_SaferCond ctx; Locker locker{entity_name_t::CLIENT(1), "auto 123", "1.2.3.4:0/0", 123}; - MockBreakRequest *req = MockBreakRequest::create(mock_image_ctx, locker, - true, true, &ctx); + MockBreakRequest *req = MockBreakRequest::create( + mock_image_ctx.md_ctx, ictx->op_work_queue, mock_image_ctx.header_oid, + locker, true, 0, true, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -132,8 +134,9 @@ TEST_F(TestMockManagedLockBreakRequest, GetWatchersError) { C_SaferCond ctx; Locker locker{entity_name_t::CLIENT(1), "auto 123", "1.2.3.4:0/0", 123}; - MockBreakRequest *req = MockBreakRequest::create(mock_image_ctx, locker, - true, false, &ctx); + MockBreakRequest *req = MockBreakRequest::create( + mock_image_ctx.md_ctx, ictx->op_work_queue, mock_image_ctx.header_oid, + locker, true, 0, false, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -152,8 +155,9 @@ TEST_F(TestMockManagedLockBreakRequest, GetWatchersAlive) { C_SaferCond ctx; Locker locker{entity_name_t::CLIENT(1), "auto 123", "1.2.3.4:0/0", 123}; - MockBreakRequest *req = MockBreakRequest::create(mock_image_ctx, locker, - true, false, &ctx); + MockBreakRequest *req = MockBreakRequest::create( + mock_image_ctx.md_ctx, ictx->op_work_queue, mock_image_ctx.header_oid, + locker, true, 0, false, &ctx); req->send(); ASSERT_EQ(-EAGAIN, ctx.wait()); } @@ -173,8 +177,9 @@ TEST_F(TestMockManagedLockBreakRequest, BlacklistDisabled) { C_SaferCond ctx; Locker locker{entity_name_t::CLIENT(1), "auto 123", "1.2.3.4:0/0", 123}; - MockBreakRequest *req = MockBreakRequest::create(mock_image_ctx, locker, - false, false, &ctx); + MockBreakRequest *req = MockBreakRequest::create( + mock_image_ctx.md_ctx, ictx->op_work_queue, mock_image_ctx.header_oid, + locker, false, 0, false, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -194,8 +199,9 @@ TEST_F(TestMockManagedLockBreakRequest, BlacklistError) { C_SaferCond ctx; Locker locker{entity_name_t::CLIENT(1), "auto 123", "1.2.3.4:0/0", 123}; - MockBreakRequest *req = MockBreakRequest::create(mock_image_ctx, locker, - true, false, &ctx); + MockBreakRequest *req = MockBreakRequest::create( + mock_image_ctx.md_ctx, ictx->op_work_queue, mock_image_ctx.header_oid, + locker, true, 0, false, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -216,8 +222,9 @@ TEST_F(TestMockManagedLockBreakRequest, BreakLockMissing) { C_SaferCond ctx; Locker locker{entity_name_t::CLIENT(1), "auto 123", "1.2.3.4:0/0", 123}; - MockBreakRequest *req = MockBreakRequest::create(mock_image_ctx, locker, - true, false, &ctx); + MockBreakRequest *req = MockBreakRequest::create( + mock_image_ctx.md_ctx, ictx->op_work_queue, mock_image_ctx.header_oid, + locker, true, 0, false, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); } @@ -238,8 +245,9 @@ TEST_F(TestMockManagedLockBreakRequest, BreakLockError) { C_SaferCond ctx; Locker locker{entity_name_t::CLIENT(1), "auto 123", "1.2.3.4:0/0", 123}; - MockBreakRequest *req = MockBreakRequest::create(mock_image_ctx, locker, - true, false, &ctx); + MockBreakRequest *req = MockBreakRequest::create( + mock_image_ctx.md_ctx, ictx->op_work_queue, mock_image_ctx.header_oid, + locker, true, 0, false, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } diff --git a/src/test/librbd/managed_lock/test_mock_GetLockerRequest.cc b/src/test/librbd/managed_lock/test_mock_GetLockerRequest.cc index 6b68c20ebe8..af3bb07a369 100644 --- a/src/test/librbd/managed_lock/test_mock_GetLockerRequest.cc +++ b/src/test/librbd/managed_lock/test_mock_GetLockerRequest.cc @@ -94,8 +94,8 @@ TEST_F(TestMockManagedLockGetLockerRequest, Success) { C_SaferCond ctx; Locker locker; - MockGetLockerRequest *req = MockGetLockerRequest::create(mock_image_ctx, - &locker, &ctx); + MockGetLockerRequest *req = MockGetLockerRequest::create( + mock_image_ctx.md_ctx, mock_image_ctx.header_oid, &locker, &ctx); req->send(); ASSERT_EQ(0, ctx.wait()); @@ -120,8 +120,8 @@ TEST_F(TestMockManagedLockGetLockerRequest, GetLockInfoError) { C_SaferCond ctx; Locker locker; - MockGetLockerRequest *req = MockGetLockerRequest::create(mock_image_ctx, - &locker, &ctx); + MockGetLockerRequest *req = MockGetLockerRequest::create( + mock_image_ctx.md_ctx, mock_image_ctx.header_oid, &locker, &ctx); req->send(); ASSERT_EQ(-EINVAL, ctx.wait()); } @@ -141,8 +141,8 @@ TEST_F(TestMockManagedLockGetLockerRequest, GetLockInfoEmpty) { C_SaferCond ctx; Locker locker; - MockGetLockerRequest *req = MockGetLockerRequest::create(mock_image_ctx, - &locker, &ctx); + MockGetLockerRequest *req = MockGetLockerRequest::create( + mock_image_ctx.md_ctx, mock_image_ctx.header_oid, &locker, &ctx); req->send(); ASSERT_EQ(-ENOENT, ctx.wait()); } @@ -162,8 +162,8 @@ TEST_F(TestMockManagedLockGetLockerRequest, GetLockInfoExternalTag) { C_SaferCond ctx; Locker locker; - MockGetLockerRequest *req = MockGetLockerRequest::create(mock_image_ctx, - &locker, &ctx); + MockGetLockerRequest *req = MockGetLockerRequest::create( + mock_image_ctx.md_ctx, mock_image_ctx.header_oid, &locker, &ctx); req->send(); ASSERT_EQ(-EBUSY, ctx.wait()); } @@ -184,8 +184,8 @@ TEST_F(TestMockManagedLockGetLockerRequest, GetLockInfoShared) { C_SaferCond ctx; Locker locker; - MockGetLockerRequest *req = MockGetLockerRequest::create(mock_image_ctx, - &locker, &ctx); + MockGetLockerRequest *req = MockGetLockerRequest::create( + mock_image_ctx.md_ctx, mock_image_ctx.header_oid, &locker, &ctx); req->send(); ASSERT_EQ(-EBUSY, ctx.wait()); } @@ -206,8 +206,8 @@ TEST_F(TestMockManagedLockGetLockerRequest, GetLockInfoExternalCookie) { C_SaferCond ctx; Locker locker; - MockGetLockerRequest *req = MockGetLockerRequest::create(mock_image_ctx, - &locker, &ctx); + MockGetLockerRequest *req = MockGetLockerRequest::create( + mock_image_ctx.md_ctx, mock_image_ctx.header_oid, &locker, &ctx); req->send(); ASSERT_EQ(-EBUSY, ctx.wait()); } diff --git a/src/test/librbd/test_mock_ExclusiveLock.cc b/src/test/librbd/test_mock_ExclusiveLock.cc index ea53b5da8a3..525d95b94f0 100644 --- a/src/test/librbd/test_mock_ExclusiveLock.cc +++ b/src/test/librbd/test_mock_ExclusiveLock.cc @@ -114,6 +114,15 @@ std::list BaseRequest::s_requests; template <> struct AcquireRequest : public BaseRequest > { + static AcquireRequest* create(librados::IoCtx& ioctx, + MockImageWatcher *watcher, + ContextWQ *work_queue, const std::string& oid, + const std::string& cookie, + bool blacklist_on_break_lock, + uint32_t blacklist_expire_seconds, + Context *on_finish) { + return BaseRequest::create(ioctx, watcher, work_queue, oid, cookie, on_finish); + } MOCK_METHOD0(send, void()); }; diff --git a/src/test/librbd/test_mock_ManagedLock.cc b/src/test/librbd/test_mock_ManagedLock.cc index cd38e58034a..6b7b5228e61 100644 --- a/src/test/librbd/test_mock_ManagedLock.cc +++ b/src/test/librbd/test_mock_ManagedLock.cc @@ -51,6 +51,16 @@ std::list BaseRequest::s_requests; template <> struct AcquireRequest : public BaseRequest > { + static AcquireRequest* create(librados::IoCtx& ioctx, + MockImageWatcher *watcher, + ContextWQ *work_queue, const std::string& oid, + const std::string& cookie, + bool blacklist_on_break_lock, + uint32_t blacklist_expire_seconds, + Context *on_finish) { + return BaseRequest::create(ioctx, watcher, work_queue, oid, cookie, on_finish); + } + MOCK_METHOD0(send, void()); }; @@ -174,7 +184,8 @@ TEST_F(TestMockManagedLock, StateTransitions) { MockManagedLockImageCtx mock_image_ctx(*ictx); MockManagedLock managed_lock(ictx->md_ctx, ictx->op_work_queue, - ictx->header_oid, mock_image_ctx.image_watcher); + ictx->header_oid, mock_image_ctx.image_watcher, + true, 0); InSequence seq; MockAcquireRequest request_lock_acquire1; @@ -204,8 +215,8 @@ TEST_F(TestMockManagedLock, AcquireLockLockedState) { MockManagedLockImageCtx mock_image_ctx(*ictx); MockManagedLock managed_lock(ictx->md_ctx, ictx->op_work_queue, - ictx->header_oid, mock_image_ctx.image_watcher); - + ictx->header_oid, mock_image_ctx.image_watcher, + true, 0); InSequence seq; MockAcquireRequest try_lock_acquire; @@ -224,8 +235,8 @@ TEST_F(TestMockManagedLock, AcquireLockAlreadyLocked) { MockManagedLockImageCtx mock_image_ctx(*ictx); MockManagedLock managed_lock(ictx->md_ctx, ictx->op_work_queue, - ictx->header_oid, mock_image_ctx.image_watcher); - + ictx->header_oid, mock_image_ctx.image_watcher, + true, 0); InSequence seq; MockAcquireRequest try_lock_acquire; @@ -242,8 +253,8 @@ TEST_F(TestMockManagedLock, AcquireLockBusy) { MockManagedLockImageCtx mock_image_ctx(*ictx); MockManagedLock managed_lock(ictx->md_ctx, ictx->op_work_queue, - ictx->header_oid, mock_image_ctx.image_watcher); - + ictx->header_oid, mock_image_ctx.image_watcher, + true, 0); InSequence seq; MockAcquireRequest try_lock_acquire; @@ -260,8 +271,8 @@ TEST_F(TestMockManagedLock, AcquireLockError) { MockManagedLockImageCtx mock_image_ctx(*ictx); MockManagedLock managed_lock(ictx->md_ctx, ictx->op_work_queue, - ictx->header_oid, mock_image_ctx.image_watcher); - + ictx->header_oid, mock_image_ctx.image_watcher, + true, 0); InSequence seq; MockAcquireRequest try_lock_acquire; @@ -279,8 +290,8 @@ TEST_F(TestMockManagedLock, AcquireLockBlacklist) { MockManagedLockImageCtx mock_image_ctx(*ictx); MockManagedLock managed_lock(ictx->md_ctx, ictx->op_work_queue, - ictx->header_oid, mock_image_ctx.image_watcher); - + ictx->header_oid, mock_image_ctx.image_watcher, + true, 0); InSequence seq; // will abort after seeing blacklist error (avoid infinite request loop) @@ -298,8 +309,8 @@ TEST_F(TestMockManagedLock, ReleaseLockUnlockedState) { MockManagedLockImageCtx mock_image_ctx(*ictx); MockManagedLock managed_lock(ictx->md_ctx, ictx->op_work_queue, - ictx->header_oid, mock_image_ctx.image_watcher); - + ictx->header_oid, mock_image_ctx.image_watcher, + true, 0); InSequence seq; ASSERT_EQ(0, when_release_lock(managed_lock)); @@ -313,8 +324,8 @@ TEST_F(TestMockManagedLock, ReleaseLockError) { MockManagedLockImageCtx mock_image_ctx(*ictx); MockManagedLock managed_lock(ictx->md_ctx, ictx->op_work_queue, - ictx->header_oid, mock_image_ctx.image_watcher); - + ictx->header_oid, mock_image_ctx.image_watcher, + true, 0); InSequence seq; MockAcquireRequest try_lock_acquire; @@ -339,8 +350,8 @@ TEST_F(TestMockManagedLock, ConcurrentRequests) { MockManagedLockImageCtx mock_image_ctx(*ictx); MockManagedLock managed_lock(ictx->md_ctx, ictx->op_work_queue, - ictx->header_oid, mock_image_ctx.image_watcher); - + ictx->header_oid, mock_image_ctx.image_watcher, + true, 0); InSequence seq; expect_get_watch_handle(*mock_image_ctx.image_watcher); @@ -396,8 +407,8 @@ TEST_F(TestMockManagedLock, ReacquireLock) { MockManagedLockImageCtx mock_image_ctx(*ictx); MockManagedLock managed_lock(ictx->md_ctx, ictx->op_work_queue, - ictx->header_oid, mock_image_ctx.image_watcher); - + ictx->header_oid, mock_image_ctx.image_watcher, + true, 0); InSequence seq; MockAcquireRequest request_lock_acquire; @@ -423,8 +434,8 @@ TEST_F(TestMockManagedLock, ReacquireLockError) { MockManagedLockImageCtx mock_image_ctx(*ictx); MockManagedLock managed_lock(ictx->md_ctx, ictx->op_work_queue, - ictx->header_oid, mock_image_ctx.image_watcher); - + ictx->header_oid, mock_image_ctx.image_watcher, + true, 0); InSequence seq; MockAcquireRequest request_lock_acquire;