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>

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 06423d4f105a157827ed24102080c60d54e78ac6..0ec4462687d7546e526193ea42a7ee7005cbc484 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"
@@ -376,6 +377,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 dfa0f84fe101f3d970028ac29245feb9c8b62305..e3d060771ffb1e61844380ebdb3e9fc8d0fa3091 100644 (file)
--- a/src/test/librbd/test_mock_ManagedLock.cc
+++ b/src/test/librbd/test_mock_ManagedLock.cc
@@ -716,7 +716,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));
 }