From 13adcac42967e58a7663b42b2c9d217f7c36c88c Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 18 May 2017 16:54:06 -0400 Subject: [PATCH] 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 --- src/librbd/image/CloseRequest.cc | 73 +++++++++++++++++++++----------- src/librbd/image/CloseRequest.h | 25 ++++++----- 2 files changed, 63 insertions(+), 35 deletions(-) diff --git a/src/librbd/image/CloseRequest.cc b/src/librbd/image/CloseRequest.cc index 900164fccab..ee1519bf7d5 100644 --- a/src/librbd/image/CloseRequest.cc +++ b/src/librbd/image/CloseRequest.cc @@ -31,56 +31,50 @@ CloseRequest::CloseRequest(I *image_ctx, Context *on_finish) template void CloseRequest::send() { - send_shut_down_update_watchers(); + send_block_image_watcher(); } template -void CloseRequest::send_shut_down_update_watchers() { +void CloseRequest::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, &CloseRequest::handle_shut_down_update_watchers>(this))); + // prevent incoming requests from our peers + m_image_ctx->image_watcher->block_notifies(create_context_callback< + CloseRequest, &CloseRequest::handle_block_image_watcher>(this)); } template -void CloseRequest::handle_shut_down_update_watchers(int r) { +void CloseRequest::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 -void CloseRequest::send_unregister_image_watcher() { - if (m_image_ctx->image_watcher == nullptr) { - send_shut_down_io_queue(); - return; - } - +void CloseRequest::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, &CloseRequest::handle_unregister_image_watcher>(this)); + m_image_ctx->state->shut_down_update_watchers(create_async_context_callback( + *m_image_ctx, create_context_callback< + CloseRequest, &CloseRequest::handle_shut_down_update_watchers>(this))); } template -void CloseRequest::handle_unregister_image_watcher(int r) { +void CloseRequest::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::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 @@ -178,6 +173,34 @@ void CloseRequest::handle_flush(int r) { if (r < 0) { lderr(cct) << "failed to flush IO: " << cpp_strerror(r) << dendl; } + send_unregister_image_watcher(); +} + +template +void CloseRequest::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, &CloseRequest::handle_unregister_image_watcher>(this)); +} + +template +void CloseRequest::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 c5cf2e1735e..812e91d723e 100644 --- a/src/librbd/image/CloseRequest.h +++ b/src/librbd/image/CloseRequest.h @@ -30,19 +30,21 @@ private: * * | * 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); -- 2.39.5