From 547e867628975c7144590e9332aa62b0ef82a433 Mon Sep 17 00:00:00 2001
From: Jason Dillaman <dillaman@redhat.com>
Date: Thu, 5 Jan 2017 12:12:57 -0500
Subject: [PATCH] librbd: possible deadlock with flush if refresh in-progress

Fixes: http://tracker.ceph.com/issues/18419
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit b95f92a5572d3035c20eba07e76d2c825a9853f7)

Conflicts:
	src/librbd/ImageState.h (master commit just adds a function
                      declaration, so just add it to jewel as well)
---
 src/librbd/ImageState.cc | 29 +++++++++++++++++++++++++----
 src/librbd/ImageState.h  |  2 ++
 src/librbd/internal.cc   |  5 ++++-
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/src/librbd/ImageState.cc b/src/librbd/ImageState.cc
index 28cf4274ef9ec..e1ba4a312dc63 100644
--- a/src/librbd/ImageState.cc
+++ b/src/librbd/ImageState.cc
@@ -306,7 +306,7 @@ void ImageState<I>::handle_update_notification() {
 template <typename I>
 bool ImageState<I>::is_refresh_required() const {
   Mutex::Locker locker(m_lock);
-  return (m_last_refresh != m_refresh_seq);
+  return (m_last_refresh != m_refresh_seq || find_pending_refresh() != nullptr);
 }
 
 template <typename I>
@@ -338,7 +338,14 @@ int ImageState<I>::refresh_if_required() {
   C_SaferCond ctx;
   {
     m_lock.Lock();
-    if (m_last_refresh == m_refresh_seq) {
+    Action action(ACTION_TYPE_REFRESH);
+    action.refresh_seq = m_refresh_seq;
+
+    auto refresh_action = find_pending_refresh();
+    if (refresh_action != nullptr) {
+      // if a refresh is in-flight, delay until it is finished
+      action = *refresh_action;
+    } else if (m_last_refresh == m_refresh_seq) {
       m_lock.Unlock();
       return 0;
     } else if (is_closed()) {
@@ -346,14 +353,28 @@ int ImageState<I>::refresh_if_required() {
       return -ESHUTDOWN;
     }
 
-    Action action(ACTION_TYPE_REFRESH);
-    action.refresh_seq = m_refresh_seq;
     execute_action_unlock(action, &ctx);
   }
 
   return ctx.wait();
 }
 
+template <typename I>
+const typename ImageState<I>::Action *
+ImageState<I>::find_pending_refresh() const {
+  assert(m_lock.is_locked());
+
+  auto it = std::find_if(m_actions_contexts.rbegin(),
+                         m_actions_contexts.rend(),
+                         [](const ActionContexts& action_contexts) {
+      return (action_contexts.first == ACTION_TYPE_REFRESH);
+    });
+  if (it != m_actions_contexts.rend()) {
+    return &it->first;
+  }
+  return nullptr;
+}
+
 template <typename I>
 void ImageState<I>::snap_set(const std::string &snap_name, Context *on_finish) {
   CephContext *cct = m_image_ctx->cct;
diff --git a/src/librbd/ImageState.h b/src/librbd/ImageState.h
index b6e7ce63b4426..73433ba8fc8d2 100644
--- a/src/librbd/ImageState.h
+++ b/src/librbd/ImageState.h
@@ -114,6 +114,8 @@ private:
   bool is_transition_state() const;
   bool is_closed() const;
 
+  const Action *find_pending_refresh() const;
+
   void append_context(const Action &action, Context *context);
   void execute_next_action_unlock();
   void execute_action_unlock(const Action &action, Context *context);
diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc
index d5875d8dde21f..70734789f634b 100644
--- a/src/librbd/internal.cc
+++ b/src/librbd/internal.cc
@@ -2967,10 +2967,13 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force,
     }
 
     ictx->user_flushed();
+    C_SaferCond ctx;
     {
       RWLock::RLocker owner_locker(ictx->owner_lock);
-      r = ictx->flush();
+      ictx->flush(&ctx);
     }
+    r = ctx.wait();
+
     ictx->perfcounter->inc(l_librbd_flush);
     return r;
   }
-- 
2.39.5