librbd: managed lock refactoring

src/librbd/ExclusiveLock.cc

@@ -26,7 +26,8 @@ using ML = ManagedLock<I>;
 template <typename I>
 ExclusiveLock<I>::ExclusiveLock(I &image_ctx)
   : ML<I>(image_ctx.md_ctx, image_ctx.op_work_queue, image_ctx.header_oid,
-          image_ctx.image_watcher),
+          image_ctx.image_watcher, image_ctx.blacklist_on_break_lock,
+          image_ctx.blacklist_expire_seconds),
     m_image_ctx(image_ctx), m_pre_post_callback(nullptr),
     m_shutting_down(false)  {
   ML<I>::m_state = ML<I>::STATE_UNINITIALIZED;

src/librbd/ManagedLock.cc

@@ -3,8 +3,11 @@
 
 #include "librbd/ManagedLock.h"
 #include "librbd/managed_lock/AcquireRequest.h"
+#include "librbd/managed_lock/BreakRequest.h"
+#include "librbd/managed_lock/GetLockerRequest.h"
 #include "librbd/managed_lock/ReleaseRequest.h"
 #include "librbd/managed_lock/ReacquireRequest.h"
+#include "librbd/managed_lock/Types.h"
 #include "librbd/Watcher.h"
 #include "librbd/ImageCtx.h"
 #include "cls/lock/cls_lock_client.h"
@@ -43,13 +46,17 @@ const std::string ManagedLock<I>::WATCHER_LOCK_TAG("internal");
 
 template <typename I>
 ManagedLock<I>::ManagedLock(librados::IoCtx &ioctx, ContextWQ *work_queue,
-                            const string& oid, Watcher *watcher)
+                            const string& oid, Watcher *watcher,
+                            bool blacklist_on_break_lock,
+                            uint32_t blacklist_expire_seconds)
   : m_lock(util::unique_lock_name("librbd::ManagedLock<I>::m_lock", this)),
     m_state(STATE_UNLOCKED),
     m_ioctx(ioctx), m_cct(reinterpret_cast<CephContext *>(ioctx.cct())),
     m_work_queue(work_queue),
     m_oid(oid),
