librbd: remove image header lock assertions

This assertions can sporadically fail if the watch is lost and
recovered in the background. Upon a true loss of the lock, the
client would either be blacklisted or it would have completed
all in-flight ops before releasing.

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

Conflicts:
	src/librbd/operation/SnapshotCreateRequest.cc: rbd class
	   does not support the snapshot namespaces in Jewel, skip
	   the corresponding argument

diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc
index 3fedf8ddee347e4e9abed1f7bc10df61a0bd3abb..6530813ebbfd7ae34a5fb58c93bd7f74675a15e7 100644 (file)
--- a/src/librbd/ExclusiveLock.cc
+++ b/src/librbd/ExclusiveLock.cc
@@ -249,13 +249,6 @@ void ExclusiveLock<I>::handle_peer_notification(int r) {
   handle_acquire_lock(r);
 }
 
-template <typename I>
-void ExclusiveLock<I>::assert_header_locked(librados::ObjectWriteOperation *op) {
-  Mutex::Locker locker(m_lock);
-  rados::cls::lock::assert_locked(op, RBD_LOCK_NAME, LOCK_EXCLUSIVE,
-                                  m_cookie, WATCHER_LOCK_TAG);
-}
-
 template <typename I>
 std::string ExclusiveLock<I>::encode_lock_cookie() const {
   assert(m_lock.is_locked());

diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h
index 531567a52aed1da45ad66b61860fb3c911bb6352..58eaa9376893be3d39b203e9b6dfd52e3f827ef2 100644 (file)
--- a/src/librbd/ExclusiveLock.h
+++ b/src/librbd/ExclusiveLock.h
@@ -45,8 +45,6 @@ public:
 
   void handle_peer_notification(int r);
 
-  void assert_header_locked(librados::ObjectWriteOperation *op);
-
   static bool decode_lock_cookie(const std::string &cookie, uint64_t *handle);
 
 private:

diff --git a/src/librbd/object_map/InvalidateRequest.cc b/src/librbd/object_map/InvalidateRequest.cc
index e60d04fe099a7a9f2bbff631eb8f4aae179544fd..e744add68db857b3b98f7b62f36603675c4f1700 100644 (file)
--- a/src/librbd/object_map/InvalidateRequest.cc
+++ b/src/librbd/object_map/InvalidateRequest.cc
@@ -59,10 +59,6 @@ void InvalidateRequest<I>::send() {
 
   lderr(cct) << this << " invalidating object map on-disk" << dendl;
   librados::ObjectWriteOperation op;
-  if (image_ctx.exclusive_lock != nullptr &&
-      m_snap_id == CEPH_NOSNAP && !m_force) {
-    image_ctx.exclusive_lock->assert_header_locked(&op);
-  }
   cls_client::set_flags(&op, m_snap_id, flags, flags);
 
   librados::AioCompletion *rados_completion =

diff --git a/src/librbd/operation/FlattenRequest.cc b/src/librbd/operation/FlattenRequest.cc
index 8cfddbeac74dcffe80f45bd369f20e8579fa9100..911534dea2ca8b6f14458b3fdbba75268d794a91 100644 (file)
--- a/src/librbd/operation/FlattenRequest.cc
+++ b/src/librbd/operation/FlattenRequest.cc
@@ -140,9 +140,6 @@ bool FlattenRequest<I>::send_update_header() {
 
   // remove parent from this (base) image
   librados::ObjectWriteOperation op;
-  if (image_ctx.exclusive_lock != nullptr) {
-    image_ctx.exclusive_lock->assert_header_locked(&op);
-  }
   cls_client::remove_parent(&op);
 
   librados::AioCompletion *rados_completion = this->create_callback_completion();

diff --git a/src/librbd/operation/RebuildObjectMapRequest.cc b/src/librbd/operation/RebuildObjectMapRequest.cc
index 352a439a83bd8075330caf4658ad894ee53093a5..6d5ec4ab7de839c4a5b290f30fceb53b853ae0e7 100644 (file)
--- a/src/librbd/operation/RebuildObjectMapRequest.cc
+++ b/src/librbd/operation/RebuildObjectMapRequest.cc
@@ -352,9 +352,6 @@ void RebuildObjectMapRequest<I>::send_update_header() {
   m_state = STATE_UPDATE_HEADER;
 
   librados::ObjectWriteOperation op;
-  if (m_image_ctx.exclusive_lock != nullptr) {
-    m_image_ctx.exclusive_lock->assert_header_locked(&op);
-  }
 
   uint64_t flags = RBD_FLAG_OBJECT_MAP_INVALID | RBD_FLAG_FAST_DIFF_INVALID;
   cls_client::set_flags(&op, m_image_ctx.snap_id, 0, flags);

diff --git a/src/librbd/operation/ResizeRequest.cc b/src/librbd/operation/ResizeRequest.cc
index f890537570ddae49f021073c0948b70888d133b6..2c0046ca6b99a36f5bffaac30e4a1a9512a05f70 100644 (file)
--- a/src/librbd/operation/ResizeRequest.cc
+++ b/src/librbd/operation/ResizeRequest.cc
@@ -381,9 +381,6 @@ void ResizeRequest<I>::send_update_header() {
     bl.append(reinterpret_cast<const char*>(&m_new_size), sizeof(m_new_size));
     op.write(offsetof(rbd_obj_header_ondisk, image_size), bl);
   } else {
-    if (image_ctx.exclusive_lock != nullptr) {
-      image_ctx.exclusive_lock->assert_header_locked(&op);
-    }
     cls_client::set_size(&op, m_new_size);
   }
 

diff --git a/src/librbd/operation/SnapshotCreateRequest.cc b/src/librbd/operation/SnapshotCreateRequest.cc
index 4f20ec4f977f10ad437a8f8408b379161557839c..46f4a6d90440ec5ae22e74c39af739097898cebc 100644 (file)
--- a/src/librbd/operation/SnapshotCreateRequest.cc
+++ b/src/librbd/operation/SnapshotCreateRequest.cc
@@ -207,9 +207,6 @@ void SnapshotCreateRequest<I>::send_create_snap() {
   if (image_ctx.old_format) {
     cls_client::old_snapshot_add(&op, m_snap_id, m_snap_name);
   } else {
-    if (image_ctx.exclusive_lock != nullptr) {
-      image_ctx.exclusive_lock->assert_header_locked(&op);
-    }
     cls_client::snapshot_add(&op, m_snap_id, m_snap_name);
   }
 

diff --git a/src/librbd/operation/SnapshotRemoveRequest.cc b/src/librbd/operation/SnapshotRemoveRequest.cc
index eadbfc2a106bf92959f1d5feceab541939586a0a..1e68a01bcd6616811322838df40e8157b03f7466 100644 (file)
--- a/src/librbd/operation/SnapshotRemoveRequest.cc
+++ b/src/librbd/operation/SnapshotRemoveRequest.cc
@@ -180,10 +180,6 @@ void SnapshotRemoveRequest<I>::send_remove_snap() {
   if (image_ctx.old_format) {
     cls_client::old_snapshot_remove(&op, m_snap_name);
   } else {
-    if (image_ctx.exclusive_lock != nullptr &&
-        image_ctx.exclusive_lock->is_lock_owner()) {
-      image_ctx.exclusive_lock->assert_header_locked(&op);
-    }
     cls_client::snapshot_remove(&op, m_snap_id);
   }
 

diff --git a/src/librbd/operation/SnapshotRenameRequest.cc b/src/librbd/operation/SnapshotRenameRequest.cc
index 5b836cd061c9cc6a9acc129b4d21847ddcbbc567..236d82815b3ea7220b429373c32d7f25bd395fe1 100644 (file)
--- a/src/librbd/operation/SnapshotRenameRequest.cc
+++ b/src/librbd/operation/SnapshotRenameRequest.cc
@@ -89,10 +89,6 @@ void SnapshotRenameRequest<I>::send_rename_snap() {
   if (image_ctx.old_format) {
     cls_client::old_snapshot_rename(&op, m_snap_id, m_snap_name);
   } else {
-    if (image_ctx.exclusive_lock != nullptr &&
-        image_ctx.exclusive_lock->is_lock_owner()) {
-      image_ctx.exclusive_lock->assert_header_locked(&op);
-    }
     cls_client::snapshot_rename(&op, m_snap_id, m_snap_name);
   }
 

diff --git a/src/test/librbd/mock/MockExclusiveLock.h b/src/test/librbd/mock/MockExclusiveLock.h
index 83d3058d1d8bd7a34504d9dd7079a764f7ef6382..7d8629c349f50b92f5daad84ccf1200df51e1c37 100644 (file)
--- a/src/test/librbd/mock/MockExclusiveLock.h
+++ b/src/test/librbd/mock/MockExclusiveLock.h
@@ -15,8 +15,6 @@ namespace librbd {
 struct MockExclusiveLock {
   MOCK_CONST_METHOD0(is_lock_owner, bool());
 
-  MOCK_METHOD1(assert_header_locked, void(librados::ObjectWriteOperation *));
-
   MOCK_METHOD2(init, void(uint64_t features, Context*));
   MOCK_METHOD1(shut_down, void(Context*));
 

diff --git a/src/test/librbd/object_map/test_mock_InvalidateRequest.cc b/src/test/librbd/object_map/test_mock_InvalidateRequest.cc
index 4f3831cf05953c4612144601c68dbe2afc3a5074..3c801e6e5773342e4a5a9b2c6b642320db7c919e 100644 (file)
--- a/src/test/librbd/object_map/test_mock_InvalidateRequest.cc
+++ b/src/test/librbd/object_map/test_mock_InvalidateRequest.cc
@@ -55,9 +55,6 @@ TEST_F(TestMockObjectMapInvalidateRequest, UpdatesHeadOnDiskFlag) {
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new InvalidateRequest<>(*ictx, CEPH_NOSNAP, false, &cond_ctx);
 
-  EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
-              exec(ictx->header_oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _))
-                .WillOnce(DoDefault());
   EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
               exec(ictx->header_oid, _, StrEq("rbd"), StrEq("set_flags"), _, _, _))
                 .WillOnce(DoDefault());
@@ -85,9 +82,6 @@ TEST_F(TestMockObjectMapInvalidateRequest, UpdatesSnapOnDiskFlag) {
   AsyncRequest<> *request = new InvalidateRequest<>(*ictx, ictx->snap_id, false,
                                                 &cond_ctx);
 
-  EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
-              exec(ictx->header_oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _))
-                .Times(0);
   EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
               exec(ictx->header_oid, _, StrEq("rbd"), StrEq("set_flags"), _, _, _))
                 .WillOnce(DoDefault());
@@ -133,9 +127,6 @@ TEST_F(TestMockObjectMapInvalidateRequest, IgnoresOnDiskUpdateFailure) {
   C_SaferCond cond_ctx;
   AsyncRequest<> *request = new InvalidateRequest<>(*ictx, CEPH_NOSNAP, false, &cond_ctx);
 
-  EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
-              exec(ictx->header_oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _))
-                .WillOnce(DoDefault());
   EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
               exec(ictx->header_oid, _, StrEq("rbd"), StrEq("set_flags"), _, _, _))
                 .WillOnce(Return(-EINVAL));

diff --git a/src/test/librbd/object_map/test_mock_ResizeRequest.cc b/src/test/librbd/object_map/test_mock_ResizeRequest.cc
index 44fd714353ba1f9b9e6564e8e722c77fe30503ca..5d0544c1b8491361934995b746b253e47754e0df 100644 (file)
--- a/src/test/librbd/object_map/test_mock_ResizeRequest.cc
+++ b/src/test/librbd/object_map/test_mock_ResizeRequest.cc
@@ -41,8 +41,6 @@ public:
   }
 
   void expect_invalidate(librbd::ImageCtx *ictx) {
-    EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
-                exec(ictx->header_oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _)).Times(0);
     EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
                 exec(ictx->header_oid, _, StrEq("rbd"), StrEq("set_flags"), _, _, _))
                   .WillOnce(DoDefault());

diff --git a/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc b/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc
index 270b267c53fed38df0268890ef1ef77b533dc7da..c1fadcfb390434b25d8222ac38b1f8e7ae7a4c21 100644 (file)
--- a/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc
+++ b/src/test/librbd/object_map/test_mock_SnapshotRemoveRequest.cc
@@ -63,9 +63,6 @@ public:
   }
 
   void expect_invalidate(librbd::ImageCtx *ictx) {
-    EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
-                exec(ictx->header_oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _))
-                  .Times(0);
     EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
                 exec(ictx->header_oid, _, StrEq("rbd"), StrEq("set_flags"), _, _, _))
                   .WillOnce(DoDefault());

diff --git a/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc b/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc
index ff3e06a8d70c5cc54c4b4958972703c3f247a82d..3a21db640c5656aa0cd046b581a15bc6b503a9b4 100644 (file)
--- a/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc
+++ b/src/test/librbd/object_map/test_mock_SnapshotRollbackRequest.cc
@@ -52,9 +52,6 @@ public:
   }
 
   void expect_invalidate(librbd::ImageCtx *ictx, uint32_t times) {
-    EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
-                exec(ictx->header_oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _))
-                  .Times(0);
     EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
                 exec(ictx->header_oid, _, StrEq("rbd"), StrEq("set_flags"), _, _, _))
                   .Times(times)

