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>

diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc
index 48ecbf850f57f3e8c64ee51a7892ee3bfa3b5de3..58fc21e78a0891a014696d49362d1eabe528d803 100644 (file)
--- a/src/librbd/ExclusiveLock.cc
+++ b/src/librbd/ExclusiveLock.cc
@@ -244,13 +244,6 @@ void ExclusiveLock<I>::handle_peer_notification() {
   execute_next_action();
 }
 
-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 93ddd903e10191f91cdae475c2a31b6f8c8094ac..c313a895981936e7cd7051b9bd16980b49b047f9 100644 (file)
--- a/src/librbd/ExclusiveLock.h
+++ b/src/librbd/ExclusiveLock.h
@@ -45,8 +45,6 @@ public:
 
   void handle_peer_notification();
 
-  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 281c24c598e4ed24c46ebf035d2b018f901e6ca2..ce8ee858b850ab025a460bb54229ba50ee25d60d 100644 (file)
--- a/src/librbd/operation/RebuildObjectMapRequest.cc
+++ b/src/librbd/operation/RebuildObjectMapRequest.cc
@@ -215,9 +215,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 67207d95d8f8c7a8f4e4f632d6e3fd2834741b95..edd1ca36f08c42553f7b6aba12ba35f57f858b09 100644 (file)
--- a/src/librbd/operation/ResizeRequest.cc
+++ b/src/librbd/operation/ResizeRequest.cc
@@ -388,9 +388,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 45d1ba619f14fec6591cd07ec64760b1fffee5d0..12707b8caf9a20876f33d1239e0a0df9148f96c5 100644 (file)
--- a/src/librbd/operation/SnapshotCreateRequest.cc
+++ b/src/librbd/operation/SnapshotCreateRequest.cc
@@ -210,9 +210,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, m_snap_namespace);
   }
 

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 d15c8f53287aca5d40409f8e9a2b2cd29f2eabf9..6e3621ad82bd29cf5fc469ef16e3a2e1cfc380f3 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 42007d34487a44cddb58ccb585b1a0186769452e..d2e93a9ebb31584669dcd7ac77eed40ac99f48d1 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 215c214eb8b10be4110698367e53fae7e01412da..059237a1808996a78290680fcd6c500e772a87b3 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 51fc932c0b07a9a3cda97fa88068f342d6193d0d..3cff4b4f26d934f3cdf2bf7bfdf3a24c731a9c36 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 45879f2f83e608d99dd57337d3aa193bf93e177e..7964ffc283ae7115e19366e682b2d6828c347764 100644 (file)
--- a/src/test/librbd/object_map/test_mock_UpdateRequest.cc
+++ b/src/test/librbd/object_map/test_mock_UpdateRequest.cc
@@ -42,9 +42,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 17879ecddf3d450fedfb153ab597054bf5105219..5a996599262fadf2bc55f0b48420520f8b00367c 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 23398cd197f92a9c344956641a895417ab7fca3b..6c845bf2e05b5cbc0dc6dcee47d3400d0700b08d 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" :