librbd: optionally block proxied requests with an error code

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit 93e2faf38e866fb3e32a7b3f3527d97215c60d31)

diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc
index 932fe042d3e79ed0e31e36bc665130e61dbb3b5d..c3ba7c72830a1542c8d7846ea75ebc6f1db08023 100644 (file)
--- a/src/librbd/ExclusiveLock.cc
+++ b/src/librbd/ExclusiveLock.cc
@@ -75,33 +75,36 @@ bool ExclusiveLock<I>::is_lock_owner() const {
 }
 
 template <typename I>
-bool ExclusiveLock<I>::accept_requests() const {
+bool ExclusiveLock<I>::accept_requests(int *ret_val) const {
   Mutex::Locker locker(m_lock);
 
   bool accept_requests = (!is_shutdown() && m_state == STATE_LOCKED &&
-                          m_request_blockers == 0);
+                          !m_request_blocked);
+  *ret_val = m_request_blocked_ret_val;
+
   ldout(m_image_ctx.cct, 20) << this << " " << __func__ << "="
                              << accept_requests << dendl;
   return accept_requests;
 }
 
 template <typename I>
-void ExclusiveLock<I>::block_requests() {
+void ExclusiveLock<I>::block_requests(int r) {
   Mutex::Locker locker(m_lock);
-  ++m_request_blockers;
+  assert(!m_request_blocked);
+  m_request_blocked = true;
+  m_request_blocked_ret_val = r;
 
-  ldout(m_image_ctx.cct, 20) << this << " " << __func__ << "="
-                             << m_request_blockers << dendl;
+  ldout(m_image_ctx.cct, 20) << this << " " << __func__ << dendl;
 }
 
 template <typename I>
 void ExclusiveLock<I>::unblock_requests() {
   Mutex::Locker locker(m_lock);
-  assert(m_request_blockers > 0);
-  --m_request_blockers;
+  assert(m_request_blocked);
+  m_request_blocked = false;
+  m_request_blocked_ret_val = 0;
 
-  ldout(m_image_ctx.cct, 20) << this << " " << __func__ << "="
-                             << m_request_blockers << dendl;
+  ldout(m_image_ctx.cct, 20) << this << " " << __func__ << dendl;
 }
 
 template <typename I>

diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h
index 6268c8080be11af3366e18c760f107442fe4013f..0a2a88e2949681cbf5dcd829bc09870763e07543 100644 (file)
--- a/src/librbd/ExclusiveLock.h
+++ b/src/librbd/ExclusiveLock.h
@@ -30,9 +30,9 @@ public:
   ~ExclusiveLock();
 
   bool is_lock_owner() const;
-  bool accept_requests() const;
+  bool accept_requests(int *ret_val) const;
 
-  void block_requests();
+  void block_requests(int r);
   void unblock_requests();
 
   void init(uint64_t features, Context *on_init);
@@ -130,7 +130,8 @@ private:
 
   ActionsContexts m_actions_contexts;
 
-  uint32_t m_request_blockers = 0;
+  bool m_request_blocked = false;
+  int m_request_blocked_ret_val = 0;
 
   std::string encode_lock_cookie() const;
 

diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc
index d3d4e701a736ee92c2c7e7192c62b337910e34b2..f7531bfcaa6b3d3813d4257d8e7d3d00e8c86e30 100644 (file)
--- a/src/librbd/ImageWatcher.cc
+++ b/src/librbd/ImageWatcher.cc
@@ -613,21 +613,25 @@ bool ImageWatcher::handle_payload(const RequestLockPayload &payload,
   }
 
   RWLock::RLocker l(m_image_ctx.owner_lock);
-  if (m_image_ctx.exclusive_lock != nullptr &&
-      m_image_ctx.exclusive_lock->accept_requests()) {
-    // need to send something back so the client can detect a missing leader
-    ::encode(ResponseMessage(0), ack_ctx->out);
-
-    {
-      Mutex::Locker owner_client_id_locker(m_owner_client_id_lock);
-      if (!m_owner_client_id.is_valid()) {
-       return true;
+  if (m_image_ctx.exclusive_lock != nullptr) {
+    int r;
+    if (m_image_ctx.exclusive_lock->accept_requests(&r)) {
+      // need to send something back so the client can detect a missing leader
+      ::encode(ResponseMessage(0), ack_ctx->out);
+
+      {
+        Mutex::Locker owner_client_id_locker(m_owner_client_id_lock);
+        if (!m_owner_client_id.is_valid()) {
+         return true;
+        }
       }
-    }
 
-    ldout(m_image_ctx.cct, 10) << this << " queuing release of exclusive lock"
-                               << dendl;
-    m_image_ctx.get_exclusive_lock_policy()->lock_requested(payload.force);
+      ldout(m_image_ctx.cct, 10) << this << " queuing release of exclusive lock"
+                                 << dendl;
+      m_image_ctx.get_exclusive_lock_policy()->lock_requested(payload.force);
+    } else if (r < 0) {
+      ::encode(ResponseMessage(r), ack_ctx->out);
+    }
   }
   return true;
 }
@@ -664,20 +668,24 @@ bool ImageWatcher::handle_payload(const FlattenPayload &payload,
                                  C_NotifyAck *ack_ctx) {
 
   RWLock::RLocker l(m_image_ctx.owner_lock);
-  if (m_image_ctx.exclusive_lock != nullptr &&
-      m_image_ctx.exclusive_lock->accept_requests()) {
-    bool new_request;
-    Context *ctx;
-    ProgressContext *prog_ctx;
-    int r = prepare_async_request(payload.async_request_id, &new_request,
-                                  &ctx, &prog_ctx);
-    if (new_request) {
-      ldout(m_image_ctx.cct, 10) << this << " remote flatten request: "
-                                << payload.async_request_id << dendl;
-      m_image_ctx.operations->execute_flatten(*prog_ctx, ctx);
-    }
+  if (m_image_ctx.exclusive_lock != nullptr) {
+    int r;
+    if (m_image_ctx.exclusive_lock->accept_requests(&r)) {
+      bool new_request;
+      Context *ctx;
+      ProgressContext *prog_ctx;
+      r = prepare_async_request(payload.async_request_id, &new_request,
+                                &ctx, &prog_ctx);
+      if (new_request) {
+        ldout(m_image_ctx.cct, 10) << this << " remote flatten request: "
+                                  << payload.async_request_id << dendl;
+        m_image_ctx.operations->execute_flatten(*prog_ctx, ctx);
+      }
 
-    ::encode(ResponseMessage(r), ack_ctx->out);
+      ::encode(ResponseMessage(r), ack_ctx->out);
+    } else if (r < 0) {
+      ::encode(ResponseMessage(r), ack_ctx->out);
+    }
   }
   return true;
 }
@@ -685,21 +693,25 @@ bool ImageWatcher::handle_payload(const FlattenPayload &payload,
 bool ImageWatcher::handle_payload(const ResizePayload &payload,
                                  C_NotifyAck *ack_ctx) {
   RWLock::RLocker l(m_image_ctx.owner_lock);
-  if (m_image_ctx.exclusive_lock != nullptr &&
-      m_image_ctx.exclusive_lock->accept_requests()) {
-    bool new_request;
-    Context *ctx;
-    ProgressContext *prog_ctx;
-    int r = prepare_async_request(payload.async_request_id, &new_request,
-                                  &ctx, &prog_ctx);
-    if (new_request) {
-      ldout(m_image_ctx.cct, 10) << this << " remote resize request: "
-                                << payload.async_request_id << " "
-                                << payload.size << dendl;
-      m_image_ctx.operations->execute_resize(payload.size, *prog_ctx, ctx, 0);
-    }
+  if (m_image_ctx.exclusive_lock != nullptr) {
+    int r;
+    if (m_image_ctx.exclusive_lock->accept_requests(&r)) {
+      bool new_request;
+      Context *ctx;
+      ProgressContext *prog_ctx;
+      r = prepare_async_request(payload.async_request_id, &new_request,
+                                &ctx, &prog_ctx);
+      if (new_request) {
+        ldout(m_image_ctx.cct, 10) << this << " remote resize request: "
+                                  << payload.async_request_id << " "
+                                  << payload.size << dendl;
+        m_image_ctx.operations->execute_resize(payload.size, *prog_ctx, ctx, 0);
+      }
 
-    ::encode(ResponseMessage(r), ack_ctx->out);
+      ::encode(ResponseMessage(r), ack_ctx->out);
+    } else if (r < 0) {
+      ::encode(ResponseMessage(r), ack_ctx->out);
+    }
   }
   return true;
 }
@@ -707,15 +719,19 @@ bool ImageWatcher::handle_payload(const ResizePayload &payload,
 bool ImageWatcher::handle_payload(const SnapCreatePayload &payload,
                                  C_NotifyAck *ack_ctx) {
   RWLock::RLocker l(m_image_ctx.owner_lock);
-  if (m_image_ctx.exclusive_lock != nullptr &&
-      m_image_ctx.exclusive_lock->accept_requests()) {
-    ldout(m_image_ctx.cct, 10) << this << " remote snap_create request: "
-                              << payload.snap_name << dendl;
-
-    m_image_ctx.operations->execute_snap_create(payload.snap_name.c_str(),
-                                                new C_ResponseMessage(ack_ctx),
-                                                0, false);
-    return false;
+  if (m_image_ctx.exclusive_lock != nullptr) {
+    int r;
+    if (m_image_ctx.exclusive_lock->accept_requests(&r)) {
+      ldout(m_image_ctx.cct, 10) << this << " remote snap_create request: "
+                                << payload.snap_name << dendl;
+
+      m_image_ctx.operations->execute_snap_create(payload.snap_name.c_str(),
+                                                  new C_ResponseMessage(ack_ctx),
+                                                  0, false);
+      return false;
+    } else if (r < 0) {
+      ::encode(ResponseMessage(r), ack_ctx->out);
+    }
   }
   return true;
 }
@@ -723,16 +739,20 @@ bool ImageWatcher::handle_payload(const SnapCreatePayload &payload,
 bool ImageWatcher::handle_payload(const SnapRenamePayload &payload,
                                  C_NotifyAck *ack_ctx) {
   RWLock::RLocker l(m_image_ctx.owner_lock);
-  if (m_image_ctx.exclusive_lock != nullptr &&
-      m_image_ctx.exclusive_lock->accept_requests()) {
-    ldout(m_image_ctx.cct, 10) << this << " remote snap_rename request: "
-                              << payload.snap_id << " to "
-                              << payload.snap_name << dendl;
-
-    m_image_ctx.operations->execute_snap_rename(payload.snap_id,
-                                                payload.snap_name.c_str(),
-                                                new C_ResponseMessage(ack_ctx));
-    return false;
+  if (m_image_ctx.exclusive_lock != nullptr) {
+    int r;
+    if (m_image_ctx.exclusive_lock->accept_requests(&r)) {
+      ldout(m_image_ctx.cct, 10) << this << " remote snap_rename request: "
+                                << payload.snap_id << " to "
+                                << payload.snap_name << dendl;
+
+      m_image_ctx.operations->execute_snap_rename(payload.snap_id,
+                                                  payload.snap_name.c_str(),
+                                                  new C_ResponseMessage(ack_ctx));
+      return false;
+    } else if (r < 0) {
+      ::encode(ResponseMessage(r), ack_ctx->out);
+    }
   }
   return true;
 }
@@ -740,14 +760,18 @@ bool ImageWatcher::handle_payload(const SnapRenamePayload &payload,
 bool ImageWatcher::handle_payload(const SnapRemovePayload &payload,
                                  C_NotifyAck *ack_ctx) {
   RWLock::RLocker l(m_image_ctx.owner_lock);
-  if (m_image_ctx.exclusive_lock != nullptr &&
-      m_image_ctx.exclusive_lock->accept_requests()) {
-    ldout(m_image_ctx.cct, 10) << this << " remote snap_remove request: "
-                              << payload.snap_name << dendl;
-
-    m_image_ctx.operations->execute_snap_remove(payload.snap_name.c_str(),
-                                                new C_ResponseMessage(ack_ctx));
-    return false;
+  if (m_image_ctx.exclusive_lock != nullptr) {
+    int r;
+    if (m_image_ctx.exclusive_lock->accept_requests(&r)) {
+      ldout(m_image_ctx.cct, 10) << this << " remote snap_remove request: "
+                                << payload.snap_name << dendl;
+
+      m_image_ctx.operations->execute_snap_remove(payload.snap_name.c_str(),
+                                                  new C_ResponseMessage(ack_ctx));
+      return false;
+    } else if (r < 0) {
+      ::encode(ResponseMessage(r), ack_ctx->out);
+    }
   }
   return true;
 }
@@ -755,14 +779,18 @@ bool ImageWatcher::handle_payload(const SnapRemovePayload &payload,
 bool ImageWatcher::handle_payload(const SnapProtectPayload& payload,
                                   C_NotifyAck *ack_ctx) {
   RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
-  if (m_image_ctx.exclusive_lock != nullptr &&
-      m_image_ctx.exclusive_lock->accept_requests()) {
-    ldout(m_image_ctx.cct, 10) << this << " remote snap_protect request: "
-                               << payload.snap_name << dendl;
+  if (m_image_ctx.exclusive_lock != nullptr) {
+    int r;
+    if (m_image_ctx.exclusive_lock->accept_requests(&r)) {
+      ldout(m_image_ctx.cct, 10) << this << " remote snap_protect request: "
+                                 << payload.snap_name << dendl;
 
-    m_image_ctx.operations->execute_snap_protect(payload.snap_name.c_str(),
-                                                 new C_ResponseMessage(ack_ctx));
-    return false;
+      m_image_ctx.operations->execute_snap_protect(payload.snap_name.c_str(),
+                                                   new C_ResponseMessage(ack_ctx));
+      return false;
+    } else if (r < 0) {
+      ::encode(ResponseMessage(r), ack_ctx->out);
+    }
   }
   return true;
 }
@@ -770,14 +798,18 @@ bool ImageWatcher::handle_payload(const SnapProtectPayload& payload,
 bool ImageWatcher::handle_payload(const SnapUnprotectPayload& payload,
                                   C_NotifyAck *ack_ctx) {
   RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
-  if (m_image_ctx.exclusive_lock != nullptr &&
-      m_image_ctx.exclusive_lock->accept_requests()) {
-    ldout(m_image_ctx.cct, 10) << this << " remote snap_unprotect request: "
-                               << payload.snap_name << dendl;
-
-    m_image_ctx.operations->execute_snap_unprotect(payload.snap_name.c_str(),
-                                                   new C_ResponseMessage(ack_ctx));
-    return false;
+  if (m_image_ctx.exclusive_lock != nullptr) {
+    int r;
+    if (m_image_ctx.exclusive_lock->accept_requests(&r)) {
+      ldout(m_image_ctx.cct, 10) << this << " remote snap_unprotect request: "
+                                 << payload.snap_name << dendl;
+
+      m_image_ctx.operations->execute_snap_unprotect(payload.snap_name.c_str(),
+                                                     new C_ResponseMessage(ack_ctx));
+      return false;
+    } else if (r < 0) {
+      ::encode(ResponseMessage(r), ack_ctx->out);
+    }
   }
   return true;
 }
@@ -785,21 +817,25 @@ bool ImageWatcher::handle_payload(const SnapUnprotectPayload& payload,
 bool ImageWatcher::handle_payload(const RebuildObjectMapPayload& payload,
                                   C_NotifyAck *ack_ctx) {
   RWLock::RLocker l(m_image_ctx.owner_lock);
-  if (m_image_ctx.exclusive_lock != nullptr &&
-      m_image_ctx.exclusive_lock->accept_requests()) {
-    bool new_request;
-    Context *ctx;
-    ProgressContext *prog_ctx;
-    int r = prepare_async_request(payload.async_request_id, &new_request,
-                                  &ctx, &prog_ctx);
-    if (new_request) {
-      ldout(m_image_ctx.cct, 10) << this
-                                 << " remote rebuild object map request: "
-                                 << payload.async_request_id << dendl;
-      m_image_ctx.operations->execute_rebuild_object_map(*prog_ctx, ctx);
-    }
+  if (m_image_ctx.exclusive_lock != nullptr) {
+    int r;
+    if (m_image_ctx.exclusive_lock->accept_requests(&r)) {
+      bool new_request;
+      Context *ctx;
+      ProgressContext *prog_ctx;
+      r = prepare_async_request(payload.async_request_id, &new_request,
+                                &ctx, &prog_ctx);
+      if (new_request) {
+        ldout(m_image_ctx.cct, 10) << this
+                                   << " remote rebuild object map request: "
+                                   << payload.async_request_id << dendl;
+        m_image_ctx.operations->execute_rebuild_object_map(*prog_ctx, ctx);
+      }
 
-    ::encode(ResponseMessage(r), ack_ctx->out);
+      ::encode(ResponseMessage(r), ack_ctx->out);
+    } else if (r < 0) {
+      ::encode(ResponseMessage(r), ack_ctx->out);
+    }
   }
   return true;
 }
@@ -807,14 +843,18 @@ bool ImageWatcher::handle_payload(const RebuildObjectMapPayload& payload,
 bool ImageWatcher::handle_payload(const RenamePayload& payload,
                                   C_NotifyAck *ack_ctx) {
   RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
-  if (m_image_ctx.exclusive_lock != nullptr &&
-      m_image_ctx.exclusive_lock->accept_requests()) {
-    ldout(m_image_ctx.cct, 10) << this << " remote rename request: "
-                               << payload.image_name << dendl;
-
-    m_image_ctx.operations->execute_rename(payload.image_name.c_str(),
-                                   new C_ResponseMessage(ack_ctx));
-    return false;
+  if (m_image_ctx.exclusive_lock != nullptr) {
+    int r;
+    if (m_image_ctx.exclusive_lock->accept_requests(&r)) {
+      ldout(m_image_ctx.cct, 10) << this << " remote rename request: "
+                                 << payload.image_name << dendl;
+
+      m_image_ctx.operations->execute_rename(payload.image_name.c_str(),
+                                             new C_ResponseMessage(ack_ctx));
+      return false;
+    } else if (r < 0) {
+      ::encode(ResponseMessage(r), ack_ctx->out);
+    }
   }
   return true;
 }
@@ -822,9 +862,11 @@ bool ImageWatcher::handle_payload(const RenamePayload& payload,
 bool ImageWatcher::handle_payload(const UnknownPayload &payload,
                                  C_NotifyAck *ack_ctx) {
   RWLock::RLocker l(m_image_ctx.owner_lock);
-  if (m_image_ctx.exclusive_lock != nullptr &&
-      m_image_ctx.exclusive_lock->accept_requests()) {
-    ::encode(ResponseMessage(-EOPNOTSUPP), ack_ctx->out);
+  if (m_image_ctx.exclusive_lock != nullptr) {
+    int r;
+    if (m_image_ctx.exclusive_lock->accept_requests(&r) || r < 0) {
+      ::encode(ResponseMessage(-EOPNOTSUPP), ack_ctx->out);
+    }
   }
   return true;
 }

diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc
index 0fa190b1dcaacec14300e2635bf69c2d73122c49..28afa4dda0f992cf207f2fe25d34eebb6c0258bb 100644 (file)
--- a/src/librbd/Operations.cc
+++ b/src/librbd/Operations.cc
@@ -176,8 +176,9 @@ struct C_InvokeAsyncRequest : public Context {
       return;
     }
 
+    int r;
     if (image_ctx.exclusive_lock->is_lock_owner() &&
-        image_ctx.exclusive_lock->accept_requests()) {
+        image_ctx.exclusive_lock->accept_requests(&r)) {
       send_local_request();
       owner_lock.put_read();
       return;
@@ -1050,7 +1051,7 @@ int Operations<I>::prepare_image_update() {
     RWLock::WLocker owner_locker(m_image_ctx.owner_lock);
     if (m_image_ctx.exclusive_lock != nullptr &&
         (!m_image_ctx.exclusive_lock->is_lock_owner() ||
-         !m_image_ctx.exclusive_lock->accept_requests())) {
+         !m_image_ctx.exclusive_lock->accept_requests(&r))) {
       m_image_ctx.exclusive_lock->try_lock(&ctx);
       trying_lock = true;
     }

diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index 13682df4182ab9a05d96130ea64d3fa32a740448..2518369d7c4273a2159f3011a8cea6540b983caa 100644 (file)
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -1689,7 +1689,7 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
     // avoid accepting new requests from peers while we manipulate
     // the image features
     if (ictx->exclusive_lock != nullptr) {
-      ictx->exclusive_lock->block_requests();
+      ictx->exclusive_lock->block_requests(0);
     }
     BOOST_SCOPE_EXIT_ALL( (ictx) ) {
       if (ictx->exclusive_lock != nullptr) {

diff --git a/src/test/librbd/test_mock_ExclusiveLock.cc b/src/test/librbd/test_mock_ExclusiveLock.cc
index 775767598c706376526515c42d116f1ec31ff0f3..e508c8d202710fd7b0ea6d5925b865aab0e82caf 100644 (file)
--- a/src/test/librbd/test_mock_ExclusiveLock.cc
+++ b/src/test/librbd/test_mock_ExclusiveLock.cc
@@ -589,5 +589,43 @@ TEST_F(TestMockExclusiveLock, ConcurrentRequests) {
   ASSERT_EQ(0, when_shut_down(mock_image_ctx, exclusive_lock));
 }
 
+TEST_F(TestMockExclusiveLock, BlockRequests) {
+  REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK);
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockExclusiveLockImageCtx mock_image_ctx(*ictx);
+  MockExclusiveLock exclusive_lock(mock_image_ctx);
+
+  expect_op_work_queue(mock_image_ctx);
+
+  InSequence seq;
+  expect_block_writes(mock_image_ctx);
+  ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock));
+
+  MockAcquireRequest try_lock_acquire;
+  expect_acquire_lock(mock_image_ctx, try_lock_acquire, 0);
+  ASSERT_EQ(0, when_try_lock(mock_image_ctx, exclusive_lock));
+  ASSERT_TRUE(is_lock_owner(mock_image_ctx, exclusive_lock));
+
+  int ret_val;
+  ASSERT_TRUE(exclusive_lock.accept_requests(&ret_val));
+  ASSERT_EQ(0, ret_val);
+
+  exclusive_lock.block_requests(-EROFS);
+  ASSERT_FALSE(exclusive_lock.accept_requests(&ret_val));
+  ASSERT_EQ(-EROFS, ret_val);
+
+  exclusive_lock.unblock_requests();
+  ASSERT_TRUE(exclusive_lock.accept_requests(&ret_val));
+  ASSERT_EQ(0, ret_val);
+
+  MockReleaseRequest shutdown_release;
+  expect_release_lock(mock_image_ctx, shutdown_release, 0, true);
+  ASSERT_EQ(0, when_shut_down(mock_image_ctx, exclusive_lock));
+  ASSERT_FALSE(is_lock_owner(mock_image_ctx, exclusive_lock));
+}
+
 } // namespace librbd