librbd: fix stuck with disable request

Problem: ------- Trying to disable any feature on an rbd image mapped with nbd leads to stuck in rbd-nbd. The rbd-nbd registers a watcher callback to detect image resize in NBDWatchCtx::handle_notify(). The handle_notify calls image info method, which calls refresh_if_required and it got stuck there. It is getting stuck in ImageState::refresh_if_required() because DisableFeaturesRequest issues update notifications while still holding onto the exclusive lock with everything that has to do with it blocked. Solution: -------- Change DisableFeaturesRequest to call ImageCtx::notify_update() after unblocking the exclusive lock. Fixes: https://tracker.ceph.com/issues/58740 Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>
ceph · Apr 27, 2023 · 889d124 · 889d124
1 parent 3bc2177
commit 889d124
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 7 deletions.
diff --git a/src/librbd/operation/DisableFeaturesRequest.cc b/src/librbd/operation/DisableFeaturesRequest.cc
@@ -586,6 +586,22 @@ void DisableFeaturesRequest<I>::send_notify_update() {
   CephContext *cct = image_ctx.cct;
   ldout(cct, 20) << this << " " << __func__ << dendl;
 
+  {
+    std::unique_lock locker{image_ctx.owner_lock};
+    if (image_ctx.exclusive_lock != nullptr && m_requests_blocked) {
+      image_ctx.exclusive_lock->unblock_requests();
+      m_requests_blocked = false;
+    }
+    if (m_writes_blocked) {
+      image_ctx.io_image_dispatcher->unblock_writes();
+      m_writes_blocked = false;
+    }
+  }
+  if (!m_prepare_lock_completed) {
+    image_ctx.state->handle_prepare_lock_complete();
+    m_prepare_lock_completed = true;
+  }
+
   Context *ctx = create_context_callback<
     DisableFeaturesRequest<I>,
     &DisableFeaturesRequest<I>::handle_notify_update>(this);
@@ -640,11 +656,17 @@ Context *DisableFeaturesRequest<I>::handle_finish(int r) {
     std::unique_lock locker{image_ctx.owner_lock};
     if (image_ctx.exclusive_lock != nullptr && m_requests_blocked) {
       image_ctx.exclusive_lock->unblock_requests();
+      m_requests_blocked = false;
     }
-
-    image_ctx.io_image_dispatcher->unblock_writes();
+    if (m_writes_blocked) {
+      image_ctx.io_image_dispatcher->unblock_writes();
+      m_writes_blocked = false;
+    }
+  }
+  if (!m_prepare_lock_completed) {
+    image_ctx.state->handle_prepare_lock_complete();
+    m_prepare_lock_completed = true;
   }
-  image_ctx.state->handle_prepare_lock_complete();
 
   return this->create_context_finisher(r);
 }

diff --git a/src/librbd/operation/DisableFeaturesRequest.h b/src/librbd/operation/DisableFeaturesRequest.h
@@ -88,13 +88,14 @@ class DisableFeaturesRequest : public Request<ImageCtxT> {
    *    v
    * STATE_UPDATE_FLAGS
    *    |
+   *    | (unblock exclusive lock)
    *    v
    * STATE_NOTIFY_UPDATE
    *    |
    *    v
    * STATE_REALEASE_EXCLUSIVE_LOCK (skip if not
    *    |                           required)
-   *    | (unblock writes)
+   *    | (unblock exclusive lock, skip if not required)
    *    v
    * <finish>
    *
@@ -109,6 +110,7 @@ class DisableFeaturesRequest : public Request<ImageCtxT> {
   bool m_writes_blocked = false;
   bool m_image_lock_acquired = false;
   bool m_requests_blocked = false;
+  bool m_prepare_lock_completed = false;
 
   uint64_t m_new_features = 0;
   uint64_t m_disable_flags = 0;

diff --git a/src/librbd/operation/EnableFeaturesRequest.cc b/src/librbd/operation/EnableFeaturesRequest.cc
@@ -451,6 +451,22 @@ void EnableFeaturesRequest<I>::send_notify_update() {
   CephContext *cct = image_ctx.cct;
   ldout(cct, 20) << this << " " << __func__ << dendl;
 
+  {
+    std::unique_lock locker{image_ctx.owner_lock};
+    if (image_ctx.exclusive_lock != nullptr && m_requests_blocked) {
+      image_ctx.exclusive_lock->unblock_requests();
+      m_requests_blocked = false;
+    }
+    if (m_writes_blocked) {
+      image_ctx.io_image_dispatcher->unblock_writes();
+      m_writes_blocked = false;
+    }
+  }
+  if (!m_prepare_lock_completed) {
+    image_ctx.state->handle_prepare_lock_complete();
+    m_prepare_lock_completed = true;
+  }
+
   Context *ctx = create_context_callback<
     EnableFeaturesRequest<I>,
     &EnableFeaturesRequest<I>::handle_notify_update>(this);
@@ -475,15 +491,19 @@ Context *EnableFeaturesRequest<I>::handle_finish(int r) {
 
   {
     std::unique_lock locker{image_ctx.owner_lock};
-
     if (image_ctx.exclusive_lock != nullptr && m_requests_blocked) {
       image_ctx.exclusive_lock->unblock_requests();
+      m_requests_blocked = false;
     }
     if (m_writes_blocked) {
       image_ctx.io_image_dispatcher->unblock_writes();
+      m_writes_blocked = false;
     }
   }
-  image_ctx.state->handle_prepare_lock_complete();
+  if (!m_prepare_lock_completed) {
+    image_ctx.state->handle_prepare_lock_complete();
+    m_prepare_lock_completed = true;
+  }
 
   return this->create_context_finisher(r);
 }

diff --git a/src/librbd/operation/EnableFeaturesRequest.h b/src/librbd/operation/EnableFeaturesRequest.h
@@ -72,10 +72,11 @@ class EnableFeaturesRequest : public Request<ImageCtxT> {
    *    v
    * STATE_ENABLE_MIRROR_IMAGE
    *    |
+   *    | (unblock exclusive lock)
    *    V
    * STATE_NOTIFY_UPDATE
    *    |
-   *    | (unblock writes)
+   *    | (unblock exclusive lock, skip if not required)
    *    v
    * <finish>
    * @endverbatim
@@ -87,6 +88,7 @@ class EnableFeaturesRequest : public Request<ImageCtxT> {
   bool m_enable_mirroring = false;
   bool m_requests_blocked = false;
   bool m_writes_blocked = false;
+  bool m_prepare_lock_completed = false;
 
   uint64_t m_new_features = 0;
   uint64_t m_enable_flags = 0;