-    m_watcher(watcher) {
+    m_watcher(watcher),
+    m_blacklist_on_break_lock(blacklist_on_break_lock),
+    m_blacklist_expire_seconds(blacklist_expire_seconds) {
 }
 
 template <typename I>
@@ -63,6 +70,14 @@ template <typename I>
 bool ManagedLock<I>::is_lock_owner() const {
   Mutex::Locker locker(m_lock);
 
+  return is_lock_owner(m_lock);
+}
+
+template <typename I>
+bool ManagedLock<I>::is_lock_owner(Mutex &lock) const {
+
+  assert(m_lock.is_locked());
+
   bool lock_owner;
 
   switch (m_state) {
@@ -178,6 +193,33 @@ void ManagedLock<I>::reacquire_lock(Context *on_reacquired) {
   }
 }
 
+template <typename I>
+void ManagedLock<I>::get_locker(managed_lock::Locker *locker,
+                                Context *on_finish) {
+  ldout(m_cct, 10) << dendl;
+
+  auto req = managed_lock::GetLockerRequest<I>::create(
+    m_ioctx, m_oid, locker, on_finish);
+  req->send();
+}
+
+template <typename I>
+void ManagedLock<I>::break_lock(const managed_lock::Locker &locker,
+                                bool force_break_lock, Context *on_finish) {
+  {
+    Mutex::Locker l(m_lock);
+    if (!is_lock_owner(m_lock)) {
+      auto req = managed_lock::BreakRequest<I>::create(
+        m_ioctx, m_work_queue, m_oid, locker, m_blacklist_on_break_lock,
+        m_blacklist_expire_seconds, force_break_lock, on_finish);
+      req->send();
+      return;
+    }
+  }
+
+  on_finish->complete(-EBUSY);
+}
+
 template <typename I>
 void ManagedLock<I>::shutdown_handler(int r, Context *on_finish) {
   on_finish->complete(r);
@@ -376,9 +418,10 @@ void ManagedLock<I>::handle_pre_acquire_lock(int r) {
   }
 
   using managed_lock::AcquireRequest;
-  AcquireRequest<I>* req = AcquireRequest<I>::create(m_ioctx, m_watcher,
-      m_work_queue, m_oid, m_cookie,
-      util::create_context_callback<
+  AcquireRequest<I>* req = AcquireRequest<I>::create(
+    m_ioctx, m_watcher, m_work_queue, m_oid, m_cookie,
+    m_blacklist_on_break_lock, m_blacklist_expire_seconds,
+    util::create_context_callback<
         ManagedLock<I>, &ManagedLock<I>::handle_acquire_lock>(this));
   m_work_queue->queue(new C_SendLockRequest<AcquireRequest<I>>(req), 0);
 }

src/librbd/ManagedLock.h

@@ -20,6 +20,8 @@ namespace librbd {
 
 struct ImageCtx;
 
+namespace managed_lock { struct Locker; }
+
 template <typename ImageCtxT = librbd::ImageCtx>
 class ManagedLock {
 private:
@@ -30,12 +32,17 @@ class ManagedLock {
   static const std::string WATCHER_LOCK_TAG;
 
   static ManagedLock *create(librados::IoCtx& ioctx, ContextWQ *work_queue,
-                             const std::string& oid, Watcher *watcher) {
-    return new ManagedLock(ioctx, work_queue, oid, watcher);
+                             const std::string& oid, Watcher *watcher,
+                             bool blacklist_on_break_lock,
+                             uint32_t blacklist_expire_seconds) {
+    return new ManagedLock(ioctx, work_queue, oid, watcher,
+                           blacklist_on_break_lock, blacklist_expire_seconds);
   }
 
   ManagedLock(librados::IoCtx& ioctx, ContextWQ *work_queue,
-              const std::string& oid, Watcher *watcher);
+              const std::string& oid, Watcher *watcher,
+              bool blacklist_on_break_lock,
+              uint32_t blacklist_expire_seconds);
   virtual ~ManagedLock();
 
   bool is_lock_owner() const;
@@ -45,6 +52,9 @@ class ManagedLock {
   void try_acquire_lock(Context *on_acquired);
   void release_lock(Context *on_released);
   void reacquire_lock(Context *on_reacquired = nullptr);
+  void get_locker(managed_lock::Locker *locker, Context *on_finish);
+  void break_lock(const managed_lock::Locker &locker, bool force_break_lock,
+                  Context *on_finish);
 
   bool is_shutdown() const {
     Mutex::Locker l(m_lock);
@@ -151,6 +161,8 @@ class ManagedLock {
   ContextWQ *m_work_queue;
   std::string m_oid;
   Watcher *m_watcher;
+  bool m_blacklist_on_break_lock;
+  uint32_t m_blacklist_expire_seconds;
 
   std::string m_cookie;
   std::string m_new_cookie;
@@ -161,6 +173,7 @@ class ManagedLock {
 
   static std::string encode_lock_cookie(uint64_t watch_handle);
 
+  bool is_lock_owner(Mutex &lock) const;
   bool is_transition_state() const;
 
   void append_context(Action action, Context *ctx);

src/librbd/internal.cc

@@ -33,8 +33,6 @@
 #include "librbd/internal.h"
 #include "librbd/Journal.h"
 #include "librbd/journal/Types.h"
-#include "librbd/managed_lock/BreakRequest.h"
-#include "librbd/managed_lock/GetLockerRequest.h"
 #include "librbd/managed_lock/Types.h"
 #include "librbd/mirror/DisableRequest.h"
 #include "librbd/mirror/EnableRequest.h"
@@ -1568,12 +1566,14 @@ void filter_out_mirror_watchers(ImageCtx *ictx,
     CephContext *cct = ictx->cct;
     ldout(cct, 20) << __func__ << ": ictx=" << ictx << dendl;
 
+    if (!ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) {
+      lderr(cct) << "exclusive-lock feature is not enabled" << dendl;
+      return -EINVAL;
+    }
+
     managed_lock::Locker locker;
     C_SaferCond get_owner_ctx;
-    auto get_owner_req = managed_lock::GetLockerRequest<>::create(
-      *ictx, &locker, &get_owner_ctx);
-    get_owner_req->send();
-
+    ExclusiveLock<>(*ictx).get_locker(&locker, &get_owner_ctx);
     int r = get_owner_ctx.wait();
     if (r == -ENOENT) {
       return r;
@@ -1601,12 +1601,22 @@ void filter_out_mirror_watchers(ImageCtx *ictx,
       return -EOPNOTSUPP;
     }
 
+    if (ictx->read_only) {
+      return -EROFS;
+    }
+
     managed_lock::Locker locker;
     C_SaferCond get_owner_ctx;
-    auto get_owner_req = managed_lock::GetLockerRequest<>::create(
-      *ictx, &locker, &get_owner_ctx);
-    get_owner_req->send();
+    {
+      RWLock::RLocker l(ictx->owner_lock);
 
+      if (ictx->exclusive_lock == nullptr) {
+        lderr(cct) << "exclusive-lock feature is not enabled" << dendl;
+        return -EINVAL;
+      }
+
+      ictx->exclusive_lock->get_locker(&locker, &get_owner_ctx);
+    }
     int r = get_owner_ctx.wait();
     if (r == -ENOENT) {
       return r;
@@ -1621,10 +1631,16 @@ void filter_out_mirror_watchers(ImageCtx *ictx,
     }
 
     C_SaferCond break_ctx;
-    auto break_req = managed_lock::BreakRequest<>::create(
-      *ictx, locker, ictx->blacklist_on_break_lock, true, &break_ctx);
-    break_req->send();
+    {
+      RWLock::RLocker l(ictx->owner_lock);
 
+      if (ictx->exclusive_lock == nullptr) {
+        lderr(cct) << "exclusive-lock feature is not enabled" << dendl;
+        return -EINVAL;
+      }
+
+      ictx->exclusive_lock->break_lock(locker, true, &break_ctx);
+    }
     r = break_ctx.wait();
     if (r == -ENOENT) {
       return r;