diff --git a/src/test/librbd/object_map/test_mock_UpdateRequest.cc b/src/test/librbd/object_map/test_mock_UpdateRequest.cc
index f82ffb82ac21da4be3ef19d9fabde193467bd3c2..20314308fef3ba06571a0733fe1daa76688ae4c4 100644 (file)
--- a/src/test/librbd/object_map/test_mock_UpdateRequest.cc
+++ b/src/test/librbd/object_map/test_mock_UpdateRequest.cc
@@ -43,9 +43,6 @@ public:
   }
 
   void expect_invalidate(librbd::ImageCtx *ictx) {
-    EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
-                exec(ictx->header_oid, _, StrEq("lock"), StrEq("assert_locked"), _, _, _))
-                  .Times(0);
     EXPECT_CALL(get_mock_io_ctx(ictx->md_ctx),
                 exec(ictx->header_oid, _, StrEq("rbd"), StrEq("set_flags"), _, _, _))
                   .WillOnce(DoDefault());

diff --git a/src/test/librbd/operation/test_mock_ResizeRequest.cc b/src/test/librbd/operation/test_mock_ResizeRequest.cc
index f1276ebd4cad063d0bc493ca0f76aa7e3ddcfd4e..80d2981462fed33d6671503fbfecc88c06bebaa1 100644 (file)
--- a/src/test/librbd/operation/test_mock_ResizeRequest.cc
+++ b/src/test/librbd/operation/test_mock_ResizeRequest.cc
@@ -101,9 +101,6 @@ public:
                     .WillOnce(Return(r));
     } else {
       expect_is_lock_owner(mock_image_ctx);
-      if (mock_image_ctx.exclusive_lock != nullptr) {
-        EXPECT_CALL(*mock_image_ctx.exclusive_lock, assert_header_locked(_));
-      }
       EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
                   exec(mock_image_ctx.header_oid, _, StrEq("rbd"), StrEq("set_size"), _, _, _))
                     .WillOnce(Return(r));

diff --git a/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc b/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc
index 1b94b7e1f9f1bf4edc8153c97cdd77f7652a7540..4ff2cbcaa043ab6feed7783e576e61b8039e68e9 100644 (file)
--- a/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc
+++ b/src/test/librbd/operation/test_mock_SnapshotCreateRequest.cc
@@ -63,12 +63,6 @@ public:
   }
 
   void expect_snap_create(MockImageCtx &mock_image_ctx, int r) {
-    if (!mock_image_ctx.old_format &&
-         mock_image_ctx.exclusive_lock != nullptr) {
-      EXPECT_CALL(*mock_image_ctx.exclusive_lock, assert_header_locked(_))
-                    .Times(r == -ESTALE ? 2 : 1);
-    }
-
     auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx),
                                exec(mock_image_ctx.header_oid, _, StrEq("rbd"),
                                StrEq(mock_image_ctx.old_format ? "snap_add" :