From: Jason Dillaman <dillaman@redhat.com>
Date: Tue, 3 Jan 2017 20:29:17 +0000 (-0500)
Subject: librbd: fail immediately if the exclusive lock cannot be acquired
X-Git-Tag: v12.0.0~263^2~3
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=47b89f4d48a0b99876462167389df28893a8d418;p=ceph.git

librbd: fail immediately if the exclusive lock cannot be acquired

Fixes: http://tracker.ceph.com/issues/16988
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---

diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc
index 58fc21e78a0..b8d740a9490 100644
--- a/src/librbd/ExclusiveLock.cc
+++ b/src/librbd/ExclusiveLock.cc
@@ -141,7 +141,7 @@ void ExclusiveLock<I>::shut_down(Context *on_shut_down) {
   }
 
   // if stalled in request state machine -- abort
-  handle_peer_notification();
+  handle_peer_notification(0);
 }
 
 template <typename I>
@@ -233,15 +233,23 @@ void ExclusiveLock<I>::reacquire_lock(Context *on_reacquired) {
 }
 
 template <typename I>
-void ExclusiveLock<I>::handle_peer_notification() {
-  Mutex::Locker locker(m_lock);
-  if (m_state != STATE_WAITING_FOR_PEER) {
-    return;
+void ExclusiveLock<I>::handle_peer_notification(int r) {
+  {
+    Mutex::Locker locker(m_lock);
+    if (m_state != STATE_WAITING_FOR_PEER) {
+      return;
+    }
+
+    ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl;
+    assert(get_active_action() == ACTION_REQUEST_LOCK);
+
+    if (r >= 0) {
+      execute_next_action();
+      return;
+    }
   }
 
-  ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl;
-  assert(get_active_action() == ACTION_REQUEST_LOCK);
-  execute_next_action();
+  handle_acquire_lock(r);
 }
 
 template <typename I>
@@ -445,12 +453,13 @@ void ExclusiveLock<I>::handle_acquire_lock(int r) {
   {
     m_lock.Lock();
     assert(m_state == STATE_ACQUIRING ||
-           m_state == STATE_POST_ACQUIRING);
+           m_state == STATE_POST_ACQUIRING ||
+           m_state == STATE_WAITING_FOR_PEER);
 
     Action action = get_active_action();
     assert(action == ACTION_TRY_LOCK || action == ACTION_REQUEST_LOCK);
     if (action == ACTION_REQUEST_LOCK && r < 0 && r != -EBLACKLISTED &&
-        r != -EPERM) {
+        r != -EPERM && r != -EROFS) {
       m_state = STATE_WAITING_FOR_PEER;
       m_lock.Unlock();
 
diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h
index c313a895981..3048c2bdbcb 100644
--- a/src/librbd/ExclusiveLock.h
+++ b/src/librbd/ExclusiveLock.h
@@ -43,7 +43,7 @@ public:
 
   void reacquire_lock(Context *on_reacquired = nullptr);
 
-  void handle_peer_notification();
+  void handle_peer_notification(int r);
 
   static bool decode_lock_cookie(const std::string &cookie, uint64_t *handle);
 
diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc
index 12dfb0c41db..d312260b368 100644
--- a/src/librbd/ImageWatcher.cc
+++ b/src/librbd/ImageWatcher.cc
@@ -506,7 +506,10 @@ void ImageWatcher<I>::handle_request_lock(int r) {
                               << dendl;
 
     // treat this is a dead client -- so retest acquiring the lock
-    m_image_ctx.exclusive_lock->handle_peer_notification();
+    m_image_ctx.exclusive_lock->handle_peer_notification(0);
+  } else if (r == -EROFS) {
+    ldout(m_image_ctx.cct, 5) << this << " peer will not release lock" << dendl;
+    m_image_ctx.exclusive_lock->handle_peer_notification(r);
   } else if (r < 0) {
     lderr(m_image_ctx.cct) << this << " error requesting lock: "
                            << cpp_strerror(r) << dendl;
@@ -652,7 +655,7 @@ bool ImageWatcher<I>::handle_payload(const AcquiredLockPayload &payload,
   if (m_image_ctx.exclusive_lock != nullptr) {
     // potentially wake up the exclusive lock state machine now that
     // a lock owner has advertised itself
-    m_image_ctx.exclusive_lock->handle_peer_notification();
+    m_image_ctx.exclusive_lock->handle_peer_notification(0);
   }
   if (cancel_async_requests &&
       (m_image_ctx.exclusive_lock == nullptr ||
@@ -691,7 +694,7 @@ bool ImageWatcher<I>::handle_payload(const ReleasedLockPayload &payload,
   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_peer_notification();
+    m_image_ctx.exclusive_lock->handle_peer_notification(0);
   }
   return true;
 }
diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc
index 808d145f2b8..0b2e0b21281 100644
--- a/src/test/librbd/test_librbd.cc
+++ b/src/test/librbd/test_librbd.cc
@@ -4972,7 +4972,15 @@ TEST_F(TestLibRBD, ExclusiveLock)
 	}
 
 	std::cout << m_id << ": acquiring exclusive lock" << std::endl;
-	EXPECT_EQ(0, rbd_lock_acquire(m_image, RBD_LOCK_MODE_EXCLUSIVE));
+        int r;
+        do {
+          r = rbd_lock_acquire(m_image, RBD_LOCK_MODE_EXCLUSIVE);
+          if (r == -EROFS) {
+            usleep(1000);
+          }
+        } while (r == -EROFS);
+	EXPECT_EQ(0, r);
+
 	int lock_owner;
 	EXPECT_EQ(0, rbd_is_exclusive_lock_owner(m_image, &lock_owner));
 	EXPECT_TRUE(lock_owner);
diff --git a/src/test/librbd/test_mock_ExclusiveLock.cc b/src/test/librbd/test_mock_ExclusiveLock.cc
index 5d6eb17458e..df6a4ae35f1 100644
--- a/src/test/librbd/test_mock_ExclusiveLock.cc
+++ b/src/test/librbd/test_mock_ExclusiveLock.cc
@@ -165,8 +165,9 @@ public:
   void expect_notify_request_lock(MockExclusiveLockImageCtx &mock_image_ctx,
                                   MockExclusiveLock &mock_exclusive_lock) {
     EXPECT_CALL(*mock_image_ctx.image_watcher, notify_request_lock())
-                  .WillRepeatedly(Invoke(&mock_exclusive_lock,
-                                         &MockExclusiveLock::handle_peer_notification));
+                  .WillRepeatedly(Invoke([&mock_exclusive_lock]() {
+                                           mock_exclusive_lock.handle_peer_notification(0);
+                                         }));
   }
 
   void expect_notify_acquired_lock(MockExclusiveLockImageCtx &mock_image_ctx) {