From 7e09cb132beb0a6dcbaa06dae42c7b9d468a745b Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 16 Feb 2016 21:43:03 -0500 Subject: [PATCH] librbd: notifications should be flushed between exclusive lock states Avoid leaving in-flight notification messages when transitioning lock states. Signed-off-by: Jason Dillaman --- src/librbd/ExclusiveLock.cc | 4 ++-- src/librbd/ImageCtx.cc | 4 ++-- src/librbd/ImageWatcher.cc | 11 +++++++++- src/librbd/WatchNotifyTypes.cc | 12 ++++++++++ src/librbd/WatchNotifyTypes.h | 15 +++++++++++++ src/librbd/exclusive_lock/AcquireRequest.cc | 22 +++++++++++++++++++ src/librbd/exclusive_lock/AcquireRequest.h | 7 ++++++ src/librbd/exclusive_lock/ReleaseRequest.cc | 22 +++++++++++++++++++ src/librbd/exclusive_lock/ReleaseRequest.h | 8 ++++++- src/librbd/image_watcher/Notifier.cc | 10 +++++---- src/librbd/image_watcher/Notifier.h | 5 ++++- src/librbd/operation/SnapshotCreateRequest.cc | 3 ++- .../test_mock_AcquireRequest.cc | 21 ++++++++++++++++++ .../test_mock_ReleaseRequest.cc | 9 ++++++++ .../librbd/image/test_mock_RefreshRequest.cc | 2 ++ src/test/librbd/mock/MockImageWatcher.h | 3 +++ src/test/librbd/test_ImageWatcher.cc | 4 ++++ src/test/librbd/test_mock_ExclusiveLock.cc | 12 +++++++++- 18 files changed, 161 insertions(+), 13 deletions(-) diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc index 1d0d189eadb62..1865539e9f434 100644 --- a/src/librbd/ExclusiveLock.cc +++ b/src/librbd/ExclusiveLock.cc @@ -462,8 +462,8 @@ void ExclusiveLock::send_shutdown() { if (m_state == STATE_UNLOCKED) { m_state = STATE_SHUTTING_DOWN; m_image_ctx.aio_work_queue->unblock_writes(); - m_image_ctx.op_work_queue->queue(util::create_context_callback< - ExclusiveLock, &ExclusiveLock::complete_shutdown>(this), 0); + m_image_ctx.image_watcher->flush(util::create_context_callback< + ExclusiveLock, &ExclusiveLock::complete_shutdown>(this)); return; } diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc index 6e29292bb8dd9..762d637421e2f 100644 --- a/src/librbd/ImageCtx.cc +++ b/src/librbd/ImageCtx.cc @@ -1020,8 +1020,8 @@ struct C_InvalidateCache : public Context { void ImageCtx::set_image_name(const std::string &image_name) { // update the name so rename can be invoked repeatedly - RWLock::RLocker owner_locker(m_image_ctx.owner_lock); - RWLock::WLocker snap_locker(m_image_ctx.snap_lock); + RWLock::RLocker owner_locker(owner_lock); + RWLock::WLocker snap_locker(snap_lock); name = image_name; if (old_format) { header_oid = util::old_header_name(image_name); diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc index 35b5bf9b1aa14..b1786f7e18ee6 100644 --- a/src/librbd/ImageWatcher.cc +++ b/src/librbd/ImageWatcher.cc @@ -248,6 +248,8 @@ int ImageWatcher::notify_rename(const std::string &image_name) { } void ImageWatcher::notify_header_update(Context *on_finish) { + ldout(m_image_ctx.cct, 10) << this << ": " << __func__ << dendl; + // supports legacy (empty buffer) clients bufferlist bl; ::encode(NotifyMessage(HeaderUpdatePayload()), bl); @@ -339,6 +341,9 @@ void ImageWatcher::schedule_request_lock(bool use_timer, int timer_delay) { void ImageWatcher::notify_request_lock() { RWLock::RLocker owner_locker(m_image_ctx.owner_lock); + RWLock::RLocker snap_locker(m_image_ctx.snap_lock); + assert(!m_image_ctx.exclusive_lock->is_lock_owner()); + ldout(m_image_ctx.cct, 10) << this << " notify request lock" << dendl; bufferlist bl; @@ -349,6 +354,9 @@ void ImageWatcher::notify_request_lock() { void ImageWatcher::handle_request_lock(int r) { RWLock::RLocker owner_locker(m_image_ctx.owner_lock); + RWLock::RLocker snap_locker(m_image_ctx.snap_lock); + + // ExclusiveLock state machine cannot transition assert(!m_image_ctx.exclusive_lock->is_lock_owner()); if (r == -ETIMEDOUT) { @@ -771,7 +779,8 @@ void ImageWatcher::handle_notify(uint64_t notify_id, uint64_t handle, } // if an image refresh is required, refresh before processing the request - if (m_image_ctx.state->is_refresh_required()) { + if (notify_message.check_for_refresh() && + m_image_ctx.state->is_refresh_required()) { m_image_ctx.state->refresh(new C_ProcessPayload(this, notify_id, handle, notify_message.payload)); } else { diff --git a/src/librbd/WatchNotifyTypes.cc b/src/librbd/WatchNotifyTypes.cc index f3a6f4bfe0ff8..a40cdc7e29c00 100644 --- a/src/librbd/WatchNotifyTypes.cc +++ b/src/librbd/WatchNotifyTypes.cc @@ -11,6 +11,14 @@ namespace watch_notify { namespace { +class CheckForRefreshVisitor : public boost::static_visitor { +public: + template + inline bool operator()(const Payload &payload) const { + return Payload::CHECK_FOR_REFRESH; + } +}; + class EncodePayloadVisitor : public boost::static_visitor { public: explicit EncodePayloadVisitor(bufferlist &bl) : m_bl(bl) {} @@ -257,6 +265,10 @@ void UnknownPayload::decode(__u8 version, bufferlist::iterator &iter) { void UnknownPayload::dump(Formatter *f) const { } +bool NotifyMessage::check_for_refresh() const { + return boost::apply_visitor(CheckForRefreshVisitor(), payload); +} + void NotifyMessage::encode(bufferlist& bl) const { ENCODE_START(2, 1, bl); boost::apply_visitor(EncodePayloadVisitor(bl), payload); diff --git a/src/librbd/WatchNotifyTypes.h b/src/librbd/WatchNotifyTypes.h index 468b45ab475c3..a587b231d66c5 100644 --- a/src/librbd/WatchNotifyTypes.h +++ b/src/librbd/WatchNotifyTypes.h @@ -92,6 +92,7 @@ enum NotifyOp { struct AcquiredLockPayload { static const NotifyOp NOTIFY_OP = NOTIFY_OP_ACQUIRED_LOCK; + static const bool CHECK_FOR_REFRESH = true; ClientId client_id; @@ -105,6 +106,7 @@ struct AcquiredLockPayload { struct ReleasedLockPayload { static const NotifyOp NOTIFY_OP = NOTIFY_OP_RELEASED_LOCK; + static const bool CHECK_FOR_REFRESH = true; ClientId client_id; @@ -118,6 +120,7 @@ struct ReleasedLockPayload { struct RequestLockPayload { static const NotifyOp NOTIFY_OP = NOTIFY_OP_REQUEST_LOCK; + static const bool CHECK_FOR_REFRESH = true; ClientId client_id; @@ -131,6 +134,7 @@ struct RequestLockPayload { struct HeaderUpdatePayload { static const NotifyOp NOTIFY_OP = NOTIFY_OP_HEADER_UPDATE; + static const bool CHECK_FOR_REFRESH = false; void encode(bufferlist &bl) const; void decode(__u8 version, bufferlist::iterator &iter); @@ -152,6 +156,7 @@ protected: struct AsyncProgressPayload : public AsyncRequestPayloadBase { static const NotifyOp NOTIFY_OP = NOTIFY_OP_ASYNC_PROGRESS; + static const bool CHECK_FOR_REFRESH = false; AsyncProgressPayload() : offset(0), total(0) {} AsyncProgressPayload(const AsyncRequestId &id, uint64_t offset_, uint64_t total_) @@ -167,6 +172,7 @@ struct AsyncProgressPayload : public AsyncRequestPayloadBase { struct AsyncCompletePayload : public AsyncRequestPayloadBase { static const NotifyOp NOTIFY_OP = NOTIFY_OP_ASYNC_COMPLETE; + static const bool CHECK_FOR_REFRESH = false; AsyncCompletePayload() {} AsyncCompletePayload(const AsyncRequestId &id, int r) @@ -181,6 +187,7 @@ struct AsyncCompletePayload : public AsyncRequestPayloadBase { struct FlattenPayload : public AsyncRequestPayloadBase { static const NotifyOp NOTIFY_OP = NOTIFY_OP_FLATTEN; + static const bool CHECK_FOR_REFRESH = true; FlattenPayload() {} FlattenPayload(const AsyncRequestId &id) : AsyncRequestPayloadBase(id) {} @@ -188,6 +195,7 @@ struct FlattenPayload : public AsyncRequestPayloadBase { struct ResizePayload : public AsyncRequestPayloadBase { static const NotifyOp NOTIFY_OP = NOTIFY_OP_RESIZE; + static const bool CHECK_FOR_REFRESH = true; ResizePayload() : size(0) {} ResizePayload(uint64_t size_, const AsyncRequestId &id) @@ -202,6 +210,8 @@ struct ResizePayload : public AsyncRequestPayloadBase { struct SnapPayloadBase { public: + static const bool CHECK_FOR_REFRESH = true; + std::string snap_name; void encode(bufferlist &bl) const; @@ -257,6 +267,7 @@ struct SnapUnprotectPayload : public SnapPayloadBase { struct RebuildObjectMapPayload : public AsyncRequestPayloadBase { static const NotifyOp NOTIFY_OP = NOTIFY_OP_REBUILD_OBJECT_MAP; + static const bool CHECK_FOR_REFRESH = true; RebuildObjectMapPayload() {} RebuildObjectMapPayload(const AsyncRequestId &id) @@ -265,6 +276,7 @@ struct RebuildObjectMapPayload : public AsyncRequestPayloadBase { struct RenamePayload { static const NotifyOp NOTIFY_OP = NOTIFY_OP_RENAME; + static const bool CHECK_FOR_REFRESH = true; RenamePayload() {} RenamePayload(const std::string _image_name) : image_name(_image_name) {} @@ -278,6 +290,7 @@ struct RenamePayload { struct UnknownPayload { static const NotifyOp NOTIFY_OP = static_cast(-1); + static const bool CHECK_FOR_REFRESH = false; void encode(bufferlist &bl) const; void decode(__u8 version, bufferlist::iterator &iter); @@ -307,6 +320,8 @@ struct NotifyMessage { Payload payload; + bool check_for_refresh() const; + void encode(bufferlist& bl) const; void decode(bufferlist::iterator& it); void dump(Formatter *f) const; diff --git a/src/librbd/exclusive_lock/AcquireRequest.cc b/src/librbd/exclusive_lock/AcquireRequest.cc index 5f4986948a6fd..e46714778a368 100644 --- a/src/librbd/exclusive_lock/AcquireRequest.cc +++ b/src/librbd/exclusive_lock/AcquireRequest.cc @@ -10,6 +10,7 @@ #include "include/stringify.h" #include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" +#include "librbd/ImageWatcher.h" #include "librbd/Journal.h" #include "librbd/ObjectMap.h" #include "librbd/Utils.h" @@ -73,7 +74,28 @@ AcquireRequest::~AcquireRequest() { template void AcquireRequest::send() { + send_flush_notifies(); +} + +template +void AcquireRequest::send_flush_notifies() { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 10) << __func__ << dendl; + + using klass = AcquireRequest; + Context *ctx = create_context_callback( + this); + m_image_ctx.image_watcher->flush(ctx); +} + +template +Context *AcquireRequest::handle_flush_notifies(int *ret_val) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 10) << __func__ << dendl; + + assert(*ret_val == 0); send_lock(); + return nullptr; } template diff --git a/src/librbd/exclusive_lock/AcquireRequest.h b/src/librbd/exclusive_lock/AcquireRequest.h index e3a25257e4c06..dc03829af4feb 100644 --- a/src/librbd/exclusive_lock/AcquireRequest.h +++ b/src/librbd/exclusive_lock/AcquireRequest.h @@ -35,6 +35,10 @@ private: * @verbatim * * + * | + * v + * FLUSH_NOTIFIES + * | * | /---------------------------------------------------------\ * | | | * | | (no lockers) | @@ -81,6 +85,9 @@ private: int m_error_result; + void send_flush_notifies(); + Context *handle_flush_notifies(int *ret_val); + void send_lock(); Context *handle_lock(int *ret_val); diff --git a/src/librbd/exclusive_lock/ReleaseRequest.cc b/src/librbd/exclusive_lock/ReleaseRequest.cc index a96e97b5b37d5..4885e9ce5e127 100644 --- a/src/librbd/exclusive_lock/ReleaseRequest.cc +++ b/src/librbd/exclusive_lock/ReleaseRequest.cc @@ -11,6 +11,7 @@ #include "librbd/AioImageRequestWQ.h" #include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" +#include "librbd/ImageWatcher.h" #include "librbd/Journal.h" #include "librbd/ObjectMap.h" #include "librbd/Utils.h" @@ -105,6 +106,27 @@ Context *ReleaseRequest::handle_cancel_op_requests(int *ret_val) { m_on_releasing = nullptr; } + send_flush_notifies(); + return nullptr; +} + +template +void ReleaseRequest::send_flush_notifies() { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 10) << __func__ << dendl; + + using klass = ReleaseRequest; + Context *ctx = create_context_callback< + klass, &klass::handle_flush_notifies>(this); + m_image_ctx.image_watcher->flush(ctx); +} + +template +Context *ReleaseRequest::handle_flush_notifies(int *ret_val) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 10) << __func__ << dendl; + + assert(*ret_val == 0); send_close_journal(); return nullptr; } diff --git a/src/librbd/exclusive_lock/ReleaseRequest.h b/src/librbd/exclusive_lock/ReleaseRequest.h index ecb1c21c5374c..c5bf91cca99f6 100644 --- a/src/librbd/exclusive_lock/ReleaseRequest.h +++ b/src/librbd/exclusive_lock/ReleaseRequest.h @@ -36,7 +36,10 @@ private: * BLOCK_WRITES * | * v - * CANCEL_OP_REQUESTS . . . . . . . . . . . . + * CANCEL_OP_REQUESTS + * | + * v + * FLUSH_NOTIFIES . . . . . . . . . . . . . . * | . * v . * CLOSE_JOURNAL . @@ -70,6 +73,9 @@ private: void send_cancel_op_requests(); Context *handle_cancel_op_requests(int *ret_val); + void send_flush_notifies(); + Context *handle_flush_notifies(int *ret_val); + void send_close_journal(); Context *handle_close_journal(int *ret_val); diff --git a/src/librbd/image_watcher/Notifier.cc b/src/librbd/image_watcher/Notifier.cc index fc3d07cbd71f7..7569f7572c1fe 100644 --- a/src/librbd/image_watcher/Notifier.cc +++ b/src/librbd/image_watcher/Notifier.cc @@ -33,8 +33,7 @@ void Notifier::flush(Context *on_finish) { return; } - assert(m_aio_notify_flush == nullptr); - m_aio_notify_flush = on_finish; + m_aio_notify_flush_ctxs.push_back(on_finish); } void Notifier::notify(bufferlist &bl, bufferlist *out_bl, Context *on_finish) { @@ -67,8 +66,11 @@ void Notifier::handle_notify(int r, Context *on_finish) { CephContext *cct = m_image_ctx.cct; ldout(cct, 20) << __func__ << ": pending=" << m_pending_aio_notifies << dendl; - if (m_pending_aio_notifies == 0 && m_aio_notify_flush != nullptr) { - m_image_ctx.op_work_queue->queue(m_aio_notify_flush, 0); + if (m_pending_aio_notifies == 0) { + for (auto ctx : m_aio_notify_flush_ctxs) { + m_image_ctx.op_work_queue->queue(ctx, 0); + } + m_aio_notify_flush_ctxs.clear(); } } diff --git a/src/librbd/image_watcher/Notifier.h b/src/librbd/image_watcher/Notifier.h index 008f6cbfe9a3f..aa45a2e41f42d 100644 --- a/src/librbd/image_watcher/Notifier.h +++ b/src/librbd/image_watcher/Notifier.h @@ -8,6 +8,7 @@ #include "include/buffer.h" #include "include/Context.h" #include "common/Mutex.h" +#include namespace librbd { @@ -26,6 +27,8 @@ public: void notify(bufferlist &bl, bufferlist *out_bl, Context *on_finish); private: + typedef std::list Contexts; + struct C_AioNotify : public Context { Notifier *notifier; Context *on_finish; @@ -42,7 +45,7 @@ private: Mutex m_aio_notify_lock; size_t m_pending_aio_notifies = 0; - Context *m_aio_notify_flush = nullptr; + Contexts m_aio_notify_flush_ctxs; void handle_notify(int r, Context *on_finish); diff --git a/src/librbd/operation/SnapshotCreateRequest.cc b/src/librbd/operation/SnapshotCreateRequest.cc index 39257a78c9ddd..e4a1fc31e166a 100644 --- a/src/librbd/operation/SnapshotCreateRequest.cc +++ b/src/librbd/operation/SnapshotCreateRequest.cc @@ -170,7 +170,8 @@ template Context *SnapshotCreateRequest::handle_allocate_snap_id(int *result) { I &image_ctx = this->m_image_ctx; CephContext *cct = image_ctx.cct; - ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl; + ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << ", " + << "snap_id=" << m_snap_id << dendl; if (*result < 0) { save_result(result); diff --git a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc index e899491fb622b..0d370ac1d957e 100644 --- a/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc +++ b/src/test/librbd/exclusive_lock/test_mock_AcquireRequest.cc @@ -141,6 +141,11 @@ public: exec(mock_image_ctx.header_oid, _, "lock", "break_lock", _, _, _)) .WillOnce(Return(r)); } + + void expect_flush_notifies(MockImageCtx &mock_image_ctx) { + EXPECT_CALL(*mock_image_ctx.image_watcher, flush(_)) + .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue)); + } }; TEST_F(TestMockExclusiveLockAcquireRequest, Success) { @@ -153,6 +158,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, Success) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, 0); MockObjectMap mock_object_map; @@ -185,6 +191,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessJournalDisabled) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, 0); MockObjectMap mock_object_map; @@ -214,6 +221,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, SuccessObjectMapDisabled) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, 0); expect_test_features(mock_image_ctx, RBD_FEATURE_OBJECT_MAP, false); @@ -243,6 +251,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, JournalError) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, 0); MockObjectMap *mock_object_map = new MockObjectMap(); @@ -275,6 +284,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, LockBusy) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); 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", MockExclusiveLock::WATCHER_LOCK_TAG, @@ -302,6 +312,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoError) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, -EBUSY); expect_get_lock_info(mock_image_ctx, -EINVAL, entity_name_t::CLIENT(1), "", "", "", LOCK_EXCLUSIVE); @@ -324,6 +335,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoEmpty) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); expect_lock(mock_image_ctx, -EBUSY); expect_get_lock_info(mock_image_ctx, -ENOENT, entity_name_t::CLIENT(1), "", "", "", LOCK_EXCLUSIVE); @@ -347,6 +359,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoExternalTag) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); 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); @@ -369,6 +382,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoShared) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); 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", MockExclusiveLock::WATCHER_LOCK_TAG, @@ -392,6 +406,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetLockInfoExternalCookie) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); 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", MockExclusiveLock::WATCHER_LOCK_TAG, @@ -415,6 +430,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetWatchersError) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); 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", MockExclusiveLock::WATCHER_LOCK_TAG, @@ -439,6 +455,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, GetWatchersAlive) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); 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", MockExclusiveLock::WATCHER_LOCK_TAG, @@ -464,6 +481,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BlacklistDisabled) { mock_image_ctx.blacklist_on_break_lock = false; InSequence seq; + expect_flush_notifies(mock_image_ctx); 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", MockExclusiveLock::WATCHER_LOCK_TAG, @@ -490,6 +508,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BlacklistError) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); 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", MockExclusiveLock::WATCHER_LOCK_TAG, @@ -515,6 +534,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BreakLockMissing) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); 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", MockExclusiveLock::WATCHER_LOCK_TAG, @@ -542,6 +562,7 @@ TEST_F(TestMockExclusiveLockAcquireRequest, BreakLockError) { expect_op_work_queue(mock_image_ctx); InSequence seq; + expect_flush_notifies(mock_image_ctx); 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", MockExclusiveLock::WATCHER_LOCK_TAG, diff --git a/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc b/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc index b32deee9e8f0f..0647d5ae85afb 100644 --- a/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc +++ b/src/test/librbd/exclusive_lock/test_mock_ReleaseRequest.cc @@ -60,6 +60,11 @@ public: EXPECT_CALL(mock_object_map, close(_)) .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue)); } + + void expect_flush_notifies(MockImageCtx &mock_image_ctx) { + EXPECT_CALL(*mock_image_ctx.image_watcher, flush(_)) + .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue)); + } }; TEST_F(TestMockExclusiveLockReleaseRequest, Success) { @@ -74,6 +79,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, Success) { InSequence seq; expect_block_writes(mock_image_ctx, 0); expect_cancel_op_requests(mock_image_ctx, 0); + expect_flush_notifies(mock_image_ctx); MockJournal *mock_journal = new MockJournal(); mock_image_ctx.journal = mock_journal; @@ -107,6 +113,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, SuccessJournalDisabled) { InSequence seq; expect_cancel_op_requests(mock_image_ctx, 0); + expect_flush_notifies(mock_image_ctx); MockObjectMap *mock_object_map = new MockObjectMap(); mock_image_ctx.object_map = mock_object_map; @@ -136,6 +143,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, SuccessObjectMapDisabled) { InSequence seq; expect_cancel_op_requests(mock_image_ctx, 0); + expect_flush_notifies(mock_image_ctx); expect_unlock(mock_image_ctx, 0); @@ -182,6 +190,7 @@ TEST_F(TestMockExclusiveLockReleaseRequest, UnlockError) { InSequence seq; expect_block_writes(mock_image_ctx, 0); expect_cancel_op_requests(mock_image_ctx, 0); + expect_flush_notifies(mock_image_ctx); expect_unlock(mock_image_ctx, -EINVAL); diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc index 6d8791f204a32..d3502f71ad928 100644 --- a/src/test/librbd/image/test_mock_RefreshRequest.cc +++ b/src/test/librbd/image/test_mock_RefreshRequest.cc @@ -8,6 +8,7 @@ #include "test/librbd/mock/MockObjectMap.h" #include "test/librados_test_stub/MockTestMemIoCtxImpl.h" #include "test/librados_test_stub/MockTestMemRadosClient.h" +#include "librbd/ImageState.h" #include "librbd/internal.h" #include "librbd/Operations.h" #include "librbd/image/RefreshRequest.h" @@ -287,6 +288,7 @@ TEST_F(TestMockImageRefreshRequest, SuccessSnapshotV1) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); ASSERT_EQ(0, snap_create(*ictx, "snap")); + ASSERT_EQ(0, ictx->state->refresh()); MockImageCtx mock_image_ctx(*ictx); expect_op_work_queue(mock_image_ctx); diff --git a/src/test/librbd/mock/MockImageWatcher.h b/src/test/librbd/mock/MockImageWatcher.h index 20164eafa3cc8..d1a7c64f348ba 100644 --- a/src/test/librbd/mock/MockImageWatcher.h +++ b/src/test/librbd/mock/MockImageWatcher.h @@ -6,10 +6,13 @@ #include "gmock/gmock.h" +class Context; + namespace librbd { struct MockImageWatcher { MOCK_METHOD0(unregister_watch, void()); + MOCK_METHOD1(flush, void(Context *)); MOCK_CONST_METHOD0(get_watch_handle, uint64_t()); diff --git a/src/test/librbd/test_ImageWatcher.cc b/src/test/librbd/test_ImageWatcher.cc index c8b245b392dc2..7e7f9eced0fdc 100644 --- a/src/test/librbd/test_ImageWatcher.cc +++ b/src/test/librbd/test_ImageWatcher.cc @@ -300,6 +300,10 @@ TEST_F(TestImageWatcher, NotifyRequestLock) { m_notify_acks = {{NOTIFY_OP_REQUEST_LOCK, {}}}; ictx->image_watcher->notify_request_lock(); + C_SaferCond ctx; + ictx->image_watcher->flush(&ctx); + ctx.wait(); + ASSERT_TRUE(wait_for_notifies(*ictx)); NotifyOps expected_notify_ops; diff --git a/src/test/librbd/test_mock_ExclusiveLock.cc b/src/test/librbd/test_mock_ExclusiveLock.cc index 6913d1d333789..f57dca987b21f 100644 --- a/src/test/librbd/test_mock_ExclusiveLock.cc +++ b/src/test/librbd/test_mock_ExclusiveLock.cc @@ -129,6 +129,11 @@ public: .WillRepeatedly(Return(true)); } + void expect_flush_notifies(MockImageCtx &mock_image_ctx) { + EXPECT_CALL(*mock_image_ctx.image_watcher, flush(_)) + .WillOnce(CompleteContext(0, mock_image_ctx.image_ctx->op_work_queue)); + } + int when_init(MockImageCtx &mock_image_ctx, MockExclusiveLock &exclusive_lock) { C_SaferCond ctx; @@ -262,6 +267,7 @@ TEST_F(TestMockExclusiveLock, TryLockAlreadyLocked) { ASSERT_FALSE(is_lock_owner(mock_image_ctx, exclusive_lock)); expect_unblock_writes(mock_image_ctx); + expect_flush_notifies(mock_image_ctx); ASSERT_EQ(0, when_shut_down(mock_image_ctx, exclusive_lock)); } @@ -285,6 +291,7 @@ TEST_F(TestMockExclusiveLock, TryLockBusy) { ASSERT_FALSE(is_lock_owner(mock_image_ctx, exclusive_lock)); expect_unblock_writes(mock_image_ctx); + expect_flush_notifies(mock_image_ctx); ASSERT_EQ(0, when_shut_down(mock_image_ctx, exclusive_lock)); } @@ -309,6 +316,7 @@ TEST_F(TestMockExclusiveLock, TryLockError) { ASSERT_FALSE(is_lock_owner(mock_image_ctx, exclusive_lock)); expect_unblock_writes(mock_image_ctx); + expect_flush_notifies(mock_image_ctx); ASSERT_EQ(0, when_shut_down(mock_image_ctx, exclusive_lock)); } @@ -359,6 +367,7 @@ TEST_F(TestMockExclusiveLock, RequestLockBlacklist) { ASSERT_FALSE(is_lock_owner(mock_image_ctx, exclusive_lock)); expect_unblock_writes(mock_image_ctx); + expect_flush_notifies(mock_image_ctx); ASSERT_EQ(0, when_shut_down(mock_image_ctx, exclusive_lock)); } @@ -437,6 +446,7 @@ TEST_F(TestMockExclusiveLock, ReleaseLockUnlockedState) { ASSERT_EQ(0, when_release_lock(mock_image_ctx, exclusive_lock)); expect_unblock_writes(mock_image_ctx); + expect_flush_notifies(mock_image_ctx); ASSERT_EQ(0, when_shut_down(mock_image_ctx, exclusive_lock)); } @@ -542,7 +552,7 @@ TEST_F(TestMockExclusiveLock, ConcurrentRequests) { ASSERT_EQ(0, release_lock_ctx1.wait()); expect_unblock_writes(mock_image_ctx); - expect_op_work_queue(mock_image_ctx); + expect_flush_notifies(mock_image_ctx); ASSERT_EQ(0, when_shut_down(mock_image_ctx, exclusive_lock)); } -- 2.39.5