librbd: skip flush from exclusive-lock dispatch layer on init/shutdown

If the exclusive-lock layer is being initialized/shut down at image
open/close, there is no IO flowing so there is no need to flush.

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

diff --git a/src/librbd/ExclusiveLock.cc b/src/librbd/ExclusiveLock.cc
index a475d1c53e3b341c5a9b360cbabbd8e184f3a87b..92ddc665b7f69462bb365fb92af66b9a32197fd6 100644 (file)
--- a/src/librbd/ExclusiveLock.cc
+++ b/src/librbd/ExclusiveLock.cc
@@ -75,9 +75,10 @@ bool ExclusiveLock<I>::accept_ops(const ceph::mutex &lock) const {
 }
 
 template <typename I>
-void ExclusiveLock<I>::set_require_lock(io::Direction direction,
+void ExclusiveLock<I>::set_require_lock(bool init_shutdown,
+                                        io::Direction direction,
                                         Context* on_finish) {
-  m_image_dispatch->set_require_lock(direction, on_finish);
+  m_image_dispatch->set_require_lock(init_shutdown, direction, on_finish);
 }
 
 template <typename I>
@@ -209,9 +210,9 @@ void ExclusiveLock<I>::handle_init_complete(int r, uint64_t features,
   if (m_image_ctx.clone_copy_on_read ||
       (features & RBD_FEATURE_JOURNALING) != 0 ||
       rwl_enabled) {
-    m_image_dispatch->set_require_lock(io::DIRECTION_BOTH, on_finish);
+    m_image_dispatch->set_require_lock(true, io::DIRECTION_BOTH, on_finish);
   } else {
-    m_image_dispatch->set_require_lock(io::DIRECTION_WRITE, on_finish);
+    m_image_dispatch->set_require_lock(true, io::DIRECTION_WRITE, on_finish);
   }
 }
 

diff --git a/src/librbd/ExclusiveLock.h b/src/librbd/ExclusiveLock.h
index b77c961e2f0ccf34dee7e5da4371d19d4d076932..4a3215ce98c361ff187b7c73cdb1d0b2f999505e 100644 (file)
--- a/src/librbd/ExclusiveLock.h
+++ b/src/librbd/ExclusiveLock.h
@@ -30,7 +30,8 @@ public:
                       int *ret_val) const;
   bool accept_ops() const;
 
-  void set_require_lock(io::Direction direction, Context* on_finish);
+  void set_require_lock(bool init_shutdown, io::Direction direction,
+                        Context* on_finish);
   void unset_require_lock(io::Direction direction);
 
   void block_requests(int r);

diff --git a/src/librbd/exclusive_lock/ImageDispatch.cc b/src/librbd/exclusive_lock/ImageDispatch.cc
index fe3ab14bace24e3fb6805965443e50e98925297a..e734cecf22a64cc11d87eb9f2ac5bdb388650a9d 100644 (file)
--- a/src/librbd/exclusive_lock/ImageDispatch.cc
+++ b/src/librbd/exclusive_lock/ImageDispatch.cc
@@ -57,7 +57,8 @@ void ImageDispatch<I>::shut_down(Context* on_finish) {
 }
 
 template <typename I>
-void ImageDispatch<I>::set_require_lock(io::Direction direction,
+void ImageDispatch<I>::set_require_lock(bool init_shutdown,
+                                        io::Direction direction,
                                         Context* on_finish) {
   // pause any matching IO from proceeding past this layer
   set_require_lock(direction, true);
@@ -72,7 +73,9 @@ void ImageDispatch<I>::set_require_lock(io::Direction direction,
     on_finish, util::get_image_ctx(m_image_ctx), io::AIO_TYPE_FLUSH);
   auto req = io::ImageDispatchSpec::create_flush(
     *m_image_ctx, io::IMAGE_DISPATCH_LAYER_EXCLUSIVE_LOCK, aio_comp,
-    io::FLUSH_SOURCE_EXCLUSIVE_LOCK, {});
+    (init_shutdown ?
+      io::FLUSH_SOURCE_EXCLUSIVE_LOCK_SKIP_REFRESH :
+      io::FLUSH_SOURCE_EXCLUSIVE_LOCK), {});
   req->send();
 }
 

