librbd: block RPC requests before unregistering watch during close

The watch should be released after exclusive lock is released to
avoid a potential race condition which would result in the
client being blacklisted by a peer.

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

diff --git a/src/librbd/image/CloseRequest.cc b/src/librbd/image/CloseRequest.cc
index 900164fccabd887fa35b45bb8375c2061021262e..ee1519bf7d5f1bf0fdf1953cf51cb744a812882d 100644 (file)
--- a/src/librbd/image/CloseRequest.cc
+++ b/src/librbd/image/CloseRequest.cc
@@ -31,56 +31,50 @@ CloseRequest<I>::CloseRequest(I *image_ctx, Context *on_finish)
 
 template <typename I>
 void CloseRequest<I>::send() {
-  send_shut_down_update_watchers();
+  send_block_image_watcher();
 }
 
 template <typename I>
-void CloseRequest<I>::send_shut_down_update_watchers() {
+void CloseRequest<I>::send_block_image_watcher() {
+  if (m_image_ctx->image_watcher == nullptr) {
+    send_shut_down_update_watchers();
+    return;
+  }
+
   CephContext *cct = m_image_ctx->cct;
   ldout(cct, 10) << this << " " << __func__ << dendl;
 
-  m_image_ctx->state->shut_down_update_watchers(create_async_context_callback(
-    *m_image_ctx, create_context_callback<
-      CloseRequest<I>, &CloseRequest<I>::handle_shut_down_update_watchers>(this)));
+  // prevent incoming requests from our peers
+  m_image_ctx->image_watcher->block_notifies(create_context_callback<
+    CloseRequest<I>, &CloseRequest<I>::handle_block_image_watcher>(this));
 }
 
 template <typename I>
-void CloseRequest<I>::handle_shut_down_update_watchers(int r) {
+void CloseRequest<I>::handle_block_image_watcher(int r) {
   CephContext *cct = m_image_ctx->cct;
   ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
 
-  save_result(r);
-  if (r < 0) {
-    lderr(cct) << "failed to shut down update watchers: " << cpp_strerror(r)
-               << dendl;
-  }
-
-  send_unregister_image_watcher();
+  send_shut_down_update_watchers();
 }
 
 template <typename I>
-void CloseRequest<I>::send_unregister_image_watcher() {
-  if (m_image_ctx->image_watcher == nullptr) {
-    send_shut_down_io_queue();
-    return;
-  }
-
+void CloseRequest<I>::send_shut_down_update_watchers() {
   CephContext *cct = m_image_ctx->cct;
   ldout(cct, 10) << this << " " << __func__ << dendl;
 
-  // prevent incoming requests from our peers
-  m_image_ctx->image_watcher->unregister_watch(create_context_callback<
-    CloseRequest<I>, &CloseRequest<I>::handle_unregister_image_watcher>(this));
+  m_image_ctx->state->shut_down_update_watchers(create_async_context_callback(
+    *m_image_ctx, create_context_callback<
+      CloseRequest<I>, &CloseRequest<I>::handle_shut_down_update_watchers>(this)));
 }
 
 template <typename I>
-void CloseRequest<I>::handle_unregister_image_watcher(int r) {
+void CloseRequest<I>::handle_shut_down_update_watchers(int r) {
   CephContext *cct = m_image_ctx->cct;
   ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
 
   save_result(r);
   if (r < 0) {
-    lderr(cct) << "failed to unregister image watcher: " << cpp_strerror(r)
+    lderr(cct) << "failed to shut down update watchers: " << cpp_strerror(r)
                << dendl;
   }
 
@@ -156,7 +150,8 @@ void CloseRequest<I>::handle_shut_down_exclusive_lock(int r) {
     lderr(cct) << "failed to shut down exclusive lock: " << cpp_strerror(r)
                << dendl;
   }
-  send_flush_readahead();
+
+  send_unregister_image_watcher();
 }
 
 template <typename I>
@@ -178,6 +173,34 @@ void CloseRequest<I>::handle_flush(int r) {
   if (r < 0) {
     lderr(cct) << "failed to flush IO: " << cpp_strerror(r) << dendl;
   }
+  send_unregister_image_watcher();
+}
+
+template <typename I>
+void CloseRequest<I>::send_unregister_image_watcher() {
+  if (m_image_ctx->image_watcher == nullptr) {
+    send_flush_readahead();
+    return;
+  }
+
+  CephContext *cct = m_image_ctx->cct;
+  ldout(cct, 10) << this << " " << __func__ << dendl;
+
+  m_image_ctx->image_watcher->unregister_watch(create_context_callback<
+    CloseRequest<I>, &CloseRequest<I>::handle_unregister_image_watcher>(this));
+}
+
+template <typename I>
+void CloseRequest<I>::handle_unregister_image_watcher(int r) {
+  CephContext *cct = m_image_ctx->cct;
+  ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
+
+  save_result(r);
+  if (r < 0) {
+    lderr(cct) << "failed to unregister image watcher: " << cpp_strerror(r)
+               << dendl;
+  }
+
   send_flush_readahead();
 }
 

diff --git a/src/librbd/image/CloseRequest.h b/src/librbd/image/CloseRequest.h
index c5cf2e1735e34c2dd498983dfd01aa47b2f5a7d6..812e91d723e18597df72a90ad285e7193489046f 100644 (file)
--- a/src/librbd/image/CloseRequest.h
+++ b/src/librbd/image/CloseRequest.h
@@ -30,19 +30,21 @@ private:
    * <start>
    *    |
    *    v
-   * SHUT_DOWN_UPDATE_WATCHERS
+   * BLOCK_IMAGE_WATCHER (skip if R/O)
    *    |
    *    v
-   * UNREGISTER_IMAGE_WATCHER
+   * SHUT_DOWN_UPDATE_WATCHERS
    *    |
    *    v
    * SHUT_DOWN_AIO_WORK_QUEUE . . .
-   *    |                         .
-   *    v                         .
-   * SHUT_DOWN_EXCLUSIVE_LOCK     . (exclusive lock
-   *    |                         .  disabled)
+   *    |                         . (exclusive lock disabled)
    *    v                         v
-   * FLUSH  < . . . . . . . . . . .
+   * SHUT_DOWN_EXCLUSIVE_LOCK   FLUSH
+   *    |                         .
+   *    |     . . . . . . . . . . .
+   *    |     .
+   *    v     v
+   * UNREGISTER_IMAGE_WATCHER (skip if R/O)
    *    |
    *    v
    * FLUSH_READAHEAD
@@ -74,12 +76,12 @@ private:
 
   decltype(m_image_ctx->exclusive_lock) m_exclusive_lock;
 
+  void send_block_image_watcher();
+  void handle_block_image_watcher(int r);
+
   void send_shut_down_update_watchers();
   void handle_shut_down_update_watchers(int r);
 
-  void send_unregister_image_watcher();
-  void handle_unregister_image_watcher(int r);
-
   void send_shut_down_io_queue();
   void handle_shut_down_io_queue(int r);
 
@@ -89,6 +91,9 @@ private:
   void send_flush();
   void handle_flush(int r);
 
+  void send_unregister_image_watcher();
+  void handle_unregister_image_watcher(int r);
+
   void send_flush_readahead();
   void handle_flush_readahead(int r);