Skip to content

Commit

Permalink
librbd: acquire exclusive-lock during copy on read
Browse files Browse the repository at this point in the history
Fixes: http://tracker.ceph.com/issues/18888
Signed-off-by: Venky Shankar <vshankar@redhat.com>
  • Loading branch information
vshankar committed Feb 23, 2017
1 parent c733d48 commit 7dba531
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 1 deletion.
5 changes: 5 additions & 0 deletions src/librbd/image/RefreshRequest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,7 @@ void RefreshRequest<I>::apply() {
// object map and journaling
assert(m_exclusive_lock == nullptr);
m_exclusive_lock = m_image_ctx.exclusive_lock;
m_image_ctx.io_work_queue->clear_require_lock_on_read();
} else {
if (m_exclusive_lock != nullptr) {
assert(m_image_ctx.exclusive_lock == nullptr);
Expand All @@ -1094,6 +1095,10 @@ void RefreshRequest<I>::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.io_work_queue->is_lock_required()) {
m_image_ctx.io_work_queue->set_require_lock_on_read();
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librbd/io/ImageRequestWQ.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class ImageRequestWQ : protected ThreadPool::PointerWQ<ImageRequest<ImageCtx> >

void shut_down(Context *on_shutdown);

bool is_lock_required() const;
bool is_lock_request_needed() const;

inline bool writes_blocked() const {
Expand Down Expand Up @@ -114,7 +115,6 @@ class ImageRequestWQ : protected ThreadPool::PointerWQ<ImageRequest<ImageCtx> >
int start_in_flight_op(AioCompletion *c);
void finish_in_flight_op();

bool is_lock_required() const;
void queue(ImageRequest<ImageCtx> *req);

void handle_refreshed(int r, ImageRequest<ImageCtx> *req);
Expand Down
57 changes: 57 additions & 0 deletions src/test/librbd/image/test_mock_RefreshRequest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ class TestMockImageRefreshRequest : public TestMockFixture {
typedef RefreshRequest<MockRefreshImageCtx> MockRefreshRequest;
typedef RefreshParentRequest<MockRefreshImageCtx> MockRefreshParentRequest;

void expect_is_lock_required(MockRefreshImageCtx &mock_image_ctx, bool require_lock) {
EXPECT_CALL(*mock_image_ctx.io_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.io_work_queue, set_require_lock_on_read());
}
Expand Down Expand Up @@ -631,6 +635,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;
Expand Down Expand Up @@ -735,6 +740,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);

Expand Down
3 changes: 3 additions & 0 deletions src/test/librbd/mock/MockImageCtx.h
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -208,6 +209,8 @@ struct MockImageCtx {
bool old_format;
bool read_only;

bool clone_copy_on_read;

std::map<rados::cls::lock::locker_id_t,
rados::cls::lock::locker_info_t> lockers;
bool exclusive_locked;
Expand Down
1 change: 1 addition & 0 deletions src/test/librbd/mock/io/MockImageRequestWQ.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ struct MockImageRequestWQ {
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());
};

Expand Down

0 comments on commit 7dba531

Please sign in to comment.