librbd: integrate asynchronous image rewatch state machine

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

diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc
index b6c75ea14aab1381fa1aa7a10d4e09348de62f5f..654529026157cbe81fde69362b53560820bccabc 100644 (file)
--- a/src/librbd/ExclusiveLock.cc
+++ b/src/librbd/ExclusiveLock.cc
@@ -202,38 +202,32 @@ void ExclusiveLock<I>::reacquire_lock(Context *on_reacquired) {
     Mutex::Locker locker(m_lock);
     assert(m_image_ctx.owner_lock.is_locked());
 
-    // ignore request if shutdown or not in a locked-related state
-    if (!is_shutdown() &&
-        (m_state == STATE_LOCKED ||
-         m_state == STATE_ACQUIRING ||
-         m_state == STATE_POST_ACQUIRING ||
-         m_state == STATE_WAITING_FOR_REGISTER ||
-         m_state == STATE_WAITING_FOR_PEER)) {
+    if (m_state == STATE_WAITING_FOR_REGISTER) {
+      // restart the acquire lock process now that watch is valid
+      ldout(m_image_ctx.cct, 10) << this << " " << __func__ << ": "
+                                 << "woke up waiting acquire" << dendl;
+      Action active_action = get_active_action();
+      assert(active_action == ACTION_TRY_LOCK ||
+             active_action == ACTION_REQUEST_LOCK);
+      execute_next_action();
+    } else if (!is_shutdown() &&
+               (m_state == STATE_LOCKED ||
+                m_state == STATE_ACQUIRING ||
+                m_state == STATE_POST_ACQUIRING ||
+                m_state == STATE_WAITING_FOR_PEER)) {
+      // interlock the lock operation with other image state ops
       ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl;
       execute_action(ACTION_REACQUIRE_LOCK, on_reacquired);
       return;
     }
   }
 
+  // ignore request if shutdown or not in a locked-related state
   if (on_reacquired != nullptr) {
     on_reacquired->complete(0);
   }
 }
 
-template <typename I>
-void ExclusiveLock<I>::handle_watch_registered() {
-  Mutex::Locker locker(m_lock);
-  if (m_state != STATE_WAITING_FOR_REGISTER) {
-    return;
-  }
-
-  ldout(m_image_ctx.cct, 10) << this << " " << __func__ << dendl;
-  Action active_action = get_active_action();
-  assert(active_action == ACTION_TRY_LOCK ||
-         active_action == ACTION_REQUEST_LOCK);
-  execute_next_action();
-}
-
 template <typename I>
 void ExclusiveLock<I>::handle_peer_notification() {
   Mutex::Locker locker(m_lock);

diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h
index 5ba17aed3713a582cfae0066ea63674e3ad4efda..5543bbb01f5798811686a8871908895c18485751 100644 (file)
--- a/src/librbd/ExclusiveLock.h
+++ b/src/librbd/ExclusiveLock.h
@@ -43,7 +43,6 @@ public:
 
   void reacquire_lock(Context *on_reacquired = nullptr);
 
-  void handle_watch_registered();
   void handle_peer_notification();
 
   void assert_header_locked(librados::ObjectWriteOperation *op);

diff --git a/src/librbd/ImageWatcher.cc b/src/librbd/ImageWatcher.cc
index 55da07dbcb8d0ab02f8b03c36c4423cd203478f7..58031033aa7e3f14324e291f5e5a1d37317c551c 100644 (file)
--- a/src/librbd/ImageWatcher.cc
+++ b/src/librbd/ImageWatcher.cc
@@ -13,6 +13,7 @@
 #include "librbd/exclusive_lock/Policy.h"
 #include "librbd/image_watcher/Notifier.h"
 #include "librbd/image_watcher/NotifyLockOwner.h"
+#include "librbd/image_watcher/RewatchRequest.h"
 #include "include/encoding.h"
 #include "common/errno.h"
 #include "common/WorkQueue.h"
@@ -118,31 +119,43 @@ void ImageWatcher<I>::handle_register_watch(int r) {
 
 template <typename I>
 void ImageWatcher<I>::unregister_watch(Context *on_finish) {
-  ldout(m_image_ctx.cct, 10) << this << " unregistering image watcher" << dendl;
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << this << " unregistering image watcher" << dendl;
 
   cancel_async_requests();
 
-  C_Gather *g = new C_Gather(m_image_ctx.cct, create_async_context_callback(
-          m_image_ctx, on_finish));
-  m_task_finisher->cancel_all(g->new_sub());
-
+  C_Gather *gather_ctx = nullptr;
   {
-    RWLock::WLocker l(m_watch_lock);
+    RWLock::WLocker watch_locker(m_watch_lock);
+    if (m_watch_state == WATCH_STATE_REWATCHING) {
+      ldout(cct, 10) << this << " delaying unregister until rewatch completed"
+                     << dendl;
+
+      assert(m_unregister_watch_ctx == nullptr);
+      m_unregister_watch_ctx = new FunctionContext([this, on_finish](int r) {
+          unregister_watch(on_finish);
+        });
+      return;
+    }
+
+    gather_ctx = new C_Gather(m_image_ctx.cct, create_async_context_callback(
+      m_image_ctx, on_finish));
     if (m_watch_state == WATCH_STATE_REGISTERED) {
       m_watch_state = WATCH_STATE_UNREGISTERED;
 
       librados::AioCompletion *aio_comp = create_rados_safe_callback(
-        new C_UnwatchAndFlush(m_image_ctx.md_ctx, g->new_sub()));
+        new C_UnwatchAndFlush(m_image_ctx.md_ctx, gather_ctx->new_sub()));
       int r = m_image_ctx.md_ctx.aio_unwatch(m_watch_handle, aio_comp);
       assert(r == 0);
       aio_comp->release();
-      g->activate();
-      return;
     } else if (m_watch_state == WATCH_STATE_ERROR) {
       m_watch_state = WATCH_STATE_UNREGISTERED;
     }
   }
-  g->activate();
+
+  assert(gather_ctx != nullptr);
+  m_task_finisher->cancel_all(gather_ctx->new_sub());
+  gather_ctx->activate();
 }
 
 template <typename I>
@@ -998,7 +1011,7 @@ void ImageWatcher<I>::handle_error(uint64_t handle, int err) {
     m_watch_state = WATCH_STATE_ERROR;
 
     FunctionContext *ctx = new FunctionContext(
-      boost::bind(&ImageWatcher<I>::reregister_watch, this));
+      boost::bind(&ImageWatcher<I>::rewatch, this));
     m_task_finisher->queue(TASK_CODE_REREGISTER_WATCH, ctx);
   }
 }