diff --git a/src/librbd/exclusive_lock/ImageDispatch.h b/src/librbd/exclusive_lock/ImageDispatch.h
index 546163e87b2106874355666f7d120c22c98cb01a..2a20407f3169657942006ef0b366fbe6dbe74e3a 100644 (file)
--- a/src/librbd/exclusive_lock/ImageDispatch.h
+++ b/src/librbd/exclusive_lock/ImageDispatch.h
@@ -45,7 +45,8 @@ public:
     return io::IMAGE_DISPATCH_LAYER_EXCLUSIVE_LOCK;
   }
 
-  void set_require_lock(io::Direction direction, Context* on_finish);
+  void set_require_lock(bool init_shutdown,
+                        io::Direction direction, Context* on_finish);
   void unset_require_lock(io::Direction direction);
 
   void shut_down(Context* on_finish) override;

diff --git a/src/librbd/exclusive_lock/PreReleaseRequest.cc b/src/librbd/exclusive_lock/PreReleaseRequest.cc
index 5eae45dab3a7c714d3905caf869de695aad7a127..644aa57415e689c8f728889c78fbd95dded9250f 100644 (file)
--- a/src/librbd/exclusive_lock/PreReleaseRequest.cc
+++ b/src/librbd/exclusive_lock/PreReleaseRequest.cc
@@ -116,9 +116,11 @@ void PreReleaseRequest<I>::send_block_writes() {
   // queue to re-request the lock if any IO is queued
   if (m_image_ctx.clone_copy_on_read ||
       m_image_ctx.test_features(RBD_FEATURE_JOURNALING)) {
-    m_image_dispatch->set_require_lock(io::DIRECTION_BOTH, ctx);
+    m_image_dispatch->set_require_lock(m_shutting_down,
+                                       io::DIRECTION_BOTH, ctx);
   } else {
-    m_image_dispatch->set_require_lock(io::DIRECTION_WRITE, ctx);
+    m_image_dispatch->set_require_lock(m_shutting_down,
+                                       io::DIRECTION_WRITE, ctx);
   }
 }
 

diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc
index b66f375315f14e8f1b485d772299a6c4110206de..ce125e2ce398fb274a12caee137e083985824548 100644 (file)
--- a/src/librbd/image/RefreshRequest.cc
+++ b/src/librbd/image/RefreshRequest.cc
@@ -922,7 +922,7 @@ void RefreshRequest<I>::send_v2_open_journal() {
           send_v2_block_writes();
         });
       m_image_ctx.exclusive_lock->set_require_lock(
-        librbd::io::DIRECTION_BOTH, ctx);
+        true, librbd::io::DIRECTION_BOTH, ctx);
       return;
     }
 

diff --git a/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc b/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc
index ac43ddf4f3215da3e5520922f201cdbea123e460..a1429c619f1a41ddfa6479c5115a2c58e98a60d7 100644 (file)
--- a/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc
+++ b/src/test/librbd/exclusive_lock/test_mock_PreReleaseRequest.cc
@@ -33,7 +33,8 @@ namespace exclusive_lock {
 
 template <>
 struct ImageDispatch<MockTestImageCtx> {
-  MOCK_METHOD2(set_require_lock, void(io::Direction, Context*));
+  MOCK_METHOD3(set_require_lock, void(bool init_shutdown, io::Direction,
+                                      Context*));
   MOCK_METHOD1(unset_require_lock, void(io::Direction));
 };
 
@@ -106,22 +107,25 @@ public:
   }
 
   void expect_set_require_lock(MockImageDispatch &mock_image_dispatch,
+                               bool init_shutdown,
                                librbd::io::Direction direction, int r) {
-    EXPECT_CALL(mock_image_dispatch, set_require_lock(direction, _))
-      .WillOnce(WithArg<1>(Invoke([r](Context* ctx) { ctx->complete(r); })));
+    EXPECT_CALL(mock_image_dispatch, set_require_lock(init_shutdown,
+                                                      direction, _))
+      .WillOnce(WithArg<2>(Invoke([r](Context* ctx) { ctx->complete(r); })));
   }
 
   void expect_set_require_lock(MockTestImageCtx &mock_image_ctx,
-                               MockImageDispatch &mock_image_dispatch, int r) {
+                               MockImageDispatch &mock_image_dispatch,
+                               bool init_shutdown, int r) {
     expect_test_features(mock_image_ctx, RBD_FEATURE_JOURNALING,
                          ((mock_image_ctx.features & RBD_FEATURE_JOURNALING) != 0));
     if (mock_image_ctx.clone_copy_on_read ||
         (mock_image_ctx.features & RBD_FEATURE_JOURNALING) != 0) {
-      expect_set_require_lock(mock_image_dispatch, librbd::io::DIRECTION_BOTH,
-                              r);
+      expect_set_require_lock(mock_image_dispatch, init_shutdown,
+                              librbd::io::DIRECTION_BOTH, r);
     } else {
-      expect_set_require_lock(mock_image_dispatch, librbd::io::DIRECTION_WRITE,
-                              r);
+      expect_set_require_lock(mock_image_dispatch, init_shutdown,
+                              librbd::io::DIRECTION_WRITE, r);
     }
   }
 
