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>

diff --git a/src/librbd/ManagedLock.cc b/src/librbd/ManagedLock.cc
index a9c6f6234fa6d19a4fa55168291600a92666cb57..53bfd2622665531d0272e8897b8f26e5a059bee5 100644 (file)
--- 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 c619f4823dd226adc5776c454b76b9019c38396e..fdb6d43d78c417e32a583e928fa28ea8707f9504 100644 (file)
--- 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 4cd72a13d202fe55182c94eff0bc695c7160f11b..e4b8c4c0b0c10ccffed9ef333da6114dfa35cfca 100644 (file)
--- 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>
@@ -192,6 +204,17 @@ 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;
     {
@@ -503,4 +526,33 @@ 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(0, 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