From: Jason Dillaman Date: Tue, 8 Dec 2015 18:48:27 +0000 (-0500) Subject: librbd: decouple ImageWatcher from exclusive lock management X-Git-Tag: v10.0.2~35^2~14 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ccdeb151578321b17bd14ac12a97f93ea27f1054;p=ceph.git librbd: decouple ImageWatcher from exclusive lock management Use new ExclusiveLock state machine to handle all the proper transitions between lock states. Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/AioImageRequest.cc b/src/librbd/AioImageRequest.cc index 755779136e9..14c9dd12b25 100644 --- a/src/librbd/AioImageRequest.cc +++ b/src/librbd/AioImageRequest.cc @@ -4,6 +4,7 @@ #include "librbd/AioImageRequest.h" #include "librbd/AioCompletion.h" #include "librbd/AioObjectRequest.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" #include "librbd/internal.h" @@ -247,8 +248,8 @@ void AbstractAioImageWrite::send_request() { !m_image_ctx.journal->is_journal_replaying()); } - assert(!m_image_ctx.image_watcher->is_lock_supported() || - m_image_ctx.image_watcher->is_lock_owner()); + assert(m_image_ctx.exclusive_lock == nullptr || + m_image_ctx.exclusive_lock->is_lock_owner()); m_aio_comp->set_request_count( m_image_ctx.cct, object_extents.size() + diff --git a/src/librbd/AioImageRequestWQ.cc b/src/librbd/AioImageRequestWQ.cc index 8dbfd755536..ef09c2a9e18 100644 --- a/src/librbd/AioImageRequestWQ.cc +++ b/src/librbd/AioImageRequestWQ.cc @@ -4,6 +4,7 @@ #include "librbd/AioImageRequestWQ.h" #include "librbd/AioCompletion.h" #include "librbd/AioImageRequest.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/internal.h" @@ -17,9 +18,7 @@ AioImageRequestWQ::AioImageRequestWQ(ImageCtx *image_ctx, const string &name, time_t ti, ThreadPool *tp) : ThreadPool::PointerWQ(name, ti, 0, tp), m_image_ctx(*image_ctx), m_lock("AioImageRequestWQ::m_lock"), - m_write_blockers(0), m_in_progress_writes(0), m_queued_writes(0), - m_lock_listener(this), m_blocking_writes(false) { - + m_write_blockers(0), m_in_progress_writes(0), m_queued_writes(0) { CephContext *cct = m_image_ctx.cct; ldout(cct, 5) << this << " " << ": ictx=" << image_ctx << dendl; } @@ -207,10 +206,6 @@ void AioImageRequestWQ::unblock_writes() { } } -void AioImageRequestWQ::register_lock_listener() { - m_image_ctx.image_watcher->register_listener(&m_lock_listener); -} - void *AioImageRequestWQ::_void_dequeue() { AioImageRequest *peek_item = front(); if (peek_item == NULL) { @@ -271,12 +266,11 @@ bool AioImageRequestWQ::is_journal_required() const { bool AioImageRequestWQ::is_lock_required() const { assert(m_image_ctx.owner_lock.is_locked()); - if (m_image_ctx.image_watcher == NULL) { + if (m_image_ctx.exclusive_lock == NULL) { return false; } - return (m_image_ctx.image_watcher->is_lock_supported() && - !m_image_ctx.image_watcher->is_lock_owner()); + return (!m_image_ctx.exclusive_lock->is_lock_owner()); } void AioImageRequestWQ::queue(AioImageRequest *req) { @@ -286,44 +280,16 @@ void AioImageRequestWQ::queue(AioImageRequest *req) { assert(m_image_ctx.owner_lock.is_locked()); - bool first_write_op = false; { Mutex::Locker locker(m_lock); if (req->is_write_op()) { - if (++m_queued_writes == 1) { - first_write_op = true; - } + ++m_queued_writes; } } ThreadPool::PointerWQ::queue(req); - if (is_lock_required() && first_write_op) { - m_image_ctx.image_watcher->request_lock(); - } -} - -void AioImageRequestWQ::handle_lock_updated( - ImageWatcher::LockUpdateState state) { - assert(m_image_ctx.owner_lock.is_locked()); - - CephContext *cct = m_image_ctx.cct; - ldout(cct, 20) << __func__ << ": ictx=" << &m_image_ctx << ", " - << "state=" << state << dendl; - - if ((state == ImageWatcher::LOCK_UPDATE_STATE_NOT_SUPPORTED || - state == ImageWatcher::LOCK_UPDATE_STATE_LOCKED) && m_blocking_writes) { - m_blocking_writes = false; - unblock_writes(); - } else if (state == ImageWatcher::LOCK_UPDATE_STATE_RELEASING && - !m_blocking_writes) { - m_blocking_writes = true; - block_writes(); - } else if (state == ImageWatcher::LOCK_UPDATE_STATE_UNLOCKED) { - assert(m_blocking_writes); - assert(writes_blocked()); - } else if (state == ImageWatcher::LOCK_UPDATE_STATE_NOTIFICATION && - !writes_empty()) { - m_image_ctx.image_watcher->request_lock(); + if (is_lock_required()) { + m_image_ctx.exclusive_lock->request_lock(nullptr); } } diff --git a/src/librbd/AioImageRequestWQ.h b/src/librbd/AioImageRequestWQ.h index b0fb80505ee..bb323cbb627 100644 --- a/src/librbd/AioImageRequestWQ.h +++ b/src/librbd/AioImageRequestWQ.h @@ -7,7 +7,6 @@ #include "include/Context.h" #include "common/WorkQueue.h" #include "common/Mutex.h" -#include "librbd/ImageWatcher.h" namespace librbd { @@ -48,8 +47,6 @@ public: void block_writes(Context *on_blocked); void unblock_writes(); - void register_lock_listener(); - protected: virtual void *_void_dequeue(); virtual void process(AioImageRequest *req); @@ -57,20 +54,6 @@ protected: private: typedef std::list Contexts; - struct LockListener : public ImageWatcher::Listener { - AioImageRequestWQ *aio_work_queue; - LockListener(AioImageRequestWQ *_aio_work_queue) - : aio_work_queue(_aio_work_queue) { - } - - virtual bool handle_requested_lock() { - return true; - } - virtual void handle_lock_updated(ImageWatcher::LockUpdateState state) { - aio_work_queue->handle_lock_updated(state); - } - }; - struct C_BlockedWrites : public Context { AioImageRequestWQ *aio_work_queue; C_BlockedWrites(AioImageRequestWQ *_aio_work_queue) @@ -89,14 +72,10 @@ private: uint32_t m_in_progress_writes; uint32_t m_queued_writes; - LockListener m_lock_listener; - bool m_blocking_writes; - bool is_journal_required() const; bool is_lock_required() const; void queue(AioImageRequest *req); - void handle_lock_updated(ImageWatcher::LockUpdateState state); void handle_blocked_writes(int r); }; diff --git a/src/librbd/AioObjectRequest.cc b/src/librbd/AioObjectRequest.cc index aef9870de96..d0805fbfdb5 100644 --- a/src/librbd/AioObjectRequest.cc +++ b/src/librbd/AioObjectRequest.cc @@ -9,6 +9,7 @@ #include "librbd/AioCompletion.h" #include "librbd/AioImageRequest.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" #include "librbd/internal.h" @@ -387,7 +388,7 @@ namespace librbd { write = true; } else { // should have been flushed prior to releasing lock - assert(m_ictx->image_watcher->is_lock_owner()); + assert(m_ictx->exclusive_lock->is_lock_owner()); ldout(m_ictx->cct, 20) << "send_pre " << this << " " << m_oid << " " << m_object_off << "~" << m_object_len << dendl; @@ -424,7 +425,7 @@ namespace librbd { } // should have been flushed prior to releasing lock - assert(m_ictx->image_watcher->is_lock_owner()); + assert(m_ictx->exclusive_lock->is_lock_owner()); ldout(m_ictx->cct, 20) << "send_post " << this << " " << m_oid << " " << m_object_off << "~" << m_object_len << dendl; diff --git a/src/librbd/CopyupRequest.cc b/src/librbd/CopyupRequest.cc index 91e997f2494..736b1d51c0a 100644 --- a/src/librbd/CopyupRequest.cc +++ b/src/librbd/CopyupRequest.cc @@ -11,6 +11,7 @@ #include "librbd/AioObjectRequest.h" #include "librbd/AsyncObjectThrottle.h" #include "librbd/CopyupRequest.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" #include "librbd/internal.h" @@ -45,7 +46,7 @@ public: if (snap_id == CEPH_NOSNAP) { RWLock::RLocker snap_locker(m_image_ctx.snap_lock); RWLock::WLocker object_map_locker(m_image_ctx.object_map_lock); - assert(m_image_ctx.image_watcher->is_lock_owner()); + assert(m_image_ctx.exclusive_lock->is_lock_owner()); bool sent = m_image_ctx.object_map.aio_update(m_object_no, OBJECT_EXISTS, boost::optional(), this); @@ -271,7 +272,7 @@ private: RWLock::RLocker snap_locker(m_ictx->snap_lock); if (m_ictx->object_map.enabled()) { bool copy_on_read = m_pending_requests.empty(); - if (!m_ictx->image_watcher->is_lock_owner()) { + if (!m_ictx->exclusive_lock->is_lock_owner()) { ldout(m_ictx->cct, 20) << "exclusive lock not held for copyup request" << dendl; assert(copy_on_read); diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc index 9131223869f..2dbfd669cdf 100644 --- a/src/librbd/ExclusiveLock.cc +++ b/src/librbd/ExclusiveLock.cc @@ -2,6 +2,7 @@ // vim: ts=8 sw=2 smarttab #include "librbd/ExclusiveLock.h" +#include "cls/lock/cls_lock_client.h" #include "common/dout.h" #include "common/errno.h" #include "librbd/AioImageRequestWQ.h" @@ -43,30 +44,42 @@ ExclusiveLock::~ExclusiveLock() { template bool ExclusiveLock::is_lock_owner() const { - ldout(m_image_ctx.cct, 20) << __func__ << dendl; - assert(m_image_ctx.owner_lock.is_locked()); - Mutex::Locker locker(m_lock); - return (m_state == STATE_LOCKED); + + bool lock_owner; + switch (m_state) { + case STATE_LOCKED: + case STATE_POST_ACQUIRING: + case STATE_PRE_RELEASING: + lock_owner = true; + break; + default: + lock_owner = false; + break; + } + + ldout(m_image_ctx.cct, 20) << this << " " << __func__ << "=" << lock_owner + << dendl; + return lock_owner; } template void ExclusiveLock::init(Context *on_init) { - ldout(m_image_ctx.cct, 10) << __func__ << dendl; - - assert(m_image_ctx.owner_lock.is_wlocked()); + assert(m_image_ctx.owner_lock.is_locked()); + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; - Mutex::Locker locker(m_lock); - assert(m_state == STATE_UNINITIALIZED); - m_state = STATE_INITIALIZING; + { + Mutex::Locker locker(m_lock); + assert(m_state == STATE_UNINITIALIZED); + m_state = STATE_INITIALIZING; + } m_image_ctx.aio_work_queue->block_writes(new C_InitComplete(this, on_init)); } template void ExclusiveLock::shut_down(Context *on_shut_down) { - ldout(m_image_ctx.cct, 10) << __func__ << dendl; - assert(m_image_ctx.owner_lock.is_wlocked()); + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; Mutex::Locker locker(m_lock); assert(!is_shutdown()); @@ -81,7 +94,7 @@ void ExclusiveLock::try_lock(Context *on_tried_lock) { assert(!is_shutdown()); if (m_state != STATE_LOCKED || !m_actions_contexts.empty()) { - ldout(m_image_ctx.cct, 10) << __func__ << dendl; + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; execute_action(ACTION_TRY_LOCK, on_tried_lock); return; } @@ -94,11 +107,10 @@ template void ExclusiveLock::request_lock(Context *on_locked) { { Mutex::Locker locker(m_lock); + assert(m_image_ctx.owner_lock.is_locked()); assert(!is_shutdown()); - assert(m_image_ctx.owner_lock.is_wlocked()); - if (m_state != STATE_LOCKED || !m_actions_contexts.empty()) { - ldout(m_image_ctx.cct, 10) << __func__ << dendl; + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; execute_action(ACTION_REQUEST_LOCK, on_locked); return; } @@ -113,11 +125,11 @@ template void ExclusiveLock::release_lock(Context *on_released) { { Mutex::Locker locker(m_lock); - assert(m_image_ctx.owner_lock.is_wlocked()); + assert(m_image_ctx.owner_lock.is_locked()); assert(!is_shutdown()); if (m_state != STATE_UNLOCKED || !m_actions_contexts.empty()) { - ldout(m_image_ctx.cct, 10) << __func__ << dendl; + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; execute_action(ACTION_RELEASE_LOCK, on_released); return; } @@ -133,16 +145,16 @@ void ExclusiveLock::handle_lock_released() { return; } - ldout(m_image_ctx.cct, 10) << __func__ << dendl; + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; assert(get_active_action() == ACTION_REQUEST_LOCK); execute_next_action(); } template -void ExclusiveLock::set_watch_handle(uint64_t watch_handle) { +void ExclusiveLock::assert_header_locked(librados::ObjectWriteOperation *op) { Mutex::Locker locker(m_lock); - assert(m_watch_handle == 0 || watch_handle == 0); - m_watch_handle = watch_handle; + rados::cls::lock::assert_locked(op, RBD_LOCK_NAME, LOCK_EXCLUSIVE, + encode_lock_cookie(), WATCHER_LOCK_TAG); } template @@ -172,6 +184,8 @@ bool ExclusiveLock::is_transition_state() const { case STATE_INITIALIZING: case STATE_ACQUIRING: case STATE_WAITING_FOR_PEER: + case STATE_POST_ACQUIRING: + case STATE_PRE_RELEASING: case STATE_RELEASING: case STATE_SHUTTING_DOWN: return true; @@ -257,7 +271,7 @@ void ExclusiveLock::complete_active_action(State next_state, int r) { } m_lock.Lock(); - if (!m_actions_contexts.empty()) { + if (!is_transition_state() && !m_actions_contexts.empty()) { execute_next_action(); } } @@ -273,7 +287,7 @@ bool ExclusiveLock::is_shutdown() const { template void ExclusiveLock::handle_init_complete() { - ldout(m_image_ctx.cct, 10) << __func__ << dendl; + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; Mutex::Locker locker(m_lock); m_state = STATE_UNLOCKED; @@ -287,20 +301,25 @@ void ExclusiveLock::send_acquire_lock() { return; } - ldout(m_image_ctx.cct, 10) << __func__ << dendl; + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; m_state = STATE_ACQUIRING; + m_watch_handle = m_image_ctx.image_watcher->get_watch_handle(); + using el = ExclusiveLock; AcquireRequest* req = AcquireRequest::create( m_image_ctx, encode_lock_cookie(), util::create_context_callback(this)); + + m_lock.Unlock(); req->send(); + m_lock.Lock(); } template void ExclusiveLock::handle_acquire_lock(int r) { CephContext *cct = m_image_ctx.cct; - ldout(cct, 10) << __func__ << ": r=" << r << dendl; + ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl; if (r == -EBUSY) { ldout(cct, 5) << "unable to acquire exclusive lock" << dendl; @@ -331,10 +350,16 @@ void ExclusiveLock::handle_acquire_lock(int r) { r = 0; } - Mutex::Locker locker(m_lock); if (next_state == STATE_LOCKED) { + { + Mutex::Locker locker(m_lock); + m_state = STATE_POST_ACQUIRING; + } + m_image_ctx.image_watcher->notify_acquired_lock(); m_image_ctx.aio_work_queue->unblock_writes(); } + + Mutex::Locker locker(m_lock); complete_active_action(next_state, r); } @@ -346,8 +371,8 @@ void ExclusiveLock::send_release_lock() { return; } - ldout(m_image_ctx.cct, 10) << __func__ << dendl; - m_state = STATE_RELEASING; + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; + m_state = STATE_PRE_RELEASING; m_image_ctx.op_work_queue->queue( new C_BlockWrites(m_image_ctx, new C_ReleaseBlockWrites(this)), 0); @@ -360,28 +385,49 @@ void ExclusiveLock::handle_release_blocked_writes(int r) { return; } - ldout(m_image_ctx.cct, 10) << __func__ << ": r=" << r << dendl; + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << ": r=" << r << dendl; Mutex::Locker locker(m_lock); - assert(m_state == STATE_RELEASING); + assert(m_state == STATE_PRE_RELEASING); + m_state = STATE_RELEASING; using el = ExclusiveLock; ReleaseRequest* req = ReleaseRequest::create( m_image_ctx, encode_lock_cookie(), util::create_context_callback(this)); + + m_lock.Unlock(); req->send(); + m_lock.Lock(); } template void ExclusiveLock::handle_release_lock(int r) { - Mutex::Locker locker(m_lock); + bool pending_writes = false; + { + Mutex::Locker locker(m_lock); + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << ": r=" << r + << dendl; - ldout(m_image_ctx.cct, 10) << __func__ << ": r=" << r << dendl; + assert(m_state == STATE_RELEASING); + if (r < 0) { + m_image_ctx.aio_work_queue->unblock_writes(); + } else { + m_lock.Unlock(); + m_image_ctx.image_watcher->notify_released_lock(); + pending_writes = !m_image_ctx.aio_work_queue->writes_empty(); + m_lock.Lock(); - if (r < 0) { - m_image_ctx.aio_work_queue->unblock_writes(); + m_watch_handle = 0; + } + complete_active_action(r < 0 ? STATE_LOCKED : STATE_UNLOCKED, r); + } + + if (r >= 0 && pending_writes) { + // if we have pending writes -- re-request the lock + RWLock::RLocker owner_locker(m_image_ctx.owner_lock); + request_lock(nullptr); } - complete_active_action(r < 0 ? STATE_LOCKED : STATE_UNLOCKED, r); } template @@ -389,27 +435,66 @@ void ExclusiveLock::send_shutdown() { assert(m_lock.is_locked()); if (m_state == STATE_UNLOCKED) { m_image_ctx.aio_work_queue->unblock_writes(); - complete_active_action(STATE_SHUTDOWN, 0); + m_image_ctx.op_work_queue->queue(util::create_context_callback< + ExclusiveLock, &ExclusiveLock::complete_shutdown>(this), 0); return; } - ldout(m_image_ctx.cct, 10) << __func__ << dendl; + ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl; assert(m_state == STATE_LOCKED); m_state = STATE_SHUTTING_DOWN; + m_lock.Unlock(); + m_image_ctx.op_work_queue->queue(new C_ShutDownRelease(this), 0); + m_lock.Lock(); +} + +template +void ExclusiveLock::send_shutdown_release() { + std::string cookie; + { + Mutex::Locker locker(m_lock); + cookie = encode_lock_cookie(); + } + using el = ExclusiveLock; ReleaseRequest* req = ReleaseRequest::create( - m_image_ctx, encode_lock_cookie(), + m_image_ctx, cookie, util::create_context_callback(this)); req->send(); } template void ExclusiveLock::handle_shutdown(int r) { - Mutex::Locker locker(m_lock); - ldout(m_image_ctx.cct, 10) << __func__ << ": r=" << r << dendl; + CephContext *cct = m_image_ctx.cct; + ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl; - complete_active_action(r == 0 ? STATE_SHUTDOWN : STATE_LOCKED, r); + if (r < 0) { + lderr(cct) << "failed to shut down exclusive lock: " << cpp_strerror(r) + << dendl; + } + + m_image_ctx.image_watcher->notify_released_lock(); + complete_shutdown(r); +} + +template +void ExclusiveLock::complete_shutdown(int r) { + ActionContexts action_contexts; + { + Mutex::Locker locker(m_lock); + assert(m_lock.is_locked()); + assert(m_actions_contexts.size() == 1); + + action_contexts = std::move(m_actions_contexts.front()); + m_actions_contexts.pop_front(); + m_state = STATE_SHUTDOWN; + } + + // expect to be destroyed after firing callback + for (auto ctx : action_contexts.second) { + ctx->complete(r); + } } } // namespace librbd diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h index e849de0ec18..4434b615a12 100644 --- a/src/librbd/ExclusiveLock.h +++ b/src/librbd/ExclusiveLock.h @@ -6,6 +6,7 @@ #include "include/int_types.h" #include "include/Context.h" +#include "include/rados/librados.hpp" #include "common/Mutex.h" #include "common/RWLock.h" #include @@ -39,7 +40,8 @@ public: void handle_lock_released(); - void set_watch_handle(uint64_t watch_handle); + void assert_header_locked(librados::ObjectWriteOperation *op); + static bool decode_lock_cookie(const std::string &cookie, uint64_t *handle); private: @@ -53,9 +55,12 @@ private: * v (init) (try_lock/request_lock) * | * UNINITIALIZED -------> UNLOCKED ------------------------> ACQUIRING <--/ * ^ | + * | v + * RELEASING POST_ACQUIRING + * | | * | | * | (release_lock) v - * RELEASING <------------------------- LOCKED + * PRE_RELEASING <------------------------ LOCKED * * * | @@ -69,7 +74,9 @@ private: STATE_LOCKED, STATE_INITIALIZING, STATE_ACQUIRING, + STATE_POST_ACQUIRING, STATE_WAITING_FOR_PEER, + STATE_PRE_RELEASING, STATE_RELEASING, STATE_SHUTTING_DOWN, STATE_SHUTDOWN, @@ -122,6 +129,16 @@ private: } }; + struct C_ShutDownRelease : public Context { + ExclusiveLock *exclusive_lock; + C_ShutDownRelease(ExclusiveLock *exclusive_lock) + : exclusive_lock(exclusive_lock) { + } + virtual void finish(int r) override { + exclusive_lock->send_shutdown_release(); + } + }; + ImageCtxT &m_image_ctx; mutable Mutex m_lock; @@ -153,7 +170,9 @@ private: void handle_release_lock(int r); void send_shutdown(); + void send_shutdown_release(); void handle_shutdown(int r); + void complete_shutdown(int r); }; } // namespace librbd diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc index f37c8e87d90..959f8977609 100644 --- a/src/librbd/ImageCtx.cc +++ b/src/librbd/ImageCtx.cc @@ -886,7 +886,6 @@ struct C_InvalidateCache : public Context { int ImageCtx::register_watch() { assert(image_watcher == NULL); image_watcher = new ImageWatcher(*this); - aio_work_queue->register_lock_listener(); return image_watcher->register_watch(); } diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc index 8ffd751930f..15a7616c9ac 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -2,13 +2,12 @@ // vim: ts=8 sw=2 smarttab #include "librbd/ImageWatcher.h" #include "librbd/AioCompletion.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/internal.h" #include "librbd/ObjectMap.h" #include "librbd/TaskFinisher.h" #include "librbd/Utils.h" -#include "cls/lock/cls_lock_client.h" -#include "cls/lock/cls_lock_types.h" #include "include/encoding.h" #include "include/stringify.h" #include "common/errno.h" @@ -25,9 +24,6 @@ namespace librbd { using namespace watch_notify; -static const std::string WATCHER_LOCK_TAG = "internal"; -static const std::string WATCHER_LOCK_COOKIE_PREFIX = "auto"; - static const uint64_t NOTIFY_TIMEOUT = 5000; static const double RETRY_DELAY_SECONDS = 1.0; @@ -36,10 +32,6 @@ ImageWatcher::ImageWatcher(ImageCtx &image_ctx) m_watch_lock(util::unique_lock_name("librbd::ImageWatcher::m_watch_lock", this)), m_watch_ctx(*this), m_watch_handle(0), m_watch_state(WATCH_STATE_UNREGISTERED), - m_refresh_lock(util::unique_lock_name("librbd::ImageWatcher::m_refresh_lock", this)), - m_lock_supported(false), m_lock_owner_state(LOCK_OWNER_STATE_NOT_LOCKED), - m_listeners_lock(util::unique_lock_name("librbd::ImageWatcher::m_listeners_lock", this)), - m_listeners_in_use(false), m_task_finisher(new TaskFinisher(*m_image_ctx.cct)), m_async_request_lock(util::unique_lock_name("librbd::ImageWatcher::m_async_request_lock", this)), m_owner_client_id_lock(util::unique_lock_name("librbd::ImageWatcher::m_owner_client_id_lock", this)) @@ -53,42 +45,6 @@ ImageWatcher::~ImageWatcher() RWLock::RLocker l(m_watch_lock); assert(m_watch_state != WATCH_STATE_REGISTERED); } - { - RWLock::RLocker l(m_image_ctx.owner_lock); - assert(m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED); - } -} - -bool ImageWatcher::is_lock_supported() const { - RWLock::RLocker l(m_image_ctx.snap_lock); - return is_lock_supported(m_image_ctx.snap_lock); -} - -bool ImageWatcher::is_lock_supported(const RWLock &) const { - assert(m_image_ctx.owner_lock.is_locked()); - assert(m_image_ctx.snap_lock.is_locked()); - return ((m_image_ctx.features & RBD_FEATURE_EXCLUSIVE_LOCK) != 0 && - !m_image_ctx.read_only && m_image_ctx.snap_id == CEPH_NOSNAP); -} - -bool ImageWatcher::is_lock_owner() const { - assert(m_image_ctx.owner_lock.is_locked()); - return (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED || - m_lock_owner_state == LOCK_OWNER_STATE_RELEASING); -} - -void ImageWatcher::register_listener(Listener *listener) { - Mutex::Locker listeners_locker(m_listeners_lock); - m_listeners.push_back(listener); -} - -void ImageWatcher::unregister_listener(Listener *listener) { - // TODO CoW listener list - Mutex::Locker listeners_locker(m_listeners_lock); - while (m_listeners_in_use) { - m_listeners_cond.Wait(m_listeners_lock); - } - m_listeners.remove(listener); } int ImageWatcher::register_watch() { @@ -127,325 +83,6 @@ int ImageWatcher::unregister_watch() { return r; } -int ImageWatcher::refresh() { - assert(m_image_ctx.owner_lock.is_locked()); - - bool lock_support_changed = false; - { - Mutex::Locker refresh_locker(m_refresh_lock); - if (m_lock_supported != is_lock_supported()) { - m_lock_supported = is_lock_supported(); - lock_support_changed = true; - } - } - - int r = 0; - if (lock_support_changed) { - if (is_lock_supported()) { - // image opened, exclusive lock dynamically enabled, or now HEAD - notify_listeners_updated_lock(LOCK_UPDATE_STATE_RELEASING); - notify_listeners_updated_lock(LOCK_UPDATE_STATE_UNLOCKED); - } else if (!is_lock_supported()) { - if (is_lock_owner()) { - // exclusive lock dynamically disabled or now snapshot - m_image_ctx.owner_lock.put_read(); - { - RWLock::WLocker owner_locker(m_image_ctx.owner_lock); - r = release_lock(); - } - m_image_ctx.owner_lock.get_read(); - } - notify_listeners_updated_lock(LOCK_UPDATE_STATE_NOT_SUPPORTED); - } - } - return r; -} - -int ImageWatcher::try_lock() { - assert(m_image_ctx.owner_lock.is_wlocked()); - assert(m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED); - assert(is_lock_supported()); - - while (true) { - int r = lock(); - if (r != -EBUSY) { - return r; - } - - // determine if the current lock holder is still alive - entity_name_t locker; - std::string locker_cookie; - std::string locker_address; - uint64_t locker_handle; - r = get_lock_owner_info(&locker, &locker_cookie, &locker_address, - &locker_handle); - if (r < 0) { - return r; - } - if (locker_cookie.empty() || locker_address.empty()) { - // lock is now unlocked ... try again - continue; - } - - std::list watchers; - r = m_image_ctx.md_ctx.list_watchers(m_image_ctx.header_oid, &watchers); - if (r < 0) { - return r; - } - - for (std::list::iterator iter = watchers.begin(); - iter != watchers.end(); ++iter) { - if ((strncmp(locker_address.c_str(), - iter->addr, sizeof(iter->addr)) == 0) && - (locker_handle == iter->cookie)) { - Mutex::Locker l(m_owner_client_id_lock); - set_owner_client_id(ClientId(iter->watcher_id, locker_handle)); - return 0; - } - } - - if (m_image_ctx.blacklist_on_break_lock) { - ldout(m_image_ctx.cct, 1) << this << " blacklisting client: " << locker - << "@" << locker_address << dendl; - librados::Rados rados(m_image_ctx.md_ctx); - r = rados.blacklist_add(locker_address, - m_image_ctx.blacklist_expire_seconds); - if (r < 0) { - lderr(m_image_ctx.cct) << this << " unable to blacklist client: " - << cpp_strerror(r) << dendl; - return r; - } - } - - ldout(m_image_ctx.cct, 5) << this << " breaking exclusive lock: " << locker - << dendl; - r = rados::cls::lock::break_lock(&m_image_ctx.md_ctx, - m_image_ctx.header_oid, RBD_LOCK_NAME, - locker_cookie, locker); - if (r < 0 && r != -ENOENT) { - return r; - } - } - return 0; -} - -void ImageWatcher::request_lock() { - schedule_request_lock(false); -} - -bool ImageWatcher::try_request_lock() { - assert(m_image_ctx.owner_lock.is_locked()); - if (is_lock_owner()) { - return true; - } - - int r = 0; - m_image_ctx.owner_lock.put_read(); - { - RWLock::WLocker l(m_image_ctx.owner_lock); - if (!is_lock_owner()) { - r = try_lock(); - } - } - m_image_ctx.owner_lock.get_read(); - - if (r < 0) { - ldout(m_image_ctx.cct, 5) << this << " failed to acquire exclusive lock:" - << cpp_strerror(r) << dendl; - return false; - } - - if (is_lock_owner()) { - ldout(m_image_ctx.cct, 15) << this << " successfully acquired exclusive lock" - << dendl; - } else { - ldout(m_image_ctx.cct, 15) << this - << " unable to acquire exclusive lock, retrying" - << dendl; - } - return is_lock_owner(); -} - -int ImageWatcher::get_lock_owner_info(entity_name_t *locker, std::string *cookie, - std::string *address, uint64_t *handle) { - std::map lockers; - ClsLockType lock_type; - std::string lock_tag; - int r = rados::cls::lock::get_lock_info(&m_image_ctx.md_ctx, - m_image_ctx.header_oid, - RBD_LOCK_NAME, &lockers, &lock_type, - &lock_tag); - if (r < 0) { - return r; - } - - if (lockers.empty()) { - ldout(m_image_ctx.cct, 20) << this << " no lockers detected" << dendl; - return 0; - } - - if (lock_tag != WATCHER_LOCK_TAG) { - ldout(m_image_ctx.cct, 5) << this << " locked by external mechanism: tag=" - << lock_tag << dendl; - return -EBUSY; - } - - if (lock_type == LOCK_SHARED) { - ldout(m_image_ctx.cct, 5) << this << " shared lock type detected" << dendl; - return -EBUSY; - } - - std::map::iterator iter = lockers.begin(); - if (!decode_lock_cookie(iter->first.cookie, handle)) { - ldout(m_image_ctx.cct, 5) << this << " locked by external mechanism: " - << "cookie=" << iter->first.cookie << dendl; - return -EBUSY; - } - - *locker = iter->first.locker; - *cookie = iter->first.cookie; - *address = stringify(iter->second.addr); - ldout(m_image_ctx.cct, 10) << this << " retrieved exclusive locker: " - << *locker << "@" << *address << dendl; - return 0; -} - -int ImageWatcher::lock() { - assert(m_image_ctx.owner_lock.is_wlocked()); - assert(m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED); - - int r = rados::cls::lock::lock(&m_image_ctx.md_ctx, m_image_ctx.header_oid, - RBD_LOCK_NAME, LOCK_EXCLUSIVE, - encode_lock_cookie(), WATCHER_LOCK_TAG, "", - utime_t(), 0); - if (r < 0) { - return r; - } - - ldout(m_image_ctx.cct, 10) << this << " acquired exclusive lock" << dendl; - m_lock_owner_state = LOCK_OWNER_STATE_LOCKED; - - ClientId owner_client_id = get_client_id(); - { - Mutex::Locker l(m_owner_client_id_lock); - set_owner_client_id(owner_client_id); - } - - if (m_image_ctx.object_map.enabled()) { - r = m_image_ctx.object_map.lock(); - if (r < 0 && r != -ENOENT) { - unlock(); - return r; - } - RWLock::WLocker l2(m_image_ctx.snap_lock); - m_image_ctx.object_map.refresh(CEPH_NOSNAP); - } - - // send the notification when we aren't holding locks - FunctionContext *ctx = new FunctionContext( - boost::bind(&ImageWatcher::notify_acquired_lock, this)); - m_task_finisher->queue(TASK_CODE_ACQUIRED_LOCK, ctx); - return 0; -} - -int ImageWatcher::unlock() -{ - assert(m_image_ctx.owner_lock.is_wlocked()); - - ldout(m_image_ctx.cct, 10) << this << " releasing exclusive lock" << dendl; - m_lock_owner_state = LOCK_OWNER_STATE_NOT_LOCKED; - int r = rados::cls::lock::unlock(&m_image_ctx.md_ctx, m_image_ctx.header_oid, - RBD_LOCK_NAME, encode_lock_cookie()); - if (r < 0 && r != -ENOENT) { - lderr(m_image_ctx.cct) << this << " failed to release exclusive lock: " - << cpp_strerror(r) << dendl; - return r; - } - - if (m_image_ctx.object_map.enabled()) { - m_image_ctx.object_map.unlock(); - } - - { - Mutex::Locker l(m_owner_client_id_lock); - set_owner_client_id(ClientId()); - } - - FunctionContext *ctx = new FunctionContext( - boost::bind(&ImageWatcher::notify_released_lock, this)); - m_task_finisher->queue(TASK_CODE_RELEASED_LOCK, ctx); - return 0; -} - -int ImageWatcher::release_lock() -{ - assert(m_image_ctx.owner_lock.is_wlocked()); - - CephContext *cct = m_image_ctx.cct; - ldout(cct, 10) << this << " releasing exclusive lock by request" << dendl; - if (m_lock_owner_state != LOCK_OWNER_STATE_LOCKED) { - return 0; - } - - m_lock_owner_state = LOCK_OWNER_STATE_RELEASING; - m_image_ctx.owner_lock.put_write(); - - // ensure all maint operations are canceled - m_image_ctx.cancel_async_requests(); - m_image_ctx.flush_async_operations(); - - int r; - { - RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - - // alert listeners that all incoming IO needs to be stopped since the - // lock is being released - notify_listeners_updated_lock(LOCK_UPDATE_STATE_RELEASING); - - RWLock::WLocker md_locker(m_image_ctx.md_lock); - r = m_image_ctx.flush(); - if (r < 0) { - lderr(cct) << this << " failed to flush: " << cpp_strerror(r) << dendl; - goto err_cancel_unlock; - } - } - - m_image_ctx.owner_lock.get_write(); - assert(m_lock_owner_state == LOCK_OWNER_STATE_RELEASING); - r = unlock(); - - // notify listeners of the change w/ owner read locked - m_image_ctx.owner_lock.put_write(); - { - RWLock::RLocker owner_lock(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED) { - notify_listeners_updated_lock(LOCK_UPDATE_STATE_UNLOCKED); - } - } - m_image_ctx.owner_lock.get_write(); - - if (r < 0) { - lderr(cct) << this << " failed to unlock: " << cpp_strerror(r) << dendl; - return r; - } - - return 0; - -err_cancel_unlock: - m_image_ctx.owner_lock.get_write(); - if (m_lock_owner_state == LOCK_OWNER_STATE_RELEASING) { - m_lock_owner_state = LOCK_OWNER_STATE_LOCKED; - } - return r; -} - -void ImageWatcher::assert_header_locked(librados::ObjectWriteOperation *op) { - rados::cls::lock::assert_locked(op, RBD_LOCK_NAME, LOCK_EXCLUSIVE, - encode_lock_cookie(), WATCHER_LOCK_TAG); -} - void ImageWatcher::schedule_async_progress(const AsyncRequestId &request, uint64_t offset, uint64_t total) { FunctionContext *ctx = new FunctionContext( @@ -503,7 +140,8 @@ int ImageWatcher::notify_async_complete(const AsyncRequestId &request, int ImageWatcher::notify_flatten(uint64_t request_id, ProgressContext &prog_ctx) { assert(m_image_ctx.owner_lock.is_locked()); - assert(!is_lock_owner()); + assert(m_image_ctx.exclusive_lock && + !m_image_ctx.exclusive_lock->is_lock_owner()); AsyncRequestId async_request_id(get_client_id(), request_id); @@ -516,7 +154,8 @@ int ImageWatcher::notify_flatten(uint64_t request_id, ProgressContext &prog_ctx) int ImageWatcher::notify_resize(uint64_t request_id, uint64_t size, ProgressContext &prog_ctx) { assert(m_image_ctx.owner_lock.is_locked()); - assert(!is_lock_owner()); + assert(m_image_ctx.exclusive_lock && + !m_image_ctx.exclusive_lock->is_lock_owner()); AsyncRequestId async_request_id(get_client_id(), request_id); @@ -528,7 +167,8 @@ int ImageWatcher::notify_resize(uint64_t request_id, uint64_t size, int ImageWatcher::notify_snap_create(const std::string &snap_name) { assert(m_image_ctx.owner_lock.is_locked()); - assert(!is_lock_owner()); + assert(m_image_ctx.exclusive_lock && + !m_image_ctx.exclusive_lock->is_lock_owner()); bufferlist bl; ::encode(NotifyMessage(SnapCreatePayload(snap_name)), bl); @@ -539,7 +179,8 @@ int ImageWatcher::notify_snap_create(const std::string &snap_name) { int ImageWatcher::notify_snap_rename(const snapid_t &src_snap_id, const std::string &dst_snap_name) { assert(m_image_ctx.owner_lock.is_locked()); - assert(!is_lock_owner()); + assert(m_image_ctx.exclusive_lock && + !m_image_ctx.exclusive_lock->is_lock_owner()); bufferlist bl; ::encode(NotifyMessage(SnapRenamePayload(src_snap_id, dst_snap_name)), bl); @@ -548,7 +189,8 @@ int ImageWatcher::notify_snap_rename(const snapid_t &src_snap_id, } int ImageWatcher::notify_snap_remove(const std::string &snap_name) { assert(m_image_ctx.owner_lock.is_locked()); - assert(!is_lock_owner()); + assert(m_image_ctx.exclusive_lock && + !m_image_ctx.exclusive_lock->is_lock_owner()); bufferlist bl; ::encode(NotifyMessage(SnapRemovePayload(snap_name)), bl); @@ -558,7 +200,8 @@ int ImageWatcher::notify_snap_remove(const std::string &snap_name) { int ImageWatcher::notify_snap_protect(const std::string &snap_name) { assert(m_image_ctx.owner_lock.is_locked()); - assert(!is_lock_owner()); + assert(m_image_ctx.exclusive_lock && + !m_image_ctx.exclusive_lock->is_lock_owner()); bufferlist bl; ::encode(NotifyMessage(SnapProtectPayload(snap_name)), bl); @@ -567,7 +210,8 @@ int ImageWatcher::notify_snap_protect(const std::string &snap_name) { int ImageWatcher::notify_snap_unprotect(const std::string &snap_name) { assert(m_image_ctx.owner_lock.is_locked()); - assert(!is_lock_owner()); + assert(m_image_ctx.exclusive_lock && + !m_image_ctx.exclusive_lock->is_lock_owner()); bufferlist bl; ::encode(NotifyMessage(SnapUnprotectPayload(snap_name)), bl); @@ -577,7 +221,8 @@ int ImageWatcher::notify_snap_unprotect(const std::string &snap_name) { int ImageWatcher::notify_rebuild_object_map(uint64_t request_id, ProgressContext &prog_ctx) { assert(m_image_ctx.owner_lock.is_locked()); - assert(!is_lock_owner()); + assert(m_image_ctx.exclusive_lock && + !m_image_ctx.exclusive_lock->is_lock_owner()); AsyncRequestId async_request_id(get_client_id(), request_id); @@ -589,28 +234,14 @@ int ImageWatcher::notify_rebuild_object_map(uint64_t request_id, int ImageWatcher::notify_rename(const std::string &image_name) { assert(m_image_ctx.owner_lock.is_locked()); - assert(!is_lock_owner()); + assert(m_image_ctx.exclusive_lock && + !m_image_ctx.exclusive_lock->is_lock_owner()); bufferlist bl; ::encode(NotifyMessage(RenamePayload(image_name)), bl); return notify_lock_owner(bl); } -void ImageWatcher::notify_lock_state() { - RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { - // re-send the acquired lock notification so that peers know they can now - // request the lock - ldout(m_image_ctx.cct, 10) << this << " notify lock state" << dendl; - - bufferlist bl; - ::encode(NotifyMessage(AcquiredLockPayload(get_client_id())), bl); - - m_image_ctx.md_ctx.notify2(m_image_ctx.header_oid, bl, NOTIFY_TIMEOUT, - NULL); - } -} - void ImageWatcher::notify_header_update(librados::IoCtx &io_ctx, const std::string &oid) { @@ -621,23 +252,6 @@ void ImageWatcher::notify_header_update(librados::IoCtx &io_ctx, io_ctx.notify2(oid, bl, NOTIFY_TIMEOUT, NULL); } -std::string ImageWatcher::encode_lock_cookie() const { - RWLock::RLocker l(m_watch_lock); - std::ostringstream ss; - ss << WATCHER_LOCK_COOKIE_PREFIX << " " << m_watch_handle; - return ss.str(); -} - -bool ImageWatcher::decode_lock_cookie(const std::string &tag, - uint64_t *handle) { - std::string prefix; - std::istringstream ss(tag); - if (!(ss >> prefix >> *handle) || prefix != WATCHER_LOCK_COOKIE_PREFIX) { - return false; - } - return true; -} - void ImageWatcher::schedule_cancel_async_requests() { FunctionContext *ctx = new FunctionContext( boost::bind(&ImageWatcher::cancel_async_requests, this)); @@ -669,25 +283,25 @@ ClientId ImageWatcher::get_client_id() { void ImageWatcher::notify_acquired_lock() { ldout(m_image_ctx.cct, 10) << this << " notify acquired lock" << dendl; - RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - if (m_lock_owner_state != LOCK_OWNER_STATE_LOCKED) { - return; + ClientId client_id = get_client_id(); + { + Mutex::Locker owner_client_id_locker(m_owner_client_id_lock); + set_owner_client_id(client_id); } - notify_listeners_updated_lock(LOCK_UPDATE_STATE_LOCKED); - bufferlist bl; - ::encode(NotifyMessage(AcquiredLockPayload(get_client_id())), bl); + ::encode(NotifyMessage(AcquiredLockPayload(client_id)), bl); m_image_ctx.md_ctx.notify2(m_image_ctx.header_oid, bl, NOTIFY_TIMEOUT, NULL); } -void ImageWatcher::notify_release_lock() { - RWLock::WLocker owner_locker(m_image_ctx.owner_lock); - release_lock(); -} - void ImageWatcher::notify_released_lock() { ldout(m_image_ctx.cct, 10) << this << " notify released lock" << dendl; + + { + Mutex::Locker owner_client_id_locker(m_owner_client_id_lock); + set_owner_client_id(ClientId()); + } + bufferlist bl; ::encode(NotifyMessage(ReleasedLockPayload(get_client_id())), bl); m_image_ctx.md_ctx.notify2(m_image_ctx.header_oid, bl, NOTIFY_TIMEOUT, NULL); @@ -695,7 +309,13 @@ void ImageWatcher::notify_released_lock() { void ImageWatcher::schedule_request_lock(bool use_timer, int timer_delay) { assert(m_image_ctx.owner_lock.is_locked()); - assert(m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED); + + if (m_image_ctx.exclusive_lock == nullptr) { + // exclusive lock dynamically disabled via image refresh + return; + } + assert(m_image_ctx.exclusive_lock && + !m_image_ctx.exclusive_lock->is_lock_owner()); RWLock::RLocker watch_locker(m_watch_lock); if (m_watch_state == WATCH_STATE_REGISTERED) { @@ -716,12 +336,8 @@ void ImageWatcher::schedule_request_lock(bool use_timer, int timer_delay) { } void ImageWatcher::notify_request_lock() { - ldout(m_image_ctx.cct, 10) << this << " notify request lock" << dendl; - RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - if (try_request_lock()) { - return; - } + ldout(m_image_ctx.cct, 10) << this << " notify request lock" << dendl; bufferlist bl; ::encode(NotifyMessage(RequestLockPayload(get_client_id())), bl); @@ -730,7 +346,9 @@ void ImageWatcher::notify_request_lock() { if (r == -ETIMEDOUT) { ldout(m_image_ctx.cct, 5) << this << " timed out requesting lock: retrying" << dendl; - schedule_request_lock(false); + + // treat this is a dead client -- so retest acquiring the lock + m_image_ctx.exclusive_lock->handle_lock_released(); } else if (r < 0) { lderr(m_image_ctx.cct) << this << " error requesting lock: " << cpp_strerror(r) << dendl; @@ -754,6 +372,7 @@ int ImageWatcher::notify_lock_owner(bufferlist &bl) { int r = m_image_ctx.md_ctx.notify2(m_image_ctx.header_oid, bl, NOTIFY_TIMEOUT, &response_bl); m_image_ctx.owner_lock.get_read(); + if (r < 0 && r != -ETIMEDOUT) { lderr(m_image_ctx.cct) << this << " lock owner notification failed: " << cpp_strerror(r) << dendl; @@ -891,7 +510,7 @@ bool ImageWatcher::handle_payload(const AcquiredLockPayload &payload, bool cancel_async_requests = true; if (payload.client_id.is_valid()) { - Mutex::Locker l(m_owner_client_id_lock); + Mutex::Locker owner_client_id_locker(m_owner_client_id_lock); if (payload.client_id == m_owner_client_id) { cancel_async_requests = false; } @@ -899,11 +518,10 @@ bool ImageWatcher::handle_payload(const AcquiredLockPayload &payload, } RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED) { - if (cancel_async_requests) { - schedule_cancel_async_requests(); - } - notify_listeners_updated_lock(LOCK_UPDATE_STATE_NOTIFICATION); + if (cancel_async_requests && + (m_image_ctx.exclusive_lock == nullptr || + !m_image_ctx.exclusive_lock->is_lock_owner())) { + schedule_cancel_async_requests(); } return true; } @@ -926,11 +544,17 @@ bool ImageWatcher::handle_payload(const ReleasedLockPayload &payload, } RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_NOT_LOCKED) { - if (cancel_async_requests) { - schedule_cancel_async_requests(); - } - notify_listeners_updated_lock(LOCK_UPDATE_STATE_NOTIFICATION); + if (cancel_async_requests && + (m_image_ctx.exclusive_lock == nullptr || + !m_image_ctx.exclusive_lock->is_lock_owner())) { + schedule_cancel_async_requests(); + } + + // alert the exclusive lock state machine that the lock is available + if (m_image_ctx.exclusive_lock != nullptr && + !m_image_ctx.exclusive_lock->is_lock_owner()) { + m_task_finisher->cancel(TASK_CODE_REQUEST_LOCK); + m_image_ctx.exclusive_lock->handle_lock_released(); } return true; } @@ -943,36 +567,23 @@ bool ImageWatcher::handle_payload(const RequestLockPayload &payload, } RWLock::RLocker l(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { + if (m_image_ctx.exclusive_lock != nullptr && + m_image_ctx.exclusive_lock->is_lock_owner()) { // need to send something back so the client can detect a missing leader ::encode(ResponseMessage(0), ack_ctx->out); { - Mutex::Locker l(m_owner_client_id_lock); + Mutex::Locker owner_client_id_locker(m_owner_client_id_lock); if (!m_owner_client_id.is_valid()) { return true; } } - bool release_permitted = true; - { - Mutex::Locker listeners_locker(m_listeners_lock); - for (Listeners::iterator it = m_listeners.begin(); - it != m_listeners.end(); ++it) { - if (!(*it)->handle_requested_lock()) { - release_permitted = false; - break; - } - } - } - - if (release_permitted) { - ldout(m_image_ctx.cct, 10) << this << " queuing release of exclusive lock" - << dendl; - FunctionContext *ctx = new FunctionContext( - boost::bind(&ImageWatcher::notify_release_lock, this)); - m_task_finisher->queue(TASK_CODE_RELEASING_LOCK, ctx); - } + ldout(m_image_ctx.cct, 10) << this << " queuing release of exclusive lock" + << dendl; + FunctionContext *ctx = new FunctionContext( + boost::bind(&ImageWatcher::notify_release_lock, this)); + m_task_finisher->queue(TASK_CODE_RELEASING_LOCK, ctx); } return true; } @@ -1011,7 +622,8 @@ bool ImageWatcher::handle_payload(const FlattenPayload &payload, C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { + if (m_image_ctx.exclusive_lock != nullptr && + m_image_ctx.exclusive_lock->is_lock_owner()) { bool new_request; Context *ctx; ProgressContext *prog_ctx; @@ -1031,7 +643,8 @@ bool ImageWatcher::handle_payload(const FlattenPayload &payload, bool ImageWatcher::handle_payload(const ResizePayload &payload, C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { + if (m_image_ctx.exclusive_lock != nullptr && + m_image_ctx.exclusive_lock->is_lock_owner()) { bool new_request; Context *ctx; ProgressContext *prog_ctx; @@ -1052,7 +665,8 @@ bool ImageWatcher::handle_payload(const ResizePayload &payload, bool ImageWatcher::handle_payload(const SnapCreatePayload &payload, C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { + if (m_image_ctx.exclusive_lock != nullptr && + m_image_ctx.exclusive_lock->is_lock_owner()) { ldout(m_image_ctx.cct, 10) << this << " remote snap_create request: " << payload.snap_name << dendl; @@ -1066,7 +680,8 @@ bool ImageWatcher::handle_payload(const SnapCreatePayload &payload, bool ImageWatcher::handle_payload(const SnapRenamePayload &payload, C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { + if (m_image_ctx.exclusive_lock != nullptr && + m_image_ctx.exclusive_lock->is_lock_owner()) { ldout(m_image_ctx.cct, 10) << this << " remote snap_rename request: " << payload.snap_id << " to " << payload.snap_name << dendl; @@ -1081,7 +696,8 @@ bool ImageWatcher::handle_payload(const SnapRenamePayload &payload, bool ImageWatcher::handle_payload(const SnapRemovePayload &payload, C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { + if (m_image_ctx.exclusive_lock != nullptr && + m_image_ctx.exclusive_lock->is_lock_owner()) { ldout(m_image_ctx.cct, 10) << this << " remote snap_remove request: " << payload.snap_name << dendl; @@ -1095,7 +711,8 @@ bool ImageWatcher::handle_payload(const SnapRemovePayload &payload, bool ImageWatcher::handle_payload(const SnapProtectPayload& payload, C_NotifyAck *ack_ctx) { RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { + if (m_image_ctx.exclusive_lock != nullptr && + m_image_ctx.exclusive_lock->is_lock_owner()) { ldout(m_image_ctx.cct, 10) << this << " remote snap_protect request: " << payload.snap_name << dendl; @@ -1109,7 +726,8 @@ bool ImageWatcher::handle_payload(const SnapProtectPayload& payload, bool ImageWatcher::handle_payload(const SnapUnprotectPayload& payload, C_NotifyAck *ack_ctx) { RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { + if (m_image_ctx.exclusive_lock != nullptr && + m_image_ctx.exclusive_lock->is_lock_owner()) { ldout(m_image_ctx.cct, 10) << this << " remote snap_unprotect request: " << payload.snap_name << dendl; @@ -1123,7 +741,8 @@ bool ImageWatcher::handle_payload(const SnapUnprotectPayload& payload, bool ImageWatcher::handle_payload(const RebuildObjectMapPayload& payload, C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { + if (m_image_ctx.exclusive_lock != nullptr && + m_image_ctx.exclusive_lock->is_lock_owner()) { bool new_request; Context *ctx; ProgressContext *prog_ctx; @@ -1144,7 +763,8 @@ bool ImageWatcher::handle_payload(const RebuildObjectMapPayload& payload, bool ImageWatcher::handle_payload(const RenamePayload& payload, C_NotifyAck *ack_ctx) { RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { + if (m_image_ctx.exclusive_lock != nullptr && + m_image_ctx.exclusive_lock->is_lock_owner()) { ldout(m_image_ctx.cct, 10) << this << " remote rename request: " << payload.image_name << dendl; @@ -1158,7 +778,8 @@ bool ImageWatcher::handle_payload(const RenamePayload& payload, bool ImageWatcher::handle_payload(const UnknownPayload &payload, C_NotifyAck *ack_ctx) { RWLock::RLocker l(m_image_ctx.owner_lock); - if (is_lock_owner()) { + if (m_image_ctx.exclusive_lock != nullptr && + m_image_ctx.exclusive_lock->is_lock_owner()) { ::encode(ResponseMessage(-EOPNOTSUPP), ack_ctx->out); } return true; @@ -1213,14 +834,23 @@ void ImageWatcher::acknowledge_notify(uint64_t notify_id, uint64_t handle, void ImageWatcher::reregister_watch() { ldout(m_image_ctx.cct, 10) << this << " re-registering image watch" << dendl; - RWLock::WLocker l(m_image_ctx.owner_lock); bool was_lock_owner = false; - if (m_lock_owner_state == LOCK_OWNER_STATE_LOCKED) { - // ensure all async requests are canceled and IO is flushed - was_lock_owner = release_lock(); + C_SaferCond release_lock_ctx; + { + RWLock::WLocker l(m_image_ctx.owner_lock); + if (m_image_ctx.exclusive_lock != nullptr && + m_image_ctx.exclusive_lock->is_lock_owner()) { + was_lock_owner = true; + m_image_ctx.exclusive_lock->release_lock(&release_lock_ctx); + } } int r; + if (was_lock_owner) { + r = release_lock_ctx.wait(); + assert(r == 0); + } + { RWLock::WLocker l(m_watch_lock); if (m_watch_state != WATCH_STATE_ERROR) { @@ -1244,18 +874,6 @@ void ImageWatcher::reregister_watch() { m_watch_state = WATCH_STATE_REGISTERED; } handle_payload(HeaderUpdatePayload(), NULL); - - if (was_lock_owner) { - r = try_lock(); - if (r == -EBUSY) { - ldout(m_image_ctx.cct, 5) << this << "lost image lock while " - << "re-registering image watch" << dendl; - } else if (r < 0) { - lderr(m_image_ctx.cct) << this - << "failed to lock image while re-registering " - << "image watch" << cpp_strerror(r) << dendl; - } - } } void ImageWatcher::WatchCtx::handle_notify(uint64_t notify_id, @@ -1273,27 +891,6 @@ void ImageWatcher::RemoteContext::finish(int r) { m_image_watcher.schedule_async_complete(m_async_request_id, r); } -void ImageWatcher::notify_listeners_updated_lock( - LockUpdateState lock_update_state) { - assert(m_image_ctx.owner_lock.is_locked()); - - Listeners listeners; - { - Mutex::Locker listeners_locker(m_listeners_lock); - m_listeners_in_use = true; - listeners = m_listeners; - } - - for (Listeners::iterator it = listeners.begin(); - it != listeners.end(); ++it) { - (*it)->handle_lock_updated(lock_update_state); - } - - Mutex::Locker listeners_locker(m_listeners_lock); - m_listeners_in_use = false; - m_listeners_cond.Signal(); -} - ImageWatcher::C_NotifyAck::C_NotifyAck(ImageWatcher *image_watcher, uint64_t notify_id, uint64_t handle) : image_watcher(image_watcher), notify_id(notify_id), handle(handle) { @@ -1320,28 +917,3 @@ void ImageWatcher::C_ResponseMessage::finish(int r) { } } // namespace librbd - -std::ostream &operator<<(std::ostream &os, - const librbd::ImageWatcher::LockUpdateState &state) { - switch (state) { - case librbd::ImageWatcher::LOCK_UPDATE_STATE_NOT_SUPPORTED: - os << "NotSupported"; - break; - case librbd::ImageWatcher::LOCK_UPDATE_STATE_LOCKED: - os << "Locked"; - break; - case librbd::ImageWatcher::LOCK_UPDATE_STATE_RELEASING: - os << "Releasing"; - break; - case librbd::ImageWatcher::LOCK_UPDATE_STATE_UNLOCKED: - os << "Unlocked"; - break; - case librbd::ImageWatcher::LOCK_UPDATE_STATE_NOTIFICATION: - os << "Notification"; - break; - default: - os << "Unknown (" << static_cast(state) << ")"; - break; - } - return os; -} diff --git a/src/librbd/ImageWatcher.h b/src/librbd/ImageWatcher.h index 5e73cf1c90c..9fd7c223c57 100644 --- a/src/librbd/ImageWatcher.h +++ b/src/librbd/ImageWatcher.h @@ -26,42 +26,12 @@ template class TaskFinisher; class ImageWatcher { public: - enum LockUpdateState { - LOCK_UPDATE_STATE_NOT_SUPPORTED, - LOCK_UPDATE_STATE_LOCKED, - LOCK_UPDATE_STATE_RELEASING, - LOCK_UPDATE_STATE_UNLOCKED, - LOCK_UPDATE_STATE_NOTIFICATION - }; - - struct Listener { - virtual ~Listener() {} - - virtual bool handle_requested_lock() = 0; - virtual void handle_lock_updated(LockUpdateState lock_update_state) = 0; - }; - ImageWatcher(ImageCtx& image_ctx); ~ImageWatcher(); - bool is_lock_supported() const; - bool is_lock_supported(const RWLock &snap_lock) const; - bool is_lock_owner() const; - - void register_listener(Listener *listener); - void unregister_listener(Listener *listener); - int register_watch(); int unregister_watch(); - int refresh(); - - int try_lock(); - void request_lock(); - int release_lock(); - - void assert_header_locked(librados::ObjectWriteOperation *op); - int notify_flatten(uint64_t request_id, ProgressContext &prog_ctx); int notify_resize(uint64_t request_id, uint64_t size, ProgressContext &prog_ctx); @@ -75,20 +45,20 @@ public: ProgressContext &prog_ctx); int notify_rename(const std::string &image_name); + void notify_acquired_lock(); + void notify_release_lock(); + void notify_released_lock(); void notify_request_lock(); - void notify_lock_state(); static void notify_header_update(librados::IoCtx &io_ctx, const std::string &oid); -private: - - enum LockOwnerState { - LOCK_OWNER_STATE_NOT_LOCKED, - LOCK_OWNER_STATE_LOCKED, - LOCK_OWNER_STATE_RELEASING - }; + uint64_t get_watch_handle() const { + RWLock::RLocker watch_locker(m_watch_lock); + return m_watch_handle; + } +private: enum WatchState { WATCH_STATE_UNREGISTERED, WATCH_STATE_REGISTERED, @@ -106,7 +76,6 @@ private: TASK_CODE_ASYNC_PROGRESS }; - typedef std::list Listeners; typedef std::pair AsyncRequest; class Task { @@ -225,21 +194,11 @@ private: ImageCtx &m_image_ctx; - RWLock m_watch_lock; + mutable RWLock m_watch_lock; WatchCtx m_watch_ctx; uint64_t m_watch_handle; WatchState m_watch_state; - Mutex m_refresh_lock; - bool m_lock_supported; - - LockOwnerState m_lock_owner_state; - - Mutex m_listeners_lock; - Cond m_listeners_cond; - Listeners m_listeners; - bool m_listeners_in_use; - TaskFinisher *m_task_finisher; RWLock m_async_request_lock; @@ -249,25 +208,12 @@ private: Mutex m_owner_client_id_lock; watch_notify::ClientId m_owner_client_id; - std::string encode_lock_cookie() const; - static bool decode_lock_cookie(const std::string &cookie, uint64_t *handle); - - int get_lock_owner_info(entity_name_t *locker, std::string *cookie, - std::string *address, uint64_t *handle); - int lock(); - int unlock(); - bool try_request_lock(); - void schedule_cancel_async_requests(); void cancel_async_requests(); void set_owner_client_id(const watch_notify::ClientId &client_id); watch_notify::ClientId get_client_id(); - void notify_acquired_lock(); - void notify_release_lock(); - void notify_released_lock(); - void schedule_request_lock(bool use_timer, int timer_delay = -1); int notify_lock_owner(bufferlist &bl); @@ -327,13 +273,8 @@ private: void acknowledge_notify(uint64_t notify_id, uint64_t handle, bufferlist &out); void reregister_watch(); - - void notify_listeners_updated_lock(LockUpdateState lock_update_state); }; } // namespace librbd -std::ostream &operator<<(std::ostream &os, - const librbd::ImageWatcher::LockUpdateState &state); - #endif // CEPH_LIBRBD_IMAGE_WATCHER_H diff --git a/src/librbd/Journal.cc b/src/librbd/Journal.cc index 755a058a447..8b2c82646c0 100644 --- a/src/librbd/Journal.cc +++ b/src/librbd/Journal.cc @@ -5,6 +5,7 @@ #include "librbd/AioCompletion.h" #include "librbd/AioImageRequestWQ.h" #include "librbd/AioObjectRequest.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/JournalReplay.h" #include "librbd/JournalTypes.h" @@ -73,14 +74,12 @@ struct C_ReplayCommitted : public Context { Journal::Journal(ImageCtx &image_ctx) : m_image_ctx(image_ctx), m_journaler(NULL), m_lock("Journal::m_lock"), m_state(STATE_UNINITIALIZED), - m_lock_listener(this), m_replay_handler(this), m_close_pending(false), + m_replay_handler(this), m_close_pending(false), m_event_lock("Journal::m_event_lock"), m_event_tid(0), m_blocking_writes(false), m_journal_replay(NULL) { ldout(m_image_ctx.cct, 5) << this << ": ictx=" << &m_image_ctx << dendl; - m_image_ctx.image_watcher->register_listener(&m_lock_listener); - Mutex::Locker locker(m_lock); block_writes(); } @@ -91,8 +90,6 @@ Journal::~Journal() { assert(m_journal_replay == NULL); assert(m_wait_for_state_contexts.empty()); - m_image_ctx.image_watcher->unregister_listener(&m_lock_listener); - Mutex::Locker locker(m_lock); unblock_writes(); } @@ -620,9 +617,6 @@ void Journal::handle_replay_complete(int r) { unblock_writes(); } - - // kick peers to let them know they can re-request the lock now - m_image_ctx.image_watcher->notify_lock_state(); } void Journal::handle_event_safe(int r, uint64_t tid) { @@ -659,7 +653,7 @@ void Journal::handle_event_safe(int r, uint64_t tid) { } else { // send any waiting aio requests now that journal entry is safe RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - assert(m_image_ctx.image_watcher->is_lock_owner()); + assert(m_image_ctx.exclusive_lock->is_lock_owner()); for (AioObjectRequests::iterator it = aio_object_requests.begin(); it != aio_object_requests.end(); ++it) { @@ -674,60 +668,6 @@ void Journal::handle_event_safe(int r, uint64_t tid) { } } -bool Journal::handle_requested_lock() { - Mutex::Locker locker(m_lock); - - CephContext *cct = m_image_ctx.cct; - ldout(cct, 20) << this << " " << __func__ << ": " << "state=" << m_state - << dendl; - - // prevent peers from taking our lock while we are replaying since that - // will stale forward progress - return (m_state != STATE_INITIALIZING && m_state != STATE_REPLAYING); -} - -void Journal::handle_lock_updated(ImageWatcher::LockUpdateState state) { - - CephContext *cct = m_image_ctx.cct; - ldout(cct, 20) << this << " " << __func__ << ": " - << "state=" << state << dendl; - - Mutex::Locker locker(m_lock); - if (state == ImageWatcher::LOCK_UPDATE_STATE_LOCKED && - m_state == STATE_UNINITIALIZED) { - create_journaler(); - } else if (state == ImageWatcher::LOCK_UPDATE_STATE_RELEASING) { - if (m_state == STATE_INITIALIZING || m_state == STATE_REPLAYING) { - // wait for replay to successfully interrupt - m_close_pending = true; - wait_for_state_transition(); - } - - if (m_state == STATE_UNINITIALIZED || m_state == STATE_RECORDING) { - // prevent new write ops but allow pending ops to flush to the journal - block_writes(); - } - if (m_state == STATE_RECORDING) { - flush_journal(); - } - } else if ((state == ImageWatcher::LOCK_UPDATE_STATE_NOT_SUPPORTED || - state == ImageWatcher::LOCK_UPDATE_STATE_UNLOCKED) && - m_state != STATE_UNINITIALIZED && - m_state != STATE_STOPPING_RECORDING) { - assert(m_state == STATE_RECORDING); - { - Mutex::Locker event_locker(m_event_lock); - assert(m_events.empty()); - } - - int r = stop_recording(); - if (r < 0) { - // TODO handle failed journal writes - assert(false); - } - } -} - int Journal::stop_recording() { assert(m_lock.is_locked()); assert(m_journaler != NULL); diff --git a/src/librbd/Journal.h b/src/librbd/Journal.h index a7d9e7201dd..190bf91397c 100644 --- a/src/librbd/Journal.h +++ b/src/librbd/Journal.h @@ -13,7 +13,6 @@ #include "common/Cond.h" #include "journal/Future.h" #include "journal/ReplayHandler.h" -#include "librbd/ImageWatcher.h" #include #include #include @@ -108,19 +107,6 @@ private: }; typedef ceph::unordered_map Events; - struct LockListener : public ImageWatcher::Listener { - Journal *journal; - LockListener(Journal *_journal) : journal(_journal) { - } - - virtual bool handle_requested_lock() { - return journal->handle_requested_lock(); - } - virtual void handle_lock_updated(ImageWatcher::LockUpdateState state) { - journal->handle_lock_updated(state); - } - }; - struct C_InitJournal : public Context { Journal *journal; @@ -186,7 +172,6 @@ private: State m_state; Contexts m_wait_for_state_contexts; - LockListener m_lock_listener; ReplayHandler m_replay_handler; bool m_close_pending; @@ -213,9 +198,6 @@ private: void handle_event_safe(int r, uint64_t tid); - bool handle_requested_lock(); - void handle_lock_updated(ImageWatcher::LockUpdateState state); - int stop_recording(); void block_writes(); diff --git a/src/librbd/LibrbdWriteback.cc b/src/librbd/LibrbdWriteback.cc index 949f75a8b9a..1147c36c1cc 100644 --- a/src/librbd/LibrbdWriteback.cc +++ b/src/librbd/LibrbdWriteback.cc @@ -12,6 +12,7 @@ #include "include/rbd/librbd.hpp" #include "librbd/AioObjectRequest.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/internal.h" #include "librbd/LibrbdWriteback.h" @@ -157,7 +158,7 @@ namespace librbd { << "journal committed: sending write request" << dendl; RWLock::RLocker owner_locker(image_ctx->owner_lock); - assert(image_ctx->image_watcher->is_lock_owner()); + assert(image_ctx->exclusive_lock->is_lock_owner()); request_sent = true; AioObjectWrite *req = new AioObjectWrite(image_ctx, oid, object_no, off, diff --git a/src/librbd/ObjectMap.cc b/src/librbd/ObjectMap.cc index d7f4b6fc8a5..9499dc10c64 100644 --- a/src/librbd/ObjectMap.cc +++ b/src/librbd/ObjectMap.cc @@ -1,6 +1,7 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab #include "librbd/ObjectMap.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" #include "librbd/internal.h" @@ -349,8 +350,8 @@ void ObjectMap::aio_resize(uint64_t new_size, uint8_t default_object_state, assert(m_image_ctx.test_features(RBD_FEATURE_OBJECT_MAP)); assert(m_image_ctx.owner_lock.is_locked()); assert(m_image_ctx.image_watcher != NULL); - assert(!m_image_ctx.image_watcher->is_lock_supported() || - m_image_ctx.image_watcher->is_lock_owner()); + assert(m_image_ctx.exclusive_lock == nullptr || + m_image_ctx.exclusive_lock->is_lock_owner()); object_map::ResizeRequest *req = new object_map::ResizeRequest( m_image_ctx, &m_object_map, m_snap_id, new_size, default_object_state, @@ -375,8 +376,8 @@ bool ObjectMap::aio_update(uint64_t start_object_no, uint64_t end_object_no, assert((m_image_ctx.features & RBD_FEATURE_OBJECT_MAP) != 0); assert(m_image_ctx.owner_lock.is_locked()); assert(m_image_ctx.image_watcher != NULL); - assert(!m_image_ctx.image_watcher->is_lock_supported(m_image_ctx.snap_lock) || - m_image_ctx.image_watcher->is_lock_owner()); + assert(m_image_ctx.exclusive_lock == nullptr || + m_image_ctx.exclusive_lock->is_lock_owner()); assert(m_image_ctx.object_map_lock.is_wlocked()); assert(start_object_no < end_object_no); diff --git a/src/librbd/image/CloseRequest.cc b/src/librbd/image/CloseRequest.cc index 34bc4b1d6c3..a038dac6802 100644 --- a/src/librbd/image/CloseRequest.cc +++ b/src/librbd/image/CloseRequest.cc @@ -7,6 +7,7 @@ #include "common/WorkQueue.h" #include "librbd/AioImageRequestWQ.h" #include "librbd/ExclusiveLock.h" +#include "librbd/ImageWatcher.h" #include "librbd/ImageCtx.h" #include "librbd/ImageState.h" #include "librbd/ImageWatcher.h" diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index b5e77e73f49..bfe917503ad 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -25,6 +25,7 @@ #include "librbd/AioObjectRequest.h" #include "librbd/CopyupRequest.h" #include "librbd/DiffIterate.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" #include "librbd/internal.h" @@ -158,28 +159,27 @@ int prepare_image_update(ImageCtx *ictx) { assert(ictx->owner_lock.is_locked() && !ictx->owner_lock.is_wlocked()); if (ictx->image_watcher == NULL) { return -EROFS; - } else if (!ictx->image_watcher->is_lock_supported() || - ictx->image_watcher->is_lock_owner()) { - return 0; } // need to upgrade to a write lock int r = 0; - bool acquired_lock = false; + bool trying_lock = false; + C_SaferCond ctx; ictx->owner_lock.put_read(); { - RWLock::WLocker l(ictx->owner_lock); - if (!ictx->image_watcher->is_lock_owner()) { - r = ictx->image_watcher->try_lock(); - acquired_lock = ictx->image_watcher->is_lock_owner(); + RWLock::WLocker owner_locker(ictx->owner_lock); + if (ictx->exclusive_lock != nullptr && + !ictx->exclusive_lock->is_lock_owner()) { + ictx->exclusive_lock->try_lock(&ctx); + trying_lock = true; } } - if (acquired_lock) { - // finish any AIO that was previously waiting on acquiring the - // exclusive lock - ictx->flush_async_operations(); + + if (trying_lock) { + r = ctx.wait(); } ictx->owner_lock.get_read(); + return r; } @@ -200,11 +200,11 @@ int invoke_async_request(ImageCtx *ictx, const std::string& request_type, } } - while (ictx->image_watcher->is_lock_supported()) { + while (ictx->exclusive_lock != nullptr) { r = prepare_image_update(ictx); if (r < 0) { return -EROFS; - } else if (ictx->image_watcher->is_lock_owner()) { + } else if (ictx->exclusive_lock->is_lock_owner()) { break; } @@ -349,8 +349,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { void trim_image(ImageCtx *ictx, uint64_t newsize, ProgressContext& prog_ctx) { assert(ictx->owner_lock.is_locked()); - assert(!ictx->image_watcher->is_lock_supported() || - ictx->image_watcher->is_lock_owner()); + assert(ictx->exclusive_lock == nullptr || + ictx->exclusive_lock->is_lock_owner()); C_SaferCond ctx; ictx->snap_lock.get_read(); @@ -809,8 +809,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { void snap_create_helper(ImageCtx* ictx, Context* ctx, const char* snap_name) { assert(ictx->owner_lock.is_locked()); - assert(!ictx->image_watcher->is_lock_supported() || - ictx->image_watcher->is_lock_owner()); + assert(ictx->exclusive_lock == nullptr || + ictx->exclusive_lock->is_lock_owner()); ldout(ictx->cct, 20) << "snap_create_helper " << ictx << " " << snap_name << dendl; @@ -878,8 +878,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { assert(ictx->owner_lock.is_locked()); { if ((ictx->features & RBD_FEATURE_FAST_DIFF) != 0) { - assert(!ictx->image_watcher->is_lock_supported() || - ictx->image_watcher->is_lock_owner()); + assert(ictx->exclusive_lock == nullptr || + ictx->exclusive_lock->is_lock_owner()); } } @@ -973,8 +973,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { const uint64_t src_snap_id, const char* dst_name) { assert(ictx->owner_lock.is_locked()); if ((ictx->features & RBD_FEATURE_JOURNALING) != 0) { - assert(!ictx->image_watcher->is_lock_supported() || - ictx->image_watcher->is_lock_owner()); + assert(ictx->exclusive_lock == nullptr || + ictx->exclusive_lock->is_lock_owner()); } ldout(ictx->cct, 20) << __func__ << " " << ictx << " from " << src_snap_id << " to " << dst_name << dendl; @@ -1045,8 +1045,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { { assert(ictx->owner_lock.is_locked()); if (ictx->test_features(RBD_FEATURE_JOURNALING)) { - assert(!ictx->image_watcher->is_lock_supported() || - ictx->image_watcher->is_lock_owner()); + assert(ictx->exclusive_lock == nullptr || + ictx->exclusive_lock->is_lock_owner()); } ldout(ictx->cct, 20) << "snap_protect_helper " << ictx << " " << snap_name @@ -1120,8 +1120,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { { assert(ictx->owner_lock.is_locked()); if (ictx->test_features(RBD_FEATURE_JOURNALING)) { - assert(!ictx->image_watcher->is_lock_supported() || - ictx->image_watcher->is_lock_owner()); + assert(ictx->exclusive_lock == nullptr || + ictx->exclusive_lock->is_lock_owner()); } ldout(ictx->cct, 20) << "snap_unprotect_helper " << ictx << " " << snap_name @@ -1643,6 +1643,7 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { RWLock::RLocker owner_locker(p_imctx->owner_lock); r = ictx_refresh(p_imctx); } + if (r == 0) { p_imctx->snap_lock.get_read(); r = p_imctx->is_snap_protected(p_imctx->snap_id, &snap_protected); @@ -1740,8 +1741,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { { assert(ictx->owner_lock.is_locked()); if (ictx->test_features(RBD_FEATURE_JOURNALING)) { - assert(!ictx->image_watcher->is_lock_supported() || - ictx->image_watcher->is_lock_owner()); + assert(ictx->exclusive_lock == nullptr || + ictx->exclusive_lock->is_lock_owner()); } ldout(ictx->cct, 20) << "rename_helper " << ictx << " " << dstname @@ -2130,8 +2131,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { int is_exclusive_lock_owner(ImageCtx *ictx, bool *is_owner) { RWLock::RLocker l(ictx->owner_lock); - *is_owner = (ictx->image_watcher != NULL && - ictx->image_watcher->is_lock_owner()); + *is_owner = (ictx->exclusive_lock != nullptr && + ictx->exclusive_lock->is_lock_owner()); return 0; } @@ -2154,9 +2155,9 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { id = ictx->id; ictx->owner_lock.get_read(); - if (ictx->image_watcher->is_lock_supported()) { + if (ictx->exclusive_lock != nullptr) { r = prepare_image_update(ictx); - if (r < 0 || !ictx->image_watcher->is_lock_owner()) { + if (r < 0 || !ictx->exclusive_lock->is_lock_owner()) { lderr(cct) << "cannot obtain exclusive lock - not removing" << dendl; ictx->owner_lock.put_read(); close_image(ictx); @@ -2294,8 +2295,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { ProgressContext &prog_ctx) { assert(ictx->owner_lock.is_locked()); - assert(!ictx->image_watcher->is_lock_supported() || - ictx->image_watcher->is_lock_owner()); + assert(ictx->exclusive_lock == nullptr || + ictx->exclusive_lock->is_lock_owner()); CephContext *cct = ictx->cct; ictx->snap_lock.get_read(); @@ -2621,7 +2622,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { } // release snap_lock and cache_lock if (ictx->image_watcher != NULL) { - ictx->image_watcher->refresh(); + // TODO handled by new async refresh state machine + //ictx->image_watcher->refresh(); } if (new_snap) { @@ -2667,8 +2669,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { if (r < 0) { return -EROFS; } - if (ictx->image_watcher->is_lock_supported() && - !ictx->image_watcher->is_lock_owner()) { + if (ictx->exclusive_lock != nullptr && + !ictx->exclusive_lock->is_lock_owner()) { return -EROFS; } @@ -2911,13 +2913,14 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { if (snapshot_mode) { { RWLock::WLocker owner_locker(ictx->owner_lock); - if (ictx->image_watcher != NULL && - ictx->image_watcher->is_lock_owner()) { - r = ictx->image_watcher->release_lock(); - if (r < 0) { - return r; - } - } + // TODO handled by new async set snap state machine + //if (ictx->image_watcher != NULL && + // ictx->image_watcher->is_lock_owner()) { + // r = ictx->image_watcher->release_lock(); + // if (r < 0) { + // return r; + // } + //} } ictx->cancel_async_requests(); @@ -2952,7 +2955,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { RWLock::RLocker owner_locker(ictx->owner_lock); if (ictx->image_watcher != NULL) { - ictx->image_watcher->refresh(); + // TODO handled by new async set snap request state machine + //ictx->image_watcher->refresh(); } return r; } @@ -2971,9 +2975,9 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { if (!ictx->read_only) { r = ictx->register_watch(); if (r < 0) { - lderr(ictx->cct) << "error registering a watch: " << cpp_strerror(r) - << dendl; - goto err_close; + lderr(ictx->cct) << "error registering a watch: " << cpp_strerror(r) + << dendl; + goto err_close; } } @@ -2989,7 +2993,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { if (ictx->image_watcher != NULL) { RWLock::RLocker owner_locker(ictx->owner_lock); - ictx->image_watcher->refresh(); + // TODO handled by new async open image state machine + //ictx->image_watcher->refresh(); } return 0; @@ -3012,13 +3017,7 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { { // release the lock (and flush all in-flight IO) RWLock::WLocker owner_locker(ictx->owner_lock); - if (ictx->image_watcher != NULL && ictx->image_watcher->is_lock_owner()) { - r = ictx->image_watcher->release_lock(); - if (r < 0) { - lderr(ictx->cct) << "error releasing image lock: " << cpp_strerror(r) - << dendl; - } - } + // TODO replaced by new async close request } assert(!ictx->aio_work_queue->writes_blocked() || @@ -3138,8 +3137,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { void async_flatten(ImageCtx *ictx, Context *ctx, ProgressContext &prog_ctx) { assert(ictx->owner_lock.is_locked()); - assert(!ictx->image_watcher->is_lock_supported() || - ictx->image_watcher->is_lock_owner()); + assert(ictx->exclusive_lock == nullptr || + ictx->exclusive_lock->is_lock_owner()); CephContext *cct = ictx->cct; ldout(cct, 20) << "flatten" << dendl; @@ -3220,8 +3219,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) { void async_rebuild_object_map(ImageCtx *ictx, Context *ctx, ProgressContext &prog_ctx) { assert(ictx->owner_lock.is_locked()); - assert(!ictx->image_watcher->is_lock_supported() || - ictx->image_watcher->is_lock_owner()); + assert(ictx->exclusive_lock == nullptr || + ictx->exclusive_lock->is_lock_owner()); CephContext *cct = ictx->cct; ldout(cct, 20) << "async_rebuild_object_map " << ictx << dendl; diff --git a/src/librbd/object_map/InvalidateRequest.cc b/src/librbd/object_map/InvalidateRequest.cc index 7be26c63a7e..3566fdd99a7 100644 --- a/src/librbd/object_map/InvalidateRequest.cc +++ b/src/librbd/object_map/InvalidateRequest.cc @@ -4,6 +4,7 @@ #include "librbd/object_map/InvalidateRequest.h" #include "common/dout.h" #include "common/errno.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" @@ -49,17 +50,19 @@ void InvalidateRequest::send() { } // do not update on-disk flags if not image owner - if (image_ctx.image_watcher == NULL || - (image_ctx.image_watcher->is_lock_supported(image_ctx.snap_lock) && - !image_ctx.image_watcher->is_lock_owner() && !m_force)) { + if (image_ctx.image_watcher == nullptr || + (!m_force && m_snap_id == CEPH_NOSNAP && + image_ctx.exclusive_lock != nullptr && + !image_ctx.exclusive_lock->is_lock_owner())) { this->async_complete(0); return; } lderr(cct) << this << " invalidating object map on-disk" << dendl; librados::ObjectWriteOperation op; - if (m_snap_id == CEPH_NOSNAP && !m_force) { - image_ctx.image_watcher->assert_header_locked(&op); + if (image_ctx.exclusive_lock != nullptr && + m_snap_id == CEPH_NOSNAP && !m_force) { + image_ctx.exclusive_lock->assert_header_locked(&op); } cls_client::set_flags(&op, m_snap_id, flags, flags); diff --git a/src/librbd/operation/FlattenRequest.cc b/src/librbd/operation/FlattenRequest.cc index 4b089b4c806..cc4842ef53a 100644 --- a/src/librbd/operation/FlattenRequest.cc +++ b/src/librbd/operation/FlattenRequest.cc @@ -4,6 +4,7 @@ #include "librbd/operation/FlattenRequest.h" #include "librbd/AioObjectRequest.h" #include "librbd/AsyncObjectThrottle.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" #include "librbd/ObjectMap.h" @@ -34,8 +35,8 @@ public: assert(image_ctx.owner_lock.is_locked()); CephContext *cct = image_ctx.cct; - if (image_ctx.image_watcher->is_lock_supported() && - !image_ctx.image_watcher->is_lock_owner()) { + if (image_ctx.exclusive_lock != nullptr && + !image_ctx.exclusive_lock->is_lock_owner()) { ldout(cct, 1) << "lost exclusive lock during flatten" << dendl; return -ERESTART; } @@ -121,8 +122,8 @@ bool FlattenRequest::send_update_header() { m_state = STATE_UPDATE_HEADER; // should have been canceled prior to releasing lock - assert(!image_ctx.image_watcher->is_lock_supported() || - image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock == nullptr || + image_ctx.exclusive_lock->is_lock_owner()); { RWLock::RLocker parent_locker(image_ctx.parent_lock); @@ -138,8 +139,8 @@ bool FlattenRequest::send_update_header() { // remove parent from this (base) image librados::ObjectWriteOperation op; - if (image_ctx.image_watcher->is_lock_supported()) { - image_ctx.image_watcher->assert_header_locked(&op); + if (image_ctx.exclusive_lock != nullptr) { + image_ctx.exclusive_lock->assert_header_locked(&op); } cls_client::remove_parent(&op); @@ -158,8 +159,8 @@ bool FlattenRequest::send_update_children() { CephContext *cct = image_ctx.cct; // should have been canceled prior to releasing lock - assert(!image_ctx.image_watcher->is_lock_supported() || - image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock == nullptr || + image_ctx.exclusive_lock->is_lock_owner()); // if there are no snaps, remove from the children object as well // (if snapshots remain, they have their own parent info, and the child diff --git a/src/librbd/operation/RebuildObjectMapRequest.cc b/src/librbd/operation/RebuildObjectMapRequest.cc index c820da0bd8f..d043b88003f 100644 --- a/src/librbd/operation/RebuildObjectMapRequest.cc +++ b/src/librbd/operation/RebuildObjectMapRequest.cc @@ -5,6 +5,7 @@ #include "common/dout.h" #include "common/errno.h" #include "librbd/AsyncObjectThrottle.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" #include "librbd/internal.h" @@ -141,8 +142,8 @@ private: CephContext *cct = image_ctx.cct; // should have been canceled prior to releasing lock - assert(!image_ctx.image_watcher->is_lock_supported() || - image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock == nullptr || + image_ctx.exclusive_lock->is_lock_owner()); RWLock::WLocker l(image_ctx.object_map_lock); uint8_t state = image_ctx.object_map[m_object_no]; @@ -250,8 +251,8 @@ void RebuildObjectMapRequest::send_resize_object_map() { m_state = STATE_RESIZE_OBJECT_MAP; // should have been canceled prior to releasing lock - assert(!m_image_ctx.image_watcher->is_lock_supported() || - m_image_ctx.image_watcher->is_lock_owner()); + assert(m_image_ctx.exclusive_lock == nullptr || + m_image_ctx.exclusive_lock->is_lock_owner()); m_image_ctx.object_map.aio_resize(size, OBJECT_NONEXISTENT, this->create_callback_context()); } @@ -263,8 +264,8 @@ void RebuildObjectMapRequest::send_trim_image() { RWLock::RLocker l(m_image_ctx.owner_lock); // should have been canceled prior to releasing lock - assert(!m_image_ctx.image_watcher->is_lock_supported() || - m_image_ctx.image_watcher->is_lock_owner()); + assert(m_image_ctx.exclusive_lock == nullptr || + m_image_ctx.exclusive_lock->is_lock_owner()); ldout(cct, 5) << this << " send_trim_image" << dendl; m_state = STATE_TRIM_IMAGE; @@ -322,8 +323,8 @@ void RebuildObjectMapRequest::send_save_object_map() { m_state = STATE_SAVE_OBJECT_MAP; // should have been canceled prior to releasing lock - assert(!m_image_ctx.image_watcher->is_lock_supported() || - m_image_ctx.image_watcher->is_lock_owner()); + assert(m_image_ctx.exclusive_lock == nullptr || + m_image_ctx.exclusive_lock->is_lock_owner()); m_image_ctx.object_map.aio_save(this->create_callback_context()); } @@ -332,15 +333,15 @@ void RebuildObjectMapRequest::send_update_header() { assert(m_image_ctx.owner_lock.is_locked()); // should have been canceled prior to releasing lock - assert(!m_image_ctx.image_watcher->is_lock_supported() || - m_image_ctx.image_watcher->is_lock_owner()); + assert(m_image_ctx.exclusive_lock == nullptr || + m_image_ctx.exclusive_lock->is_lock_owner()); ldout(m_image_ctx.cct, 5) << this << " send_update_header" << dendl; m_state = STATE_UPDATE_HEADER; librados::ObjectWriteOperation op; - if (m_image_ctx.image_watcher->is_lock_supported()) { - m_image_ctx.image_watcher->assert_header_locked(&op); + if (m_image_ctx.exclusive_lock != nullptr) { + m_image_ctx.exclusive_lock->assert_header_locked(&op); } uint64_t flags = RBD_FLAG_OBJECT_MAP_INVALID | RBD_FLAG_FAST_DIFF_INVALID; diff --git a/src/librbd/operation/ResizeRequest.cc b/src/librbd/operation/ResizeRequest.cc index 282572e967e..597bfd5ae13 100644 --- a/src/librbd/operation/ResizeRequest.cc +++ b/src/librbd/operation/ResizeRequest.cc @@ -2,6 +2,7 @@ // vim: ts=8 sw=2 smarttab #include "librbd/operation/ResizeRequest.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" #include "librbd/internal.h" @@ -207,8 +208,8 @@ void ResizeRequest::send_grow_object_map() { m_state = STATE_GROW_OBJECT_MAP; // should have been canceled prior to releasing lock - assert(!image_ctx.image_watcher->is_lock_supported() || - image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock == nullptr || + image_ctx.exclusive_lock->is_lock_owner()); image_ctx.object_map.aio_resize(m_new_size, OBJECT_NONEXISTENT, this->create_callback_context()); @@ -228,8 +229,8 @@ bool ResizeRequest::send_shrink_object_map() { m_state = STATE_SHRINK_OBJECT_MAP; // should have been canceled prior to releasing lock - assert(!image_ctx.image_watcher->is_lock_supported() || - image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock == nullptr || + image_ctx.exclusive_lock->is_lock_owner()); image_ctx.object_map.aio_resize(m_new_size, OBJECT_NONEXISTENT, this->create_callback_context()); @@ -247,8 +248,8 @@ void ResizeRequest::send_update_header() { m_state = STATE_UPDATE_HEADER; // should have been canceled prior to releasing lock - assert(!image_ctx.image_watcher->is_lock_supported() || - image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock == nullptr || + image_ctx.exclusive_lock->is_lock_owner()); librados::ObjectWriteOperation op; if (image_ctx.old_format) { @@ -258,8 +259,8 @@ void ResizeRequest::send_update_header() { bl.append(reinterpret_cast(&m_new_size), sizeof(m_new_size)); op.write(offsetof(rbd_obj_header_ondisk, image_size), bl); } else { - if (image_ctx.image_watcher->is_lock_supported()) { - image_ctx.image_watcher->assert_header_locked(&op); + if (image_ctx.exclusive_lock != nullptr) { + image_ctx.exclusive_lock->assert_header_locked(&op); } cls_client::set_size(&op, m_new_size); } diff --git a/src/librbd/operation/SnapshotCreateRequest.cc b/src/librbd/operation/SnapshotCreateRequest.cc index 37f2841422e..7c659ec0724 100644 --- a/src/librbd/operation/SnapshotCreateRequest.cc +++ b/src/librbd/operation/SnapshotCreateRequest.cc @@ -5,6 +5,7 @@ #include "common/dout.h" #include "common/errno.h" #include "librbd/AioImageRequestWQ.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" #include "librbd/ObjectMap.h" @@ -188,8 +189,8 @@ void SnapshotCreateRequest::send_create_snap() { m_state = STATE_CREATE_SNAP; // should have been canceled prior to releasing lock - assert(!image_ctx.image_watcher->is_lock_supported(image_ctx.snap_lock) || - image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock == nullptr || + image_ctx.exclusive_lock->is_lock_owner()); // save current size / parent info for creating snapshot record in ImageCtx m_size = image_ctx.size; @@ -199,8 +200,8 @@ void SnapshotCreateRequest::send_create_snap() { if (image_ctx.old_format) { cls_client::old_snapshot_add(&op, m_snap_id, m_snap_name); } else { - if (image_ctx.image_watcher->is_lock_owner()) { - image_ctx.image_watcher->assert_header_locked(&op); + if (image_ctx.exclusive_lock != nullptr) { + image_ctx.exclusive_lock->assert_header_locked(&op); } cls_client::snapshot_add(&op, m_snap_id, m_snap_name); } @@ -296,8 +297,8 @@ void SnapshotCreateRequest::update_snap_context() { ldout(cct, 5) << this << " " << __func__ << dendl; // should have been canceled prior to releasing lock - assert(!image_ctx.image_watcher->is_lock_supported(image_ctx.snap_lock) || - image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock == nullptr || + image_ctx.exclusive_lock->is_lock_owner()); // immediately add a reference to the new snapshot image_ctx.add_snap(m_snap_name, m_snap_id, m_size, m_parent_info, diff --git a/src/librbd/operation/SnapshotRemoveRequest.cc b/src/librbd/operation/SnapshotRemoveRequest.cc index a10ffe23fd6..9d5ff3897aa 100644 --- a/src/librbd/operation/SnapshotRemoveRequest.cc +++ b/src/librbd/operation/SnapshotRemoveRequest.cc @@ -4,6 +4,7 @@ #include "librbd/operation/SnapshotRemoveRequest.h" #include "common/dout.h" #include "common/errno.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" #include "librbd/ObjectMap.h" @@ -164,8 +165,9 @@ void SnapshotRemoveRequest::send_remove_snap() { if (image_ctx.old_format) { cls_client::old_snapshot_remove(&op, m_snap_name); } else { - if (image_ctx.image_watcher->is_lock_owner()) { - image_ctx.image_watcher->assert_header_locked(&op); + if (image_ctx.exclusive_lock != nullptr && + image_ctx.exclusive_lock->is_lock_owner()) { + image_ctx.exclusive_lock->assert_header_locked(&op); } cls_client::snapshot_remove(&op, m_snap_id); } diff --git a/src/librbd/operation/SnapshotRenameRequest.cc b/src/librbd/operation/SnapshotRenameRequest.cc index 53ffb382567..ec7eb653c37 100644 --- a/src/librbd/operation/SnapshotRenameRequest.cc +++ b/src/librbd/operation/SnapshotRenameRequest.cc @@ -4,6 +4,7 @@ #include "librbd/operation/SnapshotRenameRequest.h" #include "common/dout.h" #include "common/errno.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" @@ -70,8 +71,9 @@ void SnapshotRenameRequest::send_rename_snap() { if (image_ctx.old_format) { cls_client::old_snapshot_rename(&op, m_snap_id, m_snap_name); } else { - if (image_ctx.image_watcher->is_lock_owner()) { - image_ctx.image_watcher->assert_header_locked(&op); + if (image_ctx.exclusive_lock != nullptr && + image_ctx.exclusive_lock->is_lock_owner()) { + image_ctx.exclusive_lock->assert_header_locked(&op); } cls_client::snapshot_rename(&op, m_snap_id, m_snap_name); } diff --git a/src/librbd/operation/TrimRequest.cc b/src/librbd/operation/TrimRequest.cc index d2825775084..3deae084657 100644 --- a/src/librbd/operation/TrimRequest.cc +++ b/src/librbd/operation/TrimRequest.cc @@ -4,6 +4,7 @@ #include "librbd/operation/TrimRequest.h" #include "librbd/AsyncObjectThrottle.h" #include "librbd/AioObjectRequest.h" +#include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" #include "librbd/internal.h" @@ -39,8 +40,8 @@ public: virtual int send() { I &image_ctx = this->m_image_ctx; assert(image_ctx.owner_lock.is_locked()); - assert(!image_ctx.image_watcher->is_lock_supported() || - image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock == nullptr || + image_ctx.exclusive_lock->is_lock_owner()); string oid = image_ctx.get_object_name(m_object_no); ldout(image_ctx.cct, 10) << "removing (with copyup) " << oid << dendl; @@ -67,8 +68,8 @@ public: virtual int send() { I &image_ctx = this->m_image_ctx; assert(image_ctx.owner_lock.is_locked()); - assert(!image_ctx.image_watcher->is_lock_supported() || - image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock == nullptr || + image_ctx.exclusive_lock->is_lock_owner()); if (!image_ctx.object_map.object_may_exist(m_object_no)) { return 1; } @@ -169,8 +170,8 @@ template void TrimRequest::send_copyup_objects() { I &image_ctx = this->m_image_ctx; assert(image_ctx.owner_lock.is_locked()); - assert(!image_ctx.image_watcher->is_lock_supported() || - image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock == nullptr || + image_ctx.exclusive_lock->is_lock_owner()); if (m_delete_start >= m_num_objects) { send_clean_boundary(); @@ -258,7 +259,7 @@ void TrimRequest::send_pre_remove() { << " num_objects=" << m_num_objects << dendl; m_state = STATE_PRE_REMOVE; - assert(image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock->is_lock_owner()); // flag the objects as pending deletion Context *ctx = this->create_callback_context(); @@ -295,7 +296,7 @@ void TrimRequest::send_post_remove() { << " num_objects=" << m_num_objects << dendl; m_state = STATE_POST_REMOVE; - assert(image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock->is_lock_owner()); // flag the pending objects as removed Context *ctx = this->create_callback_context(); @@ -327,8 +328,8 @@ void TrimRequest::send_clean_boundary() { } // should have been canceled prior to releasing lock - assert(!image_ctx.image_watcher->is_lock_supported() || - image_ctx.image_watcher->is_lock_owner()); + assert(image_ctx.exclusive_lock == nullptr || + image_ctx.exclusive_lock->is_lock_owner()); uint64_t delete_len = m_delete_off - m_new_size; ldout(image_ctx.cct, 5) << this << " send_clean_boundary: " << " delete_off=" << m_delete_off