@@ -192,7 +196,7 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, Success) {
   expect_prepare_lock(mock_image_ctx);
   expect_cancel_op_requests(mock_image_ctx, 0);
   MockImageDispatch mock_image_dispatch;
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, 0);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, false, 0);
 
   cache::MockImageCache mock_image_cache;
   mock_image_ctx.image_cache = &mock_image_cache;
@@ -229,7 +233,7 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, SuccessJournalDisabled) {
   MockTestImageCtx mock_image_ctx(*ictx);
 
   MockImageDispatch mock_image_dispatch;
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, 0);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, false, 0);
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
@@ -267,7 +271,7 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, SuccessObjectMapDisabled) {
   MockTestImageCtx mock_image_ctx(*ictx);
 
   MockImageDispatch mock_image_dispatch;
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, 0);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true, 0);
   expect_op_work_queue(mock_image_ctx);
 
   InSequence seq;
@@ -303,7 +307,8 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, Blocklisted) {
   expect_prepare_lock(mock_image_ctx);
   expect_cancel_op_requests(mock_image_ctx, 0);
   MockImageDispatch mock_image_dispatch;
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, -EBLOCKLISTED);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, false,
+                          -EBLOCKLISTED);
 
   cache::MockImageCache mock_image_cache;
   mock_image_ctx.image_cache = &mock_image_cache;
@@ -344,7 +349,7 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, BlockWritesError) {
   InSequence seq;
   expect_cancel_op_requests(mock_image_ctx, 0);
   MockImageDispatch mock_image_dispatch;
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, -EINVAL);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true, -EINVAL);
   expect_unset_require_lock(mock_image_dispatch);
 
   C_SaferCond ctx;
@@ -354,30 +359,5 @@ TEST_F(TestMockExclusiveLockPreReleaseRequest, BlockWritesError) {
   ASSERT_EQ(-EINVAL, ctx.wait());
 }
 
-TEST_F(TestMockExclusiveLockPreReleaseRequest, UnlockError) {
-  REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK);
-
-  librbd::ImageCtx *ictx;
-  ASSERT_EQ(0, open_image(m_image_name, &ictx));
-
-  MockTestImageCtx mock_image_ctx(*ictx);
-
-  expect_op_work_queue(mock_image_ctx);
-
-  InSequence seq;
-  expect_cancel_op_requests(mock_image_ctx, 0);
-  MockImageDispatch mock_image_dispatch;
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, 0);
-  expect_invalidate_cache(mock_image_ctx, 0);
-
-  expect_flush_notifies(mock_image_ctx);
-
-  C_SaferCond ctx;
-  MockPreReleaseRequest *req = MockPreReleaseRequest::create(
-    mock_image_ctx, &mock_image_dispatch, true, m_async_op_tracker, &ctx);
-  req->send();
-  ASSERT_EQ(0, ctx.wait());
-}
-
 } // namespace exclusive_lock
 } // namespace librbd

diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc
index c9bcf4e014c9153cd8ae1a9d49ed39aa808da49e..2912237c16622e61ed731f6faf569042a4892444 100644 (file)
--- a/src/test/librbd/image/test_mock_RefreshRequest.cc
+++ b/src/test/librbd/image/test_mock_RefreshRequest.cc
@@ -157,8 +157,8 @@ public:
 
   void expect_set_require_lock(MockExclusiveLock &mock_exclusive_lock,
                                librbd::io::Direction direction) {
-    EXPECT_CALL(mock_exclusive_lock, set_require_lock(direction, _))
-      .WillOnce(WithArg<1>(Invoke([](Context* ctx) { ctx->complete(0); })));
+    EXPECT_CALL(mock_exclusive_lock, set_require_lock(true, direction, _))
+      .WillOnce(WithArg<2>(Invoke([](Context* ctx) { ctx->complete(0); })));
   }
 
   void expect_unset_require_lock(MockExclusiveLock &mock_exclusive_lock,

diff --git a/src/test/librbd/mock/MockExclusiveLock.h b/src/test/librbd/mock/MockExclusiveLock.h
index 6df6dbb1e2d351773555e9fdabb50986e47edbfa..db675abf0ef4c829beddf3955754b9cca17e317d 100644 (file)
--- a/src/test/librbd/mock/MockExclusiveLock.h
+++ b/src/test/librbd/mock/MockExclusiveLock.h
@@ -35,7 +35,8 @@ struct MockExclusiveLock {
   MOCK_METHOD0(accept_ops, bool());
   MOCK_METHOD0(get_unlocked_op_error, int());
 
-  MOCK_METHOD2(set_require_lock, void(io::Direction, Context*));
+  MOCK_METHOD3(set_require_lock, void(bool init_shutdown, io::Direction,
+                                      Context*));
   MOCK_METHOD1(unset_require_lock, void(io::Direction));
 
   MOCK_METHOD1(start_op, Context*(int*));

diff --git a/src/test/librbd/test_mock_ExclusiveLock.cc b/src/test/librbd/test_mock_ExclusiveLock.cc
index 3fb512e8fbc787c18234d788cab937654714d67e..33f7d975cd39d9a20ce45fdd50a841973636666e 100644 (file)
--- a/src/test/librbd/test_mock_ExclusiveLock.cc
+++ b/src/test/librbd/test_mock_ExclusiveLock.cc
@@ -131,7 +131,7 @@ struct ImageDispatch<MockExclusiveLockImageCtx>
     return io::IMAGE_DISPATCH_LAYER_EXCLUSIVE_LOCK;
   }
 
-  MOCK_METHOD2(set_require_lock, void(io::Direction, Context*));
+  MOCK_METHOD3(set_require_lock, void(bool, io::Direction, Context*));
   MOCK_METHOD1(unset_require_lock, void(io::Direction));
 
 };
@@ -254,19 +254,23 @@ public:
   }
 
   void expect_set_require_lock(MockImageDispatch &mock_image_dispatch,
-                               io::Direction direction) {
-    EXPECT_CALL(mock_image_dispatch, set_require_lock(direction, _))
-      .WillOnce(WithArg<1>(Invoke([](Context* ctx) { ctx->complete(0); })));
+                               bool init_shutdown, io::Direction direction) {
+    EXPECT_CALL(mock_image_dispatch, set_require_lock(init_shutdown,
+                                                      direction, _))
+      .WillOnce(WithArg<2>(Invoke([](Context* ctx) { ctx->complete(0); })));
   }
 
   void expect_set_require_lock(MockExclusiveLockImageCtx &mock_image_ctx,
-                               MockImageDispatch &mock_image_dispatch) {
+                               MockImageDispatch &mock_image_dispatch,
+                               bool init_shutdown) {
     if (mock_image_ctx.clone_copy_on_read ||
         (mock_image_ctx.features & RBD_FEATURE_JOURNALING) != 0 ||
         is_rbd_rwl_enabled(mock_image_ctx.cct)) {
-      expect_set_require_lock(mock_image_dispatch, io::DIRECTION_BOTH);
+      expect_set_require_lock(mock_image_dispatch, init_shutdown,
+                              io::DIRECTION_BOTH);
     } else {
-      expect_set_require_lock(mock_image_dispatch, io::DIRECTION_WRITE);
+      expect_set_require_lock(mock_image_dispatch, init_shutdown,
+                              io::DIRECTION_WRITE);
     }
   }
 
