librbd: avoid generating ESHUTDOWN in ManagedLock

EBLOCKLISTED has a very special meaning but happens to be an alias for
ESHUTDOWN.  If the client gets blocklisted, we always want to propagate
EBLOCKLISTED error code since it's generated by the OSD.

For ManagedLock use case of indicating that an operation on the lock
raced with lock shut down, meaning that a higher level request can just
be restarted, ERESTART should do.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit 76856d90e0e0d73276c06a724091197301f982e9)

diff --git a/src/librbd/ManagedLock.cc b/src/librbd/ManagedLock.cc
index 7b785a261a399a62b22d93ae6a934b5f30f5b207..bb11160cb5412b07b16ab91ac04078f49cecd5da 100644 (file)
--- a/src/librbd/ManagedLock.cc
+++ b/src/librbd/ManagedLock.cc
@@ -139,7 +139,7 @@ void ManagedLock<I>::shut_down(Context *on_shut_down) {
     Action active_action = get_active_action();
     ceph_assert(active_action == ACTION_TRY_LOCK ||
                 active_action == ACTION_ACQUIRE_LOCK);
-    complete_active_action(STATE_UNLOCKED, -ESHUTDOWN);
+    complete_active_action(STATE_UNLOCKED, -ERESTART);
   }
 
   execute_action(ACTION_SHUT_DOWN, on_shut_down);
