From c79d45ff52a636c136e0c5f8aec7911c04601b6f Mon Sep 17 00:00:00 2001 From: Venky Shankar Date: Mon, 20 Feb 2017 12:04:10 +0530 Subject: [PATCH] librbd: acquire exclusive-lock during copy on read Fixes: http://tracker.ceph.com/issues/18888 Signed-off-by: Venky Shankar (cherry picked from commit 7dba5311b12011a4a6e8564e68150e54c5af5ddd) Conflicts: src/librbd/AioImageRequestWQ.h: - in master this file has morphed into src/librbd/io/ImageRequestWQ.h - kraken has AioImageRequest instead of ImageRequest src/librbd/image/RefreshRequest.cc: - rename image context element to "aio_work_queue" (from "io_work_queue") because kraken doesn't have de95d862f57b56738e04d77f2351622f83f17f4a src/test/librbd/image/test_mock_RefreshRequest.cc: - rename image context element to "aio_work_queue" (from "io_work_queue") because kraken doesn't have de95d862f57b56738e04d77f2351622f83f17f4a --- src/librbd/AioImageRequestWQ.h | 2 +- src/librbd/image/RefreshRequest.cc | 5 ++ .../librbd/image/test_mock_RefreshRequest.cc | 57 +++++++++++++++++++ src/test/librbd/mock/MockAioImageRequestWQ.h | 1 + src/test/librbd/mock/MockImageCtx.h | 3 + 5 files changed, 67 insertions(+), 1 deletion(-) diff --git a/src/librbd/AioImageRequestWQ.h b/src/librbd/AioImageRequestWQ.h index 65a70e15cad..146b51e4135 100644 --- a/src/librbd/AioImageRequestWQ.h +++ b/src/librbd/AioImageRequestWQ.h @@ -38,6 +38,7 @@ public: void shut_down(Context *on_shutdown); + bool is_lock_required() const; bool is_lock_request_needed() const; inline bool writes_blocked() const { @@ -109,7 +110,6 @@ private: int start_in_flight_op(AioCompletion *c); void finish_in_flight_op(); - bool is_lock_required() const; void queue(AioImageRequest *req); void handle_refreshed(int r, AioImageRequest *req); diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc index 9884a12bf9c..559e8b2ddab 100644 --- a/src/librbd/image/RefreshRequest.cc +++ b/src/librbd/image/RefreshRequest.cc @@ -1029,6 +1029,7 @@ void RefreshRequest::apply() { // object map and journaling assert(m_exclusive_lock == nullptr); m_exclusive_lock = m_image_ctx.exclusive_lock; + m_image_ctx.aio_work_queue->clear_require_lock_on_read(); } else { if (m_exclusive_lock != nullptr) { assert(m_image_ctx.exclusive_lock == nullptr); @@ -1048,6 +1049,10 @@ void RefreshRequest::apply() { m_object_map != nullptr) { std::swap(m_object_map, m_image_ctx.object_map); } + if (m_image_ctx.clone_copy_on_read && + m_image_ctx.aio_work_queue->is_lock_required()) { + m_image_ctx.aio_work_queue->set_require_lock_on_read(); + } } } } diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc index 809dbddfa4c..b069e29f7df 100644 --- a/src/test/librbd/image/test_mock_RefreshRequest.cc +++ b/src/test/librbd/image/test_mock_RefreshRequest.cc @@ -97,6 +97,10 @@ public: typedef RefreshRequest MockRefreshRequest; typedef RefreshParentRequest MockRefreshParentRequest; + void expect_is_lock_required(MockRefreshImageCtx &mock_image_ctx, bool require_lock) { + EXPECT_CALL(*mock_image_ctx.aio_work_queue, is_lock_required()).WillOnce(Return(require_lock)); + } + void expect_set_require_lock_on_read(MockRefreshImageCtx &mock_image_ctx) { EXPECT_CALL(*mock_image_ctx.aio_work_queue, set_require_lock_on_read()); } @@ -619,6 +623,7 @@ TEST_F(TestMockImageRefreshRequest, DisableExclusiveLock) { expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); + expect_clear_require_lock_on_read(mock_image_ctx); expect_shut_down_exclusive_lock(mock_image_ctx, *mock_exclusive_lock, 0); C_SaferCond ctx; @@ -723,6 +728,58 @@ TEST_F(TestMockImageRefreshRequest, JournalDisabledByPolicy) { ASSERT_EQ(0, ctx.wait()); } +TEST_F(TestMockImageRefreshRequest, ExclusiveLockWithCoR) { + REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK); + + std::string val; + ASSERT_EQ(0, _rados.conf_get("rbd_clone_copy_on_read", val)); + if (val == "false") { + std::cout << "SKIPPING due to disabled rbd_copy_on_read" << std::endl; + return; + } + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockRefreshImageCtx mock_image_ctx(*ictx); + MockRefreshParentRequest mock_refresh_parent_request; + + MockExclusiveLock mock_exclusive_lock; + mock_image_ctx.exclusive_lock = &mock_exclusive_lock; + + if (ictx->test_features(RBD_FEATURE_JOURNALING)) { + ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_JOURNALING, + false)); + } + + if (ictx->test_features(RBD_FEATURE_FAST_DIFF)) { + ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_FAST_DIFF, + false)); + } + + if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) { + ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_OBJECT_MAP, + false)); + } + + expect_op_work_queue(mock_image_ctx); + expect_test_features(mock_image_ctx); + + InSequence seq; + expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_flags(mock_image_ctx, 0); + expect_get_group(mock_image_ctx, 0); + expect_refresh_parent_is_required(mock_refresh_parent_request, false); + expect_is_lock_required(mock_image_ctx, true); + expect_set_require_lock_on_read(mock_image_ctx); + + C_SaferCond ctx; + MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, false, false, &ctx); + req->send(); + + ASSERT_EQ(0, ctx.wait()); +} + TEST_F(TestMockImageRefreshRequest, EnableJournalWithExclusiveLock) { REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); diff --git a/src/test/librbd/mock/MockAioImageRequestWQ.h b/src/test/librbd/mock/MockAioImageRequestWQ.h index 77419c04d8f..2b1cefee405 100644 --- a/src/test/librbd/mock/MockAioImageRequestWQ.h +++ b/src/test/librbd/mock/MockAioImageRequestWQ.h @@ -17,6 +17,7 @@ struct MockAioImageRequestWQ { MOCK_METHOD0(set_require_lock_on_read, void()); MOCK_METHOD0(clear_require_lock_on_read, void()); + MOCK_CONST_METHOD0(is_lock_required, bool()); MOCK_CONST_METHOD0(is_lock_request_needed, bool()); }; diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h index 943f0e87488..285aecd1eee 100644 --- a/src/test/librbd/mock/MockImageCtx.h +++ b/src/test/librbd/mock/MockImageCtx.h @@ -51,6 +51,7 @@ struct MockImageCtx { object_set(image_ctx.object_set), old_format(image_ctx.old_format), read_only(image_ctx.read_only), + clone_copy_on_read(image_ctx.clone_copy_on_read), lockers(image_ctx.lockers), exclusive_locked(image_ctx.exclusive_locked), lock_tag(image_ctx.lock_tag), @@ -207,6 +208,8 @@ struct MockImageCtx { bool old_format; bool read_only; + bool clone_copy_on_read; + std::map lockers; bool exclusive_locked; -- 2.47.3