@@ -1010,62 +1023,49 @@ void ImageWatcher<I>::acknowledge_notify(uint64_t notify_id, uint64_t handle,
 }
 
 template <typename I>
-void ImageWatcher<I>::reregister_watch() {
+void ImageWatcher<I>::rewatch() {
   ldout(m_image_ctx.cct, 10) << this << " re-registering image watch" << dendl;
 
-  bool releasing_lock = false;
-  C_SaferCond release_lock_ctx;
-  {
-    RWLock::WLocker l(m_image_ctx.owner_lock);
-    if (m_image_ctx.exclusive_lock != nullptr) {
-      releasing_lock = true;
-      m_image_ctx.exclusive_lock->release_lock(&release_lock_ctx);
-    }
+  RWLock::WLocker l(m_watch_lock);
+  if (m_watch_state != WATCH_STATE_ERROR) {
+    return;
   }
+  m_watch_state = WATCH_STATE_REWATCHING;
 
-  int r;
-  if (releasing_lock) {
-    r = release_lock_ctx.wait();
-    if (r == -EBLACKLISTED) {
-      lderr(m_image_ctx.cct) << this << " client blacklisted" << dendl;
-      return;
-    }
+  Context *ctx = create_context_callback<
+    ImageWatcher<I>, &ImageWatcher<I>::handle_rewatch>(this);
+  RewatchRequest<I> *req = RewatchRequest<I>::create(m_image_ctx, m_watch_lock,
+                                                     &m_watch_ctx,
+                                                     &m_watch_handle, ctx);
+  req->send();
+}
 
-    assert(r == 0);
+template <typename I>
+void ImageWatcher<I>::handle_rewatch(int r) {
+  CephContext *cct = m_image_ctx.cct;
+  ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
+
+  WatchState next_watch_state = WATCH_STATE_REGISTERED;
+  if (r < 0) {
+    next_watch_state = WATCH_STATE_ERROR;
   }
 
+  Context *unregister_watch_ctx = nullptr;
   {
-    RWLock::WLocker l(m_watch_lock);
-    if (m_watch_state != WATCH_STATE_ERROR) {
-      return;
-    }
+    RWLock::WLocker watch_locker(m_watch_lock);
+    assert(m_watch_state == WATCH_STATE_REWATCHING);
+    m_watch_state = next_watch_state;
 
-    r = m_image_ctx.md_ctx.watch2(m_image_ctx.header_oid,
-                                  &m_watch_handle, &m_watch_ctx);
-    if (r < 0) {
-      lderr(m_image_ctx.cct) << this << " failed to re-register image watch: "
-                             << cpp_strerror(r) << dendl;
-      if (r != -ESHUTDOWN) {
-        FunctionContext *ctx = new FunctionContext(boost::bind(
-          &ImageWatcher<I>::reregister_watch, this));
-        m_task_finisher->add_event_after(TASK_CODE_REREGISTER_WATCH,
-                                         RETRY_DELAY_SECONDS, ctx);
-      }
-      return;
-    }
+    std::swap(unregister_watch_ctx, m_unregister_watch_ctx);
 
-    m_watch_state = WATCH_STATE_REGISTERED;
+    // image might have been updated while we didn't have active watch
+    handle_payload(HeaderUpdatePayload(), nullptr);
   }
 
-  // if the exclusive lock state machine was paused waiting for the
-  // watch to be re-registered, wake it up
-  RWLock::RLocker owner_locker(m_image_ctx.owner_lock);
-  RWLock::RLocker snap_locker(m_image_ctx.snap_lock);
-  if (m_image_ctx.exclusive_lock != nullptr) {
-    m_image_ctx.exclusive_lock->handle_watch_registered();
+  // wake up pending unregister request
+  if (unregister_watch_ctx != nullptr) {
+    unregister_watch_ctx->complete(0);
   }
-
-  handle_payload(HeaderUpdatePayload(), NULL);
 }
 
 template <typename I>

diff --git a/src/librbd/ImageWatcher.h b/src/librbd/ImageWatcher.h
index f90d2b46b6440438f847d0a638fe0cbb89f196dc..7dd3561c1a4be51e7e1d244190b74f821bd89765 100644 (file)
--- a/src/librbd/ImageWatcher.h
+++ b/src/librbd/ImageWatcher.h
@@ -64,7 +64,8 @@ private:
   enum WatchState {
     WATCH_STATE_UNREGISTERED,
     WATCH_STATE_REGISTERED,
-    WATCH_STATE_ERROR
+    WATCH_STATE_ERROR,
+    WATCH_STATE_REWATCHING
   };
 
   enum TaskCode {
@@ -226,6 +227,7 @@ private:
   WatchCtx m_watch_ctx;
   uint64_t m_watch_handle;
   WatchState m_watch_state;
+  Context *m_unregister_watch_ctx = nullptr;
 
   TaskFinisher<Task> *m_task_finisher;
 
@@ -310,7 +312,8 @@ private:
   void handle_error(uint64_t cookie, int err);
   void acknowledge_notify(uint64_t notify_id, uint64_t handle, bufferlist &out);
 
-  void reregister_watch();
+  void rewatch();
+  void handle_rewatch(int r);
 };
 
 } // namespace librbd

diff --git a/src/test/librbd/test_mock_ExclusiveLock.cc b/src/test/librbd/test_mock_ExclusiveLock.cc
index a0a87a1ccddf34956fa983d698470f57e9b36f4c..26b066a88d37fe817401eb66635f5dfdf60a931b 100644 (file)
--- a/src/test/librbd/test_mock_ExclusiveLock.cc
+++ b/src/test/librbd/test_mock_ExclusiveLock.cc
@@ -667,11 +667,13 @@ TEST_F(TestMockExclusiveLock, RequestLockWatchNotRegistered) {
   EXPECT_CALL(*mock_image_ctx.image_watcher, get_watch_handle())
     .WillOnce(DoAll(Invoke([&mock_image_ctx, &exclusive_lock]() {
                       mock_image_ctx.image_ctx->op_work_queue->queue(
-                        new FunctionContext([&exclusive_lock](int r) {
-                          exclusive_lock.handle_watch_registered();
+                        new FunctionContext([&mock_image_ctx, &exclusive_lock](int r) {
+                          RWLock::RLocker owner_locker(mock_image_ctx.owner_lock);
+                          exclusive_lock.reacquire_lock();
                         }));
                     }),
                     Return(0)));
+
   MockAcquireRequest request_lock_acquire;
   expect_acquire_lock(mock_image_ctx, request_lock_acquire, 0);
   ASSERT_EQ(0, when_request_lock(mock_image_ctx, exclusive_lock));