From: Jason Dillaman <dillaman@redhat.com>
Date: Thu, 6 Sep 2018 13:44:59 +0000 (-0400)
Subject: librbd: watcher should internally track blacklisted state
X-Git-Tag: v14.0.1~228^2~5
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9ea94f284061849e452dd61c8f89ecca18642b0d;p=ceph-ci.git

librbd: watcher should internally track blacklisted state

Since it will periodically attempt to re-acquire the watch,
it will know when the RADOS client has been blacklisted and
when the blacklist has been removed.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---

diff --git a/src/librbd/Watcher.cc b/src/librbd/Watcher.cc
index 8e18460757b..c02598983e7 100644
--- a/src/librbd/Watcher.cc
+++ b/src/librbd/Watcher.cc
@@ -104,9 +104,10 @@ Watcher::~Watcher() {
 void Watcher::register_watch(Context *on_finish) {
   ldout(m_cct, 10) << dendl;
 
-  RWLock::RLocker watch_locker(m_watch_lock);
+  RWLock::WLocker watch_locker(m_watch_lock);
   ceph_assert(is_unregistered(m_watch_lock));
   m_watch_state = WATCH_STATE_REGISTERING;
+  m_watch_blacklisted = false;
 
   librados::AioCompletion *aio_comp = create_rados_callback(
     new C_RegisterWatch(this, on_finish));
@@ -137,6 +138,8 @@ void Watcher::handle_register_watch(int r, Context *on_finish) {
       lderr(m_cct) << "re-registering watch after error" << dendl;
       m_watch_state = WATCH_STATE_REWATCHING;
       watch_error = true;
+    } else {
+      m_watch_blacklisted = (r == -EBLACKLISTED);
     }
   }
 
@@ -165,11 +168,13 @@ void Watcher::unregister_watch(Context *on_finish) {
       return;
     } else if (is_registered(m_watch_lock)) {
       librados::AioCompletion *aio_comp = create_rados_callback(
-                        new C_UnwatchAndFlush(m_ioctx, on_finish));
+        new C_UnwatchAndFlush(m_ioctx, on_finish));
       int r = m_ioctx.aio_unwatch(m_watch_handle, aio_comp);
       ceph_assert(r == 0);
       aio_comp->release();
+
       m_watch_handle = 0;
+      m_watch_blacklisted = false;
       return;
     }
   }
@@ -225,6 +230,9 @@ void Watcher::handle_error(uint64_t handle, int err) {
 
   if (is_registered(m_watch_lock)) {
     m_watch_state = WATCH_STATE_REWATCHING;
+    if (err == -EBLACKLISTED) {
+      m_watch_blacklisted = true;
+    }
 
     FunctionContext *ctx = new FunctionContext(
         boost::bind(&Watcher::rewatch, this));
@@ -271,12 +279,14 @@ void Watcher::handle_rewatch(int r) {
     RWLock::WLocker watch_locker(m_watch_lock);
     ceph_assert(m_watch_state == WATCH_STATE_REWATCHING);
 
+    m_watch_blacklisted = false;
     if (m_unregister_watch_ctx != nullptr) {
       ldout(m_cct, 10) << "image is closing, skip rewatch" << dendl;
       m_watch_state = WATCH_STATE_IDLE;
       std::swap(unregister_watch_ctx, m_unregister_watch_ctx);
     } else if (r  == -EBLACKLISTED) {
       lderr(m_cct) << "client blacklisted" << dendl;
+      m_watch_blacklisted = true;
     } else if (r == -ENOENT) {
       ldout(m_cct, 5) << "object does not exist" << dendl;
     } else if (r < 0) {
diff --git a/src/librbd/Watcher.h b/src/librbd/Watcher.h
index 518fbdd015e..69e87ad30ce 100644
--- a/src/librbd/Watcher.h
+++ b/src/librbd/Watcher.h
@@ -60,6 +60,10 @@ public:
     RWLock::RLocker locker(m_watch_lock);
     return is_unregistered(m_watch_lock);
   }
+  bool is_blacklisted() const {
+    RWLock::RLocker locker(m_watch_lock);
+    return m_watch_blacklisted;
+  }
 
 protected:
   enum WatchState {
@@ -75,7 +79,10 @@ protected:
   mutable RWLock m_watch_lock;
   uint64_t m_watch_handle;
   watcher::Notifier m_notifier;
+
   WatchState m_watch_state;
+  bool m_watch_blacklisted = false;
+
   AsyncOpTracker m_async_op_tracker;
 
   bool is_registered(const RWLock&) const {
diff --git a/src/test/librbd/mock/MockImageWatcher.h b/src/test/librbd/mock/MockImageWatcher.h
index db3fafe3558..c7e94279237 100644
--- a/src/test/librbd/mock/MockImageWatcher.h
+++ b/src/test/librbd/mock/MockImageWatcher.h
@@ -13,6 +13,7 @@ namespace librbd {
 struct MockImageWatcher {
   MOCK_METHOD0(is_registered, bool());
   MOCK_METHOD0(is_unregistered, bool());
+  MOCK_METHOD0(is_blacklisted, bool());
   MOCK_METHOD0(unregister_watch, void());
   MOCK_METHOD1(flush, void(Context *));
 
diff --git a/src/test/librbd/test_mock_Watcher.cc b/src/test/librbd/test_mock_Watcher.cc
index 14171856c5f..907f671e877 100644
--- a/src/test/librbd/test_mock_Watcher.cc
+++ b/src/test/librbd/test_mock_Watcher.cc
@@ -124,6 +124,7 @@ public:
         return false;
       }
     }
+    m_watch_count -= count;
     return true;
   }
 
@@ -291,22 +292,34 @@ TEST_F(TestMockWatcher, ReregisterWatchBlacklist) {
   expect_aio_watch(mock_image_ctx, 0);
   expect_aio_unwatch(mock_image_ctx, 0);
   expect_aio_watch(mock_image_ctx, -EBLACKLISTED);
-  expect_aio_watch(mock_image_ctx, 0);
+
+  C_SaferCond blacklist_ctx;
+  expect_aio_watch(mock_image_ctx, 0, [&blacklist_ctx]() {
+      blacklist_ctx.wait();
+    });
   expect_aio_unwatch(mock_image_ctx, 0);
 
   C_SaferCond register_ctx;
   mock_image_watcher.register_watch(&register_ctx);
+  ASSERT_TRUE(wait_for_watch(mock_image_ctx, 1));
   ASSERT_EQ(0, register_ctx.wait());
 
   ceph_assert(m_watch_ctx != nullptr);
   m_watch_ctx->handle_error(0, -EBLACKLISTED);
 
   // wait for recovery unwatch/watch
-  ASSERT_TRUE(wait_for_watch(mock_image_ctx, 4));
+  ASSERT_TRUE(wait_for_watch(mock_image_ctx, 2));
+
+  ASSERT_TRUE(mock_image_watcher.is_blacklisted());
+  blacklist_ctx.complete(0);
+
+  // wait for post-blacklist recovery watch
+  ASSERT_TRUE(wait_for_watch(mock_image_ctx, 1));
 
   C_SaferCond unregister_ctx;
   mock_image_watcher.unregister_watch(&unregister_ctx);
   ASSERT_EQ(0, unregister_ctx.wait());
+  ASSERT_FALSE(mock_image_watcher.is_blacklisted());
 }
 
 TEST_F(TestMockWatcher, ReregisterUnwatchPendingUnregister) {