From: Song Shun <song.shun3@zte.com.cn>
Date: Tue, 20 Mar 2018 03:07:28 +0000 (+0800)
Subject: librbd: fix refuse to release lock when cookie is the same at rewatch
X-Git-Tag: v12.2.9~107^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=e04a0d3644f1350f87ec48f0f00b4fe2f1c2bccb;p=ceph.git

librbd: fix refuse to release lock when cookie is the same at rewatch

  fix exclusive auto-mode lock refuse to release.
  when rewatch, owner_id is reset.
  at the same time, there is a chance to produce the same cookie, which should be different.
  code now skips reacquire lock when the cookie is the same, resulting in unsetting owner_id.
  when other clients request lock, client whose owner_id is null is considered invalid and
  refuse to release lock.
  but unluckily, watcher is always alive, so the client requested lock can't get lock.

Signed-off-by: Song Shun <song.shun3@zte.com.cn>
(cherry picked from commit 51ac19387c58dd81f66dce77fef4e0e36ce524ea)
---

diff --git a/src/librbd/ManagedLock.cc b/src/librbd/ManagedLock.cc
index f584d6af5fb5..55199a604a86 100644
--- a/src/librbd/ManagedLock.cc
+++ b/src/librbd/ManagedLock.cc
@@ -512,6 +512,13 @@ void ManagedLock<I>::handle_acquire_lock(int r) {
   }));
 }
 
+template <typename I>
+void ManagedLock<I>::handle_no_op_reacquire_lock(int r) {
+  ldout(m_cct, 10) << "r=" << r << dendl;
+  assert(r >= 0);
+  complete_active_action(STATE_LOCKED, 0);
+}
+
 template <typename I>
 void ManagedLock<I>::handle_post_acquire_lock(int r) {
   ldout(m_cct, 10) << ": r=" << r << dendl;
@@ -564,7 +571,9 @@ void ManagedLock<I>::send_reacquire_lock() {
   if (m_cookie == m_new_cookie) {
     ldout(m_cct, 10) << ": skipping reacquire since cookie still valid"
                      << dendl;
-    complete_active_action(STATE_LOCKED, 0);
+    auto ctx = create_context_callback<
+      ManagedLock, &ManagedLock<I>::handle_no_op_reacquire_lock>(this);
+    post_reacquire_lock_handler(0, ctx);
     return;
   }
 
diff --git a/src/librbd/ManagedLock.h b/src/librbd/ManagedLock.h
index c619f4823dd2..fdb6d43d78c4 100644
--- a/src/librbd/ManagedLock.h
+++ b/src/librbd/ManagedLock.h
@@ -240,6 +240,8 @@ private:
   void send_acquire_lock();
   void handle_pre_acquire_lock(int r);
   void handle_acquire_lock(int r);
+  void handle_no_op_reacquire_lock(int r);
+
   void handle_post_acquire_lock(int r);
   void revert_to_unlock_state(int r);
 
diff --git a/src/test/librbd/test_mock_ManagedLock.cc b/src/test/librbd/test_mock_ManagedLock.cc
index a808523fc204..550b51fc61ec 100644
--- a/src/test/librbd/test_mock_ManagedLock.cc
+++ b/src/test/librbd/test_mock_ManagedLock.cc
@@ -26,6 +26,18 @@ struct Traits<MockManagedLockImageCtx> {
 };
 }
 
+struct MockMockManagedLock : public ManagedLock<MockManagedLockImageCtx> {
+  MockMockManagedLock(librados::IoCtx& ioctx, ContextWQ *work_queue,
+                 const std::string& oid, librbd::MockImageWatcher *watcher,
+                 managed_lock::Mode  mode, bool blacklist_on_break_lock,
+                 uint32_t blacklist_expire_seconds)
+    : ManagedLock<MockManagedLockImageCtx>(ioctx, work_queue, oid, watcher,
+      librbd::managed_lock::EXCLUSIVE, true, 0) {
+  };
+  virtual ~MockMockManagedLock() = default;
+  MOCK_METHOD2(post_reacquire_lock_handler, void(int, Context*));
+};
+
 namespace managed_lock {
 
 template<typename T>
@@ -188,6 +200,18 @@ public:
                   .WillOnce(CompleteContext(0, (ContextWQ *)nullptr));
   }
 
+  void expect_post_reacquired_lock_handler(MockImageWatcher& watcher,
+                        MockMockManagedLock &managed_lock,
+                        uint64_t &client_id) {
+    expect_get_watch_handle(watcher);
+    EXPECT_CALL(managed_lock, post_reacquire_lock_handler(_, _))
+      .WillOnce(Invoke([&watcher, &client_id](int r, Context *on_finish){
+        if (r >= 0) {
+          client_id = 98765;
+        }
+        on_finish->complete(r);}));
+  }
+
   int when_acquire_lock(MockManagedLock &managed_lock) {
     C_SaferCond ctx;
     {
@@ -499,4 +523,35 @@ TEST_F(TestMockManagedLock, ReacquireLockError) {
   ASSERT_FALSE(is_lock_owner(managed_lock));
 }
 
+TEST_F(TestMockManagedLock, ReacquireWithSameCookie) {
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockManagedLockImageCtx mock_image_ctx(*ictx);
+  MockMockManagedLock managed_lock(ictx->md_ctx, ictx->op_work_queue,
+                               ictx->header_oid, mock_image_ctx.image_watcher,
+                               librbd::managed_lock::EXCLUSIVE, true, 0);
+  InSequence seq;
+
+  MockAcquireRequest request_lock_acquire;
+  expect_acquire_lock(*mock_image_ctx.image_watcher, ictx->op_work_queue,
+                      request_lock_acquire, 0);
+  ASSERT_EQ(0, when_acquire_lock(managed_lock));
+  ASSERT_TRUE(is_lock_owner(managed_lock));
+
+  // watcher with same cookie after rewatch
+  uint64_t client_id = 0;
+  C_SaferCond reacquire_ctx;
+  expect_post_reacquired_lock_handler(*mock_image_ctx.image_watcher,
+                                      managed_lock, client_id);
+  managed_lock.reacquire_lock(&reacquire_ctx);
+  ASSERT_LT(0U, client_id);
+  ASSERT_TRUE(is_lock_owner(managed_lock));
+
+  MockReleaseRequest shutdown_release;
+  expect_release_lock(ictx->op_work_queue, shutdown_release, 0);
+  //ASSERT_EQ(0, when_release_lock(managed_lock));
+  ASSERT_EQ(0, when_shut_down(managed_lock));
+}
+
 } // namespace librbd