@@ -151,7 +151,7 @@ void ManagedLock<I>::acquire_lock(Context *on_acquired) {
   {
     std::lock_guard locker{m_lock};
     if (is_state_shutdown()) {
-      r = -ESHUTDOWN;
+      r = -ERESTART;
     } else if (m_state != STATE_LOCKED || !m_actions_contexts.empty()) {
       ldout(m_cct, 10) << dendl;
       execute_action(ACTION_ACQUIRE_LOCK, on_acquired);
@@ -170,7 +170,7 @@ void ManagedLock<I>::try_acquire_lock(Context *on_acquired) {
   {
     std::lock_guard locker{m_lock};
     if (is_state_shutdown()) {
-      r = -ESHUTDOWN;
+      r = -ERESTART;
     } else if (m_state != STATE_LOCKED || !m_actions_contexts.empty()) {
       ldout(m_cct, 10) << dendl;
       execute_action(ACTION_TRY_LOCK, on_acquired);
@@ -189,7 +189,7 @@ void ManagedLock<I>::release_lock(Context *on_released) {
   {
     std::lock_guard locker{m_lock};
     if (is_state_shutdown()) {
-      r = -ESHUTDOWN;
+      r = -ERESTART;
     } else if (m_state != STATE_UNLOCKED || !m_actions_contexts.empty()) {
       ldout(m_cct, 10) << dendl;
       execute_action(ACTION_RELEASE_LOCK, on_released);
@@ -241,7 +241,7 @@ void ManagedLock<I>::get_locker(managed_lock::Locker *locker,
   {
     std::lock_guard l{m_lock};
     if (is_state_shutdown()) {
-      r = -ESHUTDOWN;
+      r = -ERESTART;
     } else {
       on_finish = new C_Tracked(m_async_op_tracker, on_finish);
       auto req = managed_lock::GetLockerRequest<I>::create(
@@ -263,7 +263,7 @@ void ManagedLock<I>::break_lock(const managed_lock::Locker &locker,
   {
     std::lock_guard l{m_lock};
     if (is_state_shutdown()) {
-      r = -ESHUTDOWN;
+      r = -ERESTART;
     } else if (is_lock_owner(m_lock)) {
       r = -EBUSY;
     } else {
@@ -490,7 +490,7 @@ void ManagedLock<I>::send_acquire_lock() {
 
       // shut down might race w/ release/re-acquire of the lock
       if (is_state_shutdown()) {
-        complete_active_action(STATE_UNLOCKED, -ESHUTDOWN);
+        complete_active_action(STATE_UNLOCKED, -ERESTART);
       }
     }
     return;

diff --git a/src/librbd/exclusive_lock/ImageDispatch.cc b/src/librbd/exclusive_lock/ImageDispatch.cc
index 6b403e2e3156e44aeeb13112969f03ca75b84ac0..f53b3692a1b1c25505a212807e2d5ccb2d7a4f3b 100644 (file)
--- a/src/librbd/exclusive_lock/ImageDispatch.cc
+++ b/src/librbd/exclusive_lock/ImageDispatch.cc
@@ -293,7 +293,7 @@ void ImageDispatch<I>::handle_acquire_lock(int r) {
 
   Context* failed_dispatch = nullptr;
   Contexts on_dispatches;
-  if (r == -ESHUTDOWN) {
+  if (r == -ERESTART) {
     ldout(cct, 5) << "IO raced with exclusive lock shutdown" << dendl;
   } else if (r < 0) {
     lderr(cct) << "failed to acquire exclusive lock: " << cpp_strerror(r)

diff --git a/src/test/librbd/test_internal.cc b/src/test/librbd/test_internal.cc
index 4a793a021b6024356951bb1cc3d50186af78ccce..b57d0b074c34f5ac4308c70c01ffdf7dca169619 100644 (file)
--- a/src/test/librbd/test_internal.cc
+++ b/src/test/librbd/test_internal.cc
@@ -4,6 +4,7 @@
 #include "cls/journal/cls_journal_client.h"
 #include "cls/rbd/cls_rbd_client.h"
 #include "cls/rbd/cls_rbd_types.h"
+#include "test/librados/test_cxx.h"
 #include "test/librbd/test_fixture.h"
 #include "test/librbd/test_support.h"
 #include "include/rbd/librbd.h"
@@ -374,6 +375,77 @@ TEST_F(TestInternal, FlattenFailsToLockImage) {
   ASSERT_EQ(-EROFS, ictx2->operations->flatten(no_op));
 }
 
+TEST_F(TestInternal, WriteFailsToLockImageBlocklisted) {
+  REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK);
+
+  librados::Rados blocklist_rados;
+  ASSERT_EQ("", connect_cluster_pp(blocklist_rados));
+
+  librados::IoCtx blocklist_ioctx;
+  ASSERT_EQ(0, blocklist_rados.ioctx_create(_pool_name.c_str(),
+                                            blocklist_ioctx));
+
+  auto ictx = new librbd::ImageCtx(m_image_name, "", nullptr, blocklist_ioctx,
+                                   false);
+  ASSERT_EQ(0, ictx->state->open(0));
+
+  std::list<librbd::image_watcher_t> watchers;
+  ASSERT_EQ(0, librbd::list_watchers(ictx, watchers));
+  ASSERT_EQ(1U, watchers.size());
+
+  bool lock_owner;
+  ASSERT_EQ(0, librbd::is_exclusive_lock_owner(ictx, &lock_owner));
+  ASSERT_FALSE(lock_owner);
+
+  ASSERT_EQ(0, blocklist_rados.blocklist_add(watchers.front().addr, 0));
+
+  ceph::bufferlist bl;
+  bl.append(std::string(256, '1'));
+  ASSERT_EQ(-EBLOCKLISTED, api::Io<>::write(*ictx, 0, bl.length(),
+                                            std::move(bl), 0));
+  ASSERT_EQ(-EBLOCKLISTED, librbd::is_exclusive_lock_owner(ictx, &lock_owner));
+
+  close_image(ictx);
+}
+
+TEST_F(TestInternal, WriteFailsToLockImageBlocklistedWatch) {
+  REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK);
+
+  librados::Rados blocklist_rados;
+  ASSERT_EQ("", connect_cluster_pp(blocklist_rados));
+
+  librados::IoCtx blocklist_ioctx;
+  ASSERT_EQ(0, blocklist_rados.ioctx_create(_pool_name.c_str(),
+                                            blocklist_ioctx));
+
+  auto ictx = new librbd::ImageCtx(m_image_name, "", nullptr, blocklist_ioctx,
+                                   false);
+  ASSERT_EQ(0, ictx->state->open(0));
+
+  std::list<librbd::image_watcher_t> watchers;
+  ASSERT_EQ(0, librbd::list_watchers(ictx, watchers));
+  ASSERT_EQ(1U, watchers.size());
+
+  bool lock_owner;
+  ASSERT_EQ(0, librbd::is_exclusive_lock_owner(ictx, &lock_owner));
+  ASSERT_FALSE(lock_owner);
+
+  ASSERT_EQ(0, blocklist_rados.blocklist_add(watchers.front().addr, 0));
+  // let ImageWatcher discover that the watch can't be re-registered to
+  // eliminate the (intended) race in WriteFailsToLockImageBlocklisted
+  while (!ictx->image_watcher->is_blocklisted()) {
+    sleep(1);
+  }
+
+  ceph::bufferlist bl;
+  bl.append(std::string(256, '1'));
+  ASSERT_EQ(-EBLOCKLISTED, api::Io<>::write(*ictx, 0, bl.length(),
+                                            std::move(bl), 0));
+  ASSERT_EQ(-EBLOCKLISTED, librbd::is_exclusive_lock_owner(ictx, &lock_owner));
+
+  close_image(ictx);
+}
+
 TEST_F(TestInternal, AioWriteRequestsLock) {
   REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK);
 

diff --git a/src/test/librbd/test_mock_ManagedLock.cc b/src/test/librbd/test_mock_ManagedLock.cc
index 29aca2d100d5b7b944d40c5b3ae5a5e6c51c135f..202072997498baae67195c594e725db5ace5f05a 100644 (file)
--- a/src/test/librbd/test_mock_ManagedLock.cc
+++ b/src/test/librbd/test_mock_ManagedLock.cc
@@ -714,7 +714,7 @@ TEST_F(TestMockManagedLock, ShutDownWhileWaiting) {
   managed_lock.acquire_lock(&acquire_ctx);
 
   ASSERT_EQ(0, when_shut_down(managed_lock));
-  ASSERT_EQ(-ESHUTDOWN, acquire_ctx.wait());
+  ASSERT_EQ(-ERESTART, acquire_ctx.wait());
   ASSERT_FALSE(is_lock_owner(managed_lock));
 }