librbd: improved state tracking within object watcher

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

diff --git a/src/librbd/Watcher.cc b/src/librbd/Watcher.cc
index edf15d102d7c4fe43d6bdc19df7be64036cc9c77..0c7b147fdf6b28e7a36996f79efb9dca8f40c11c 100644 (file)
--- a/src/librbd/Watcher.cc
+++ b/src/librbd/Watcher.cc
@@ -12,7 +12,8 @@
 
 #define dout_subsys ceph_subsys_rbd
 #undef dout_prefix
-#define dout_prefix *_dout << "librbd::Watcher: "
+#define dout_prefix *_dout << "librbd::Watcher: " << this << " " << __func__ \
+                           << ": "
 
 namespace librbd {
 
@@ -80,10 +81,12 @@ Watcher::~Watcher() {
 }
 
 void Watcher::register_watch(Context *on_finish) {
-  ldout(m_cct, 10) << this << " registering watcher" << dendl;
+  ldout(m_cct, 10) << dendl;
 
   RWLock::RLocker watch_locker(m_watch_lock);
   assert(m_watch_state == WATCH_STATE_UNREGISTERED);
+  m_watch_state = WATCH_STATE_REGISTERING;
+
   librados::AioCompletion *aio_comp = create_rados_safe_callback(
                                          new C_RegisterWatch(this, on_finish));
   int r = m_ioctx.aio_watch(m_oid, aio_comp, &m_watch_handle, &m_watch_ctx);
@@ -91,45 +94,63 @@ void Watcher::register_watch(Context *on_finish) {
   aio_comp->release();
 }
 
-void Watcher::handle_register_watch(int r) {
-  ldout(m_cct, 10) << this << " handle register r=" << r << dendl;
-  RWLock::WLocker watch_locker(m_watch_lock);
-  assert(m_watch_state == WATCH_STATE_UNREGISTERED);
-  if (r < 0) {
-    lderr(m_cct) << ": failed to register watch: " << cpp_strerror(r) << dendl;
-    m_watch_handle = 0;
-  } else if (r >= 0) {
-    m_watch_state = WATCH_STATE_REGISTERED;
+void Watcher::handle_register_watch(int r, Context *on_finish) {
+  ldout(m_cct, 10) << "r=" << r << dendl;
+  Context *unregister_watch_ctx = nullptr;
+  {
+    RWLock::WLocker watch_locker(m_watch_lock);
+    assert(m_watch_state == WATCH_STATE_REGISTERING);
+
+    std::swap(unregister_watch_ctx, m_unregister_watch_ctx);
+    if (r < 0) {
+      lderr(m_cct) << "failed to register watch: " << cpp_strerror(r)
+                   << dendl;
+      m_watch_handle = 0;
+      m_watch_state = WATCH_STATE_UNREGISTERED;
+    } else if (r >= 0) {
+      m_watch_state = WATCH_STATE_REGISTERED;
+    }
+  }
+
+  on_finish->complete(r);
+
+  // wake up pending unregister request
+  if (unregister_watch_ctx != nullptr) {
+    unregister_watch_ctx->complete(0);
   }
 }
 
 void Watcher::unregister_watch(Context *on_finish) {
-  ldout(m_cct, 10) << this << " unregistering watcher" << dendl;
+  ldout(m_cct, 10) << dendl;
 
-  RWLock::WLocker watch_locker(m_watch_lock);
-  if (m_watch_state == WATCH_STATE_REWATCHING) {
-    ldout(m_cct, 10) << this << " delaying unregister until rewatch completed"
-                     << dendl;
+  {
+    RWLock::WLocker watch_locker(m_watch_lock);
+    if (m_watch_state == WATCH_STATE_REGISTERING ||
+        m_watch_state == WATCH_STATE_REWATCHING) {
+      ldout(m_cct, 10) << "delaying unregister until register completed"
+                       << dendl;
+
+      assert(m_unregister_watch_ctx == nullptr);
+      m_unregister_watch_ctx = new FunctionContext([this, on_finish](int r) {
+          unregister_watch(on_finish);
+        });
+      return;
+    }
 
-    assert(m_unregister_watch_ctx == nullptr);
-    m_unregister_watch_ctx = new FunctionContext([this, on_finish](int r) {
-        unregister_watch(on_finish);
-      });
-    return;
-  }
+    if (m_watch_state == WATCH_STATE_REGISTERED ||
+        m_watch_state == WATCH_STATE_ERROR) {
+      m_watch_state = WATCH_STATE_UNREGISTERED;
 
-  if (m_watch_state == WATCH_STATE_REGISTERED ||
-      m_watch_state == WATCH_STATE_ERROR) {
-    m_watch_state = WATCH_STATE_UNREGISTERED;
-
-    librados::AioCompletion *aio_comp = create_rados_safe_callback(
-                      new C_UnwatchAndFlush(m_ioctx, on_finish));
-    int r = m_ioctx.aio_unwatch(m_watch_handle, aio_comp);
-    assert(r == 0);
-    aio_comp->release();
-  } else {
-    on_finish->complete(0);
+      librados::AioCompletion *aio_comp = create_rados_safe_callback(
+                        new C_UnwatchAndFlush(m_ioctx, on_finish));
+      int r = m_ioctx.aio_unwatch(m_watch_handle, aio_comp);
+      assert(r == 0);
+      aio_comp->release();
+      return;
+    }
   }
+
+  on_finish->complete(0);
 }
 
 void Watcher::flush(Context *on_finish) {
@@ -144,8 +165,7 @@ void Watcher::set_oid(const string& oid) {
 }
 
 void Watcher::handle_error(uint64_t handle, int err) {
-  lderr(m_cct) << this << " watch failed: " << handle << ", "
-               << cpp_strerror(err) << dendl;
+  lderr(m_cct) << "handle=" << handle << ": " << cpp_strerror(err) << dendl;
 
   RWLock::WLocker l(m_watch_lock);
   if (m_watch_state == WATCH_STATE_REGISTERED) {
@@ -163,7 +183,7 @@ void Watcher::acknowledge_notify(uint64_t notify_id, uint64_t handle,
 }
 
 void Watcher::rewatch() {
-  ldout(m_cct, 10) << this << " re-registering watch" << dendl;
+  ldout(m_cct, 10) << dendl;
 
   RWLock::WLocker l(m_watch_lock);
   if (m_watch_state != WATCH_STATE_ERROR) {
@@ -180,7 +200,7 @@ void Watcher::rewatch() {
 }
 
 void Watcher::handle_rewatch(int r) {
-  ldout(m_cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
+  ldout(m_cct, 10) "r=" << r << dendl;
 
   WatchState next_watch_state = WATCH_STATE_REGISTERED;
   if (r < 0) {

diff --git a/src/librbd/Watcher.h b/src/librbd/Watcher.h
index bf4b94080dbdf715cfcc37a3459c5a7568e2f3b8..f7fb3f48fd7f426c546058ba8b88368791aa77e3 100644 (file)
--- a/src/librbd/Watcher.h
+++ b/src/librbd/Watcher.h
@@ -39,10 +39,15 @@ public:
     RWLock::RLocker locker(m_watch_lock);
     return m_watch_state == WATCH_STATE_REGISTERED;
   }
+  bool is_unregistered() const {
+    RWLock::RLocker locker(m_watch_lock);
+    return m_watch_state == WATCH_STATE_UNREGISTERED;
+  }
 
 protected:
   enum WatchState {
     WATCH_STATE_UNREGISTERED,
+    WATCH_STATE_REGISTERING,
     WATCH_STATE_REGISTERED,
     WATCH_STATE_ERROR,
     WATCH_STATE_REWATCHING
@@ -81,6 +86,8 @@ private:
    *    |
    *    | (register_watch)
    *    |
+   * REGISTERING
+   *    |
    *    v      (watch error)
    * REGISTERED * * * * * * * > ERROR
    *    |   ^                     |
@@ -122,15 +129,14 @@ private:
        : watcher(watcher), on_finish(on_finish) {
     }
     virtual void finish(int r) override {
-      watcher->handle_register_watch(r);
-      on_finish->complete(r);
+      watcher->handle_register_watch(r, on_finish);
     }
   };
 
   WatchCtx m_watch_ctx;
   Context *m_unregister_watch_ctx = nullptr;
 
-  void handle_register_watch(int r);
+  void handle_register_watch(int r, Context *on_finish);
 
   void rewatch();
   void handle_rewatch(int r);