librbd: notifications should be flushed between exclusive lock states

Avoid leaving in-flight notification messages when transitioning lock
states.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc
index 1d0d189eadb62dc964ce18767addf4fccb65ea25..1865539e9f4343d8f4f0f4f1138d7cd26986b81a 100644 (file)
--- a/src/librbd/ExclusiveLock.cc
+++ b/src/librbd/ExclusiveLock.cc
@@ -462,8 +462,8 @@ void ExclusiveLock<I>::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<I>, &ExclusiveLock<I>::complete_shutdown>(this), 0);
+    m_image_ctx.image_watcher->flush(util::create_context_callback<
+      ExclusiveLock<I>, &ExclusiveLock<I>::complete_shutdown>(this));
     return;
   }
 

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index 6e29292bb8dd911858e7e979ca6ade3ffb09dcf9..762d637421e2f6ccf6baecf9fd39da6528db203d 100644 (file)
--- 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 35b5bf9b1aa146a04a8359f4288ab6e0a0322622..b1786f7e18ee6202ea84883df97fd19ca228dbbf 100644 (file)
--- 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 f3a6f4bfe0ff86c5add360cc821c7c588a2f760f..a40cdc7e29c000bdb3c2cbc37a79a4462c57f0af 100644 (file)
--- a/src/librbd/WatchNotifyTypes.cc
+++ b/src/librbd/WatchNotifyTypes.cc
@@ -11,6 +11,14 @@ namespace watch_notify {
 
 namespace {
 
+class CheckForRefreshVisitor  : public boost::static_visitor<bool> {
+public:
+  template <typename Payload>
+  inline bool operator()(const Payload &payload) const {
+    return Payload::CHECK_FOR_REFRESH;
+  }
+};
+
 class EncodePayloadVisitor : public boost::static_visitor<void> {
 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 468b45ab475c318d236c47510129c6e8cbe24467..a587b231d66c5a53c15d370f745d8172077de429 100644 (file)
--- 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<NotifyOp>(-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 5f4986948a6fd3eff96a14b8e60bfc5118a3de96..e46714778a368a8e5a252c90c0790c6fe60d0202 100644 (file)
--- 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<I>::~AcquireRequest() {
 
 template <typename I>
 void AcquireRequest<I>::send() {
+  send_flush_notifies();
+}
+
+template <typename I>
+void AcquireRequest<I>::send_flush_notifies() {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << __func__ << dendl;
+
+  using klass = AcquireRequest<I>;
+  Context *ctx = create_context_callback<klass, &klass::handle_flush_notifies>(
+    this);
+  m_image_ctx.image_watcher->flush(ctx);
+}
+
+template <typename I>
+Context *AcquireRequest<I>::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 <typename I>

diff --git a/src/librbd/exclusive_lock/AcquireRequest.h b/src/librbd/exclusive_lock/AcquireRequest.h
index e3a25257e4c0646589b01f6022a71fd4e28e59ef..dc03829af4feb513d8d99953fe1dfbd55be1a724 100644 (file)
--- a/src/librbd/exclusive_lock/AcquireRequest.h
+++ b/src/librbd/exclusive_lock/AcquireRequest.h
@@ -35,6 +35,10 @@ private:
    * @verbatim
    *
    * <start>
+   *    |
+   *    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 a96e97b5b37d513ebd8d16bd7d01fcf6c5fd2b58..4885e9ce5e127b98c881a94b1a3a85fdd095cfe0 100644 (file)
--- 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<I>::handle_cancel_op_requests(int *ret_val) {
     m_on_releasing = nullptr;
   }
 
+  send_flush_notifies();
+  return nullptr;
+}
+
+template <typename I>
+void ReleaseRequest<I>::send_flush_notifies() {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << __func__ << dendl;
+
+  using klass = ReleaseRequest<I>;
+  Context *ctx = create_context_callback<
+    klass, &klass::handle_flush_notifies>(this);
+  m_image_ctx.image_watcher->flush(ctx);
+}
+
+template <typename I>
+Context *ReleaseRequest<I>::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 ecb1c21c5374c36debae6cb739af3729d88f8354..c5bf91cca99f6f782ef5107f9d2e3c1be69de52d 100644 (file)
--- 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 fc3d07cbd71f7608ec6dcc3a587e8f4bc85d5aed..7569f7572c1fe50b3e7ee7a4e6f41a9805fce216 100644 (file)
--- 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 008f6cbfe9a3fbe506d1b9e19915d04643027576..aa45a2e41f42de65f37db8524ce43f4b57429c17 100644 (file)
--- 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 <list>
 
 namespace librbd {
 
@@ -26,6 +27,8 @@ public:
   void notify(bufferlist &bl, bufferlist *out_bl, Context *on_finish);
 
 private:
+  typedef std::list<Context*> 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 39257a78c9ddda4cf6f019fc67d8af1131856b30..e4a1fc31e166a5bb9fbfdd737f5b7c0ab10d843c 100644 (file)
--- a/src/librbd/operation/SnapshotCreateRequest.cc
+++ b/src/librbd/operation/SnapshotCreateRequest.cc
@@ -170,7 +170,8 @@ template <typename I>
 Context *SnapshotCreateRequest<I>::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 e899491fb622b76aa5d1feb44e9d427edd0cbca4..0d370ac1d957e58407eb9377db1cdf047727d1d2 100644 (file)
--- 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 b32deee9e8f0f2ed73a01bb62859bdb463ad8917..0647d5ae85afb2d579a43d5611228ea6d6ead269 100644 (file)
--- 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 6d8791f204a329b16b88427bdc028850ed6983e9..d3502f71ad928f9f5200667b38b7cb0677a72ed7 100644 (file)
--- 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 20164eafa3cc8710da22fc0e5039be742f81fae2..d1a7c64f348bacf5e5412d69eb4f6117de73dc55 100644 (file)
--- 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 c8b245b392dc2d136b5354c2268502a60ecd9307..7e7f9eced0fdc75ca1736b1c3c89e412278f6a7e 100644 (file)
--- 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 6913d1d3337893e73dd8b3b1d0fdf425717a3617..f57dca987b21fe82305653ce9a43a651a99f2c24 100644 (file)
--- 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));
 }