@@ -433,7 +437,7 @@ TEST_F(TestMockExclusiveLock, StateTransitions) {
   MockImageDispatch mock_image_dispatch;
   expect_block_writes(mock_image_ctx, mock_image_dispatch);
   expect_register_dispatch(mock_image_ctx);
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true);
   expect_unblock_writes(mock_image_ctx);
   expect_set_state_unlocked(exclusive_lock);
   ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock));
@@ -504,7 +508,7 @@ TEST_F(TestMockExclusiveLock, TryLockAlreadyLocked) {
   MockImageDispatch mock_image_dispatch;
   expect_block_writes(mock_image_ctx, mock_image_dispatch);
   expect_register_dispatch(mock_image_ctx);
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true);
   expect_unblock_writes(mock_image_ctx);
   expect_set_state_unlocked(exclusive_lock);
   ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock));
@@ -539,7 +543,7 @@ TEST_F(TestMockExclusiveLock, TryLockError) {
   MockImageDispatch mock_image_dispatch;
   expect_block_writes(mock_image_ctx, mock_image_dispatch);
   expect_register_dispatch(mock_image_ctx);
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true);
   expect_unblock_writes(mock_image_ctx);
   expect_set_state_unlocked(exclusive_lock);
   ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock));
@@ -574,7 +578,7 @@ TEST_F(TestMockExclusiveLock, AcquireLockAlreadyLocked) {
   MockImageDispatch mock_image_dispatch;
   expect_block_writes(mock_image_ctx, mock_image_dispatch);
   expect_register_dispatch(mock_image_ctx);
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true);
   expect_unblock_writes(mock_image_ctx);
   expect_set_state_unlocked(exclusive_lock);
   ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock));
@@ -612,7 +616,7 @@ TEST_F(TestMockExclusiveLock, AcquireLockBusy) {
   MockImageDispatch mock_image_dispatch;
   expect_block_writes(mock_image_ctx, mock_image_dispatch);
   expect_register_dispatch(mock_image_ctx);
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true);
   expect_unblock_writes(mock_image_ctx);
   expect_set_state_unlocked(exclusive_lock);
   ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock));
@@ -650,7 +654,7 @@ TEST_F(TestMockExclusiveLock, AcquireLockError) {
   MockImageDispatch mock_image_dispatch;
   expect_block_writes(mock_image_ctx, mock_image_dispatch);
   expect_register_dispatch(mock_image_ctx);
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true);
   expect_unblock_writes(mock_image_ctx);
   expect_set_state_unlocked(exclusive_lock);
   ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock));
@@ -686,7 +690,7 @@ TEST_F(TestMockExclusiveLock, PostAcquireLockError) {
   MockImageDispatch mock_image_dispatch;
   expect_block_writes(mock_image_ctx, mock_image_dispatch);
   expect_register_dispatch(mock_image_ctx);
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true);
   expect_unblock_writes(mock_image_ctx);
   expect_set_state_unlocked(exclusive_lock);
   ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock));
@@ -722,7 +726,7 @@ TEST_F(TestMockExclusiveLock, PreReleaseLockError) {
   MockImageDispatch mock_image_dispatch;
   expect_block_writes(mock_image_ctx, mock_image_dispatch);
   expect_register_dispatch(mock_image_ctx);
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true);
   expect_unblock_writes(mock_image_ctx);
   expect_set_state_unlocked(exclusive_lock);
   ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock));
@@ -756,7 +760,7 @@ TEST_F(TestMockExclusiveLock, ReacquireLock) {
   MockImageDispatch mock_image_dispatch;
   expect_block_writes(mock_image_ctx, mock_image_dispatch);
   expect_register_dispatch(mock_image_ctx);
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true);
   expect_unblock_writes(mock_image_ctx);
   expect_set_state_unlocked(exclusive_lock);
   ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock));
@@ -813,7 +817,7 @@ TEST_F(TestMockExclusiveLock, BlockRequests) {
   MockImageDispatch mock_image_dispatch;
   expect_block_writes(mock_image_ctx, mock_image_dispatch);
   expect_register_dispatch(mock_image_ctx);
-  expect_set_require_lock(mock_image_ctx, mock_image_dispatch);
+  expect_set_require_lock(mock_image_ctx, mock_image_dispatch, true);
   expect_unblock_writes(mock_image_ctx);
   expect_set_state_unlocked(exclusive_lock);
   ASSERT_EQ(0, when_init(mock_image_ctx, exclusive_lock));