librbd: fix handling of proxied maintenance operations during shut down

src/librbd/ExclusiveLock.cc

@@ -61,6 +61,7 @@ bool ExclusiveLock<I>::is_lock_owner() const {
   case STATE_LOCKED:
   case STATE_POST_ACQUIRING:
   case STATE_PRE_RELEASING:
+  case STATE_PRE_SHUTTING_DOWN:
     lock_owner = true;
     break;
   default:
@@ -111,54 +112,58 @@ void ExclusiveLock<I>::shut_down(Context *on_shut_down) {
 
 template <typename I>
 void ExclusiveLock<I>::try_lock(Context *on_tried_lock) {
+  int r = 0;
   {
     Mutex::Locker locker(m_lock);
     assert(m_image_ctx.owner_lock.is_locked());
-    assert(!is_shutdown());
-
-    if (m_state != STATE_LOCKED || !m_actions_contexts.empty()) {
+    if (is_shutdown()) {
+      r = -ESHUTDOWN;
+    } else if (m_state != STATE_LOCKED || !m_actions_contexts.empty()) {
       ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl;
       execute_action(ACTION_TRY_LOCK, on_tried_lock);
       return;
     }
   }
 
-  on_tried_lock->complete(0);
+  on_tried_lock->complete(r);
 }
 
 template <typename I>
 void ExclusiveLock<I>::request_lock(Context *on_locked) {
+  int r = 0;
   {
     Mutex::Locker locker(m_lock);
     assert(m_image_ctx.owner_lock.is_locked());
-    assert(!is_shutdown());
-    if (m_state != STATE_LOCKED || !m_actions_contexts.empty()) {
+    if (is_shutdown()) {
+      r = -ESHUTDOWN;
+    } else if (m_state != STATE_LOCKED || !m_actions_contexts.empty()) {
       ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl;
       execute_action(ACTION_REQUEST_LOCK, on_locked);
       return;
     }
   }
 
   if (on_locked != nullptr) {
-    on_locked->complete(0);
+    on_locked->complete(r);
   }
 }
 
 template <typename I>
 void ExclusiveLock<I>::release_lock(Context *on_released) {
+  int r = 0;
   {
     Mutex::Locker locker(m_lock);
     assert(m_image_ctx.owner_lock.is_locked());
-    assert(!is_shutdown());
-
-    if (m_state != STATE_UNLOCKED || !m_actions_contexts.empty()) {
+    if (is_shutdown()) {
+      r = -ESHUTDOWN;
+    } else if (m_state != STATE_UNLOCKED || !m_actions_contexts.empty()) {
       ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl;
       execute_action(ACTION_RELEASE_LOCK, on_released);
       return;
     }
   }
 
-  on_released->complete(0);
+  on_released->complete(r);
 }
 
 template <typename I>
@@ -210,6 +215,7 @@ bool ExclusiveLock<I>::is_transition_state() const {
   case STATE_POST_ACQUIRING:
   case STATE_PRE_RELEASING:
   case STATE_RELEASING:
+  case STATE_PRE_SHUTTING_DOWN:
   case STATE_SHUTTING_DOWN:
     return true;
   case STATE_UNINITIALIZED:
@@ -466,16 +472,14 @@ void ExclusiveLock<I>::send_shutdown() {
   assert(m_lock.is_locked());
   if (m_state == STATE_UNLOCKED) {
     m_state = STATE_SHUTTING_DOWN;
-    m_image_ctx.aio_work_queue->clear_require_lock_on_read();
-    m_image_ctx.aio_work_queue->unblock_writes();
-    m_image_ctx.image_watcher->flush(util::create_context_callback<
-      ExclusiveLock<I>, &ExclusiveLock<I>::complete_shutdown>(this));
+    m_image_ctx.op_work_queue->queue(util::create_context_callback<
+      ExclusiveLock<I>, &ExclusiveLock<I>::handle_shutdown>(this), 0);
     return;
   }
 
   ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl;
   assert(m_state == STATE_LOCKED);
-  m_state = STATE_SHUTTING_DOWN;
+  m_state = STATE_PRE_SHUTTING_DOWN;
 
   m_lock.Unlock();
   m_image_ctx.op_work_queue->queue(new C_ShutDownRelease(this), 0);
@@ -492,16 +496,34 @@ void ExclusiveLock<I>::send_shutdown_release() {
 
   using el = ExclusiveLock<I>;
   ReleaseRequest<I>* req = ReleaseRequest<I>::create(
-    m_image_ctx, cookie, nullptr,
-    util::create_context_callback<el, &el::handle_shutdown>(this));
+    m_image_ctx, cookie,
+    util::create_context_callback<el, &el::handle_shutdown_releasing>(this),
+    util::create_context_callback<el, &el::handle_shutdown_released>(this));
   req->send();
 }
 
 template <typename I>
-void ExclusiveLock<I>::handle_shutdown(int r) {
+void ExclusiveLock<I>::handle_shutdown_releasing(int r) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
 
+  assert(r == 0);
+  assert(m_state == STATE_PRE_SHUTTING_DOWN);
+
+  // all IO and ops should be blocked/canceled by this point
+  m_state = STATE_SHUTTING_DOWN;
+}
+
+template <typename I>
+void ExclusiveLock<I>::handle_shutdown_released(int r) {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
+
+  {
+    RWLock::WLocker owner_locker(m_image_ctx.owner_lock);
+    m_image_ctx.exclusive_lock = nullptr;
+  }
+
   if (r < 0) {
     lderr(cct) << "failed to shut down exclusive lock: " << cpp_strerror(r)
                << dendl;
@@ -514,6 +536,22 @@ void ExclusiveLock<I>::handle_shutdown(int r) {
   complete_shutdown(r);
 }
 
+template <typename I>
+void ExclusiveLock<I>::handle_shutdown(int r) {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
+
+  {
+    RWLock::WLocker owner_locker(m_image_ctx.owner_lock);
+    m_image_ctx.exclusive_lock = nullptr;
+  }
+
+  m_image_ctx.aio_work_queue->clear_require_lock_on_read();
+  m_image_ctx.aio_work_queue->unblock_writes();
+  m_image_ctx.image_watcher->flush(util::create_context_callback<
+    ExclusiveLock<I>, &ExclusiveLock<I>::complete_shutdown>(this));
+}
+
 template <typename I>
 void ExclusiveLock<I>::complete_shutdown(int r) {
   ActionContexts action_contexts;

src/librbd/ExclusiveLock.h

@@ -67,7 +67,7 @@ class ExclusiveLock {
    *    |
    *    |
    *    v
-   * SHUTTING_DOWN ---> SHUTDOWN ---> <finish>
+   * PRE_SHUTTING_DOWN ---> SHUTTING_DOWN ---> SHUTDOWN ---> <finish>
    */
   enum State {
     STATE_UNINITIALIZED,
@@ -79,6 +79,7 @@ class ExclusiveLock {
     STATE_WAITING_FOR_PEER,
     STATE_PRE_RELEASING,
     STATE_RELEASING,
+    STATE_PRE_SHUTTING_DOWN,
     STATE_SHUTTING_DOWN,
     STATE_SHUTDOWN,
   };
@@ -151,6 +152,8 @@ class ExclusiveLock {
 
   void send_shutdown();
   void send_shutdown_release();
+  void handle_shutdown_releasing(int r);
+  void handle_shutdown_released(int r);
   void handle_shutdown(int r);
   void complete_shutdown(int r);
 };

src/librbd/ImageWatcher.cc

@@ -459,6 +459,9 @@ Context *ImageWatcher::remove_async_request(const AsyncRequestId &id) {
 }
 
 void ImageWatcher::schedule_async_request_timed_out(const AsyncRequestId &id) {
+  ldout(m_image_ctx.cct, 20) << "scheduling async request time out: " << id
+                             << dendl;
+
   Context *ctx = new FunctionContext(boost::bind(
     &ImageWatcher::async_request_timed_out, this, id));
 
@@ -896,6 +899,11 @@ void ImageWatcher::reregister_watch() {
   int r;
   if (releasing_lock) {
     r = release_lock_ctx.wait();
+    if (r == -EBLACKLISTED) {
+      lderr(m_image_ctx.cct) << this << " client blacklisted" << dendl;
+      return;
+    }
+
     assert(r == 0);
   }
 

src/librbd/exclusive_lock/ReleaseRequest.cc

@@ -50,58 +50,58 @@ ReleaseRequest<I>::~ReleaseRequest() {
 
 template <typename I>
 void ReleaseRequest<I>::send() {
-  send_block_writes();
+  send_cancel_op_requests();
 }
 
 template <typename I>
-void ReleaseRequest<I>::send_block_writes() {
+void ReleaseRequest<I>::send_cancel_op_requests() {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << __func__ << dendl;
 
   using klass = ReleaseRequest<I>;
   Context *ctx = create_context_callback<
-    klass, &klass::handle_block_writes>(this);
-
-  {
-    RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
-    if (m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) {
-      m_image_ctx.aio_work_queue->set_require_lock_on_read();
-    }
-    m_image_ctx.aio_work_queue->block_writes(ctx);
-  }
+    klass, &klass::handle_cancel_op_requests>(this);
+  m_image_ctx.cancel_async_requests(ctx);
 }
 
 template <typename I>
-Context *ReleaseRequest<I>::handle_block_writes(int *ret_val) {
+Context *ReleaseRequest<I>::handle_cancel_op_requests(int *ret_val) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;
 
-  if (*ret_val < 0) {
-    m_image_ctx.aio_work_queue->unblock_writes();
-    return m_on_finish;
-  }
+  assert(*ret_val == 0);
 
-  send_cancel_op_requests();
+  send_block_writes();
   return nullptr;
 }
 
 template <typename I>
-void ReleaseRequest<I>::send_cancel_op_requests() {
+void ReleaseRequest<I>::send_block_writes() {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << __func__ << dendl;
 
   using klass = ReleaseRequest<I>;
   Context *ctx = create_context_callback<
-    klass, &klass::handle_cancel_op_requests>(this);
-  m_image_ctx.cancel_async_requests(ctx);
+    klass, &klass::handle_block_writes>(this);
+
+  {
+    RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
+    if (m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) {
+      m_image_ctx.aio_work_queue->set_require_lock_on_read();
+    }
+    m_image_ctx.aio_work_queue->block_writes(ctx);
+  }
 }
 
 template <typename I>
-Context *ReleaseRequest<I>::handle_cancel_op_requests(int *ret_val) {
+Context *ReleaseRequest<I>::handle_block_writes(int *ret_val) {
   CephContext *cct = m_image_ctx.cct;
   ldout(cct, 10) << __func__ << ": r=" << *ret_val << dendl;
 
-  assert(*ret_val == 0);
+  if (*ret_val < 0) {
+    m_image_ctx.aio_work_queue->unblock_writes();
+    return m_on_finish;
+  }
 
   if (m_on_releasing != nullptr) {
     // alert caller that we no longer own the exclusive lock

src/librbd/exclusive_lock/ReleaseRequest.h

@@ -33,10 +33,10 @@ class ReleaseRequest {
    * <start>
    *    |
    *    v
-   * BLOCK_WRITES
+   * CANCEL_OP_REQUESTS
    *    |
    *    v
-   * CANCEL_OP_REQUESTS
+   * BLOCK_WRITES
    *    |
    *    v
    * FLUSH_NOTIFIES . . . . . . . . . . . . . .
@@ -67,12 +67,12 @@ class ReleaseRequest {
   decltype(m_image_ctx.object_map) m_object_map;
   decltype(m_image_ctx.journal) m_journal;
 
-  void send_block_writes();
-  Context *handle_block_writes(int *ret_val);
-
   void send_cancel_op_requests();
   Context *handle_cancel_op_requests(int *ret_val);
 
+  void send_block_writes();
+  Context *handle_block_writes(int *ret_val);
+
   void send_flush_notifies();
   Context *handle_flush_notifies(int *ret_val);
 

src/librbd/image/CloseRequest.cc

@@ -87,9 +87,10 @@ template <typename I>
 void CloseRequest<I>::send_shut_down_exclusive_lock() {
   {
     RWLock::WLocker owner_locker(m_image_ctx->owner_lock);
-    RWLock::WLocker snap_locker(m_image_ctx->snap_lock);
-    std::swap(m_exclusive_lock, m_image_ctx->exclusive_lock);
+    m_exclusive_lock = m_image_ctx->exclusive_lock;
 
+    // if reading a snapshot -- possible object map is open
+    RWLock::WLocker snap_locker(m_image_ctx->snap_lock);
     if (m_exclusive_lock == nullptr) {
       delete m_image_ctx->object_map;
       m_image_ctx->object_map = nullptr;
@@ -104,6 +105,8 @@ void CloseRequest<I>::send_shut_down_exclusive_lock() {
   CephContext *cct = m_image_ctx->cct;
   ldout(cct, 10) << this << " " << __func__ << dendl;
 
+  // in-flight IO will be flushed and in-flight requests will be canceled
+  // before releasing lock
   m_exclusive_lock->shut_down(create_context_callback<
     CloseRequest<I>, &CloseRequest<I>::handle_shut_down_exclusive_lock>(this));
 }
@@ -113,10 +116,18 @@ void CloseRequest<I>::handle_shut_down_exclusive_lock(int r) {
   CephContext *cct = m_image_ctx->cct;
   ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
 
-  // object map and journal closed during exclusive lock shutdown
-  assert(m_image_ctx->journal == nullptr);
-  assert(m_image_ctx->object_map == nullptr);
+  {
+    RWLock::RLocker owner_locker(m_image_ctx->owner_lock);
+    assert(m_image_ctx->exclusive_lock == nullptr);
+
+    // object map and journal closed during exclusive lock shutdown
+    RWLock::RLocker snap_locker(m_image_ctx->snap_lock);
+    assert(m_image_ctx->journal == nullptr);
+    assert(m_image_ctx->object_map == nullptr);
+  }
+
   delete m_exclusive_lock;
+  m_exclusive_lock = nullptr;
 
   save_result(r);
   if (r < 0) {