From: Jason Dillaman Date: Thu, 10 Mar 2016 16:51:19 +0000 (-0500) Subject: librbd: replaying a journal op post-refresh requires locking X-Git-Tag: v10.1.0~133^2~1^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=aa6a8d3a8b9c433c9d1db27e9face22eb1d2d2fa;p=ceph.git librbd: replaying a journal op post-refresh requires locking Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc index 501fb3d97b18..4e5c95a6b583 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -43,11 +43,43 @@ struct ExecuteOp : public Context { event.op_tid); } + void execute(const journal::SnapRemoveEvent &_) { + image_ctx.operations->snap_remove(event.snap_name.c_str(), on_op_complete); + } + + void execute(const journal::SnapRenameEvent &_) { + image_ctx.operations->snap_rename(event.snap_id, event.snap_name.c_str(), + on_op_complete); + } + + void execute(const journal::SnapProtectEvent &_) { + image_ctx.operations->snap_protect(event.snap_name.c_str(), on_op_complete); + } + + void execute(const journal::SnapUnprotectEvent &_) { + image_ctx.operations->snap_unprotect(event.snap_name.c_str(), + on_op_complete); + } + + void execute(const journal::SnapRollbackEvent &_) { + image_ctx.operations->snap_rollback(event.snap_name.c_str(), + no_op_progress_callback, + on_op_complete); + } + + void execute(const journal::RenameEvent &_) { + image_ctx.operations->rename(event.image_name.c_str(), on_op_complete); + } + void execute(const journal::ResizeEvent &_) { image_ctx.operations->resize(event.size, no_op_progress_callback, on_op_complete, event.op_tid); } + void execute(const journal::FlattenEvent &_) { + image_ctx.operations->flatten(no_op_progress_callback, on_op_complete); + } + virtual void finish(int r) override { RWLock::RLocker owner_locker(image_ctx.owner_lock); execute(event); @@ -69,7 +101,7 @@ struct C_RefreshIfRequired : public Context { return; } - on_finish->complete(0); + image_ctx.op_work_queue->queue(on_finish, 0); } }; @@ -364,11 +396,8 @@ void Replay::handle_event(const journal::SnapRemoveEvent &event, Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe, &op_event); op_event->on_op_finish_event = new C_RefreshIfRequired( - m_image_ctx, new FunctionContext( - [this, event, on_op_complete](int r) { - m_image_ctx.operations->snap_remove(event.snap_name.c_str(), - on_op_complete); - })); + m_image_ctx, new ExecuteOp(m_image_ctx, event, + on_op_complete)); // ignore errors caused due to replay op_event->ignore_error_codes = {-ENOENT}; @@ -387,12 +416,8 @@ void Replay::handle_event(const journal::SnapRenameEvent &event, Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe, &op_event); op_event->on_op_finish_event = new C_RefreshIfRequired( - m_image_ctx, new FunctionContext( - [this, event, on_op_complete](int r) { - m_image_ctx.operations->snap_rename(event.snap_id, - event.snap_name.c_str(), - on_op_complete); - })); + m_image_ctx, new ExecuteOp(m_image_ctx, event, + on_op_complete)); // ignore errors caused due to replay op_event->ignore_error_codes = {-EEXIST}; @@ -411,11 +436,8 @@ void Replay::handle_event(const journal::SnapProtectEvent &event, Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe, &op_event); op_event->on_op_finish_event = new C_RefreshIfRequired( - m_image_ctx, new FunctionContext( - [this, event, on_op_complete](int r) { - m_image_ctx.operations->snap_protect(event.snap_name.c_str(), - on_op_complete); - })); + m_image_ctx, new ExecuteOp(m_image_ctx, event, + on_op_complete)); // ignore errors caused due to replay op_event->ignore_error_codes = {-EBUSY}; @@ -435,11 +457,9 @@ void Replay::handle_event(const journal::SnapUnprotectEvent &event, Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe, &op_event); op_event->on_op_finish_event = new C_RefreshIfRequired( - m_image_ctx, new FunctionContext( - [this, event, on_op_complete](int r) { - m_image_ctx.operations->snap_unprotect(event.snap_name.c_str(), - on_op_complete); - })); + m_image_ctx, new ExecuteOp(m_image_ctx, + event, + on_op_complete)); // ignore errors caused due to replay op_event->ignore_error_codes = {-EINVAL}; @@ -459,12 +479,9 @@ void Replay::handle_event(const journal::SnapRollbackEvent &event, Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe, &op_event); op_event->on_op_finish_event = new C_RefreshIfRequired( - m_image_ctx, new FunctionContext( - [this, event, on_op_complete](int r) { - m_image_ctx.operations->snap_rollback(event.snap_name.c_str(), - no_op_progress_callback, - on_op_complete); - })); + m_image_ctx, new ExecuteOp(m_image_ctx, + event, + on_op_complete)); on_ready->complete(0); } @@ -480,11 +497,8 @@ void Replay::handle_event(const journal::RenameEvent &event, Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe, &op_event); op_event->on_op_finish_event = new C_RefreshIfRequired( - m_image_ctx, new FunctionContext( - [this, event, on_op_complete](int r) { - m_image_ctx.operations->rename(event.image_name.c_str(), - on_op_complete); - })); + m_image_ctx, new ExecuteOp(m_image_ctx, event, + on_op_complete)); // ignore errors caused due to replay op_event->ignore_error_codes = {-EEXIST}; @@ -525,11 +539,8 @@ void Replay::handle_event(const journal::FlattenEvent &event, Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe, &op_event); op_event->on_op_finish_event = new C_RefreshIfRequired( - m_image_ctx, new FunctionContext( - [this, event, on_op_complete](int r) { - m_image_ctx.operations->flatten(no_op_progress_callback, - on_op_complete); - })); + m_image_ctx, new ExecuteOp(m_image_ctx, event, + on_op_complete)); // ignore errors caused due to replay op_event->ignore_error_codes = {-EINVAL}; diff --git a/src/test/librbd/journal/test_mock_Replay.cc b/src/test/librbd/journal/test_mock_Replay.cc index ebb3123490d7..40dc9abca0cf 100644 --- a/src/test/librbd/journal/test_mock_Replay.cc +++ b/src/test/librbd/journal/test_mock_Replay.cc @@ -514,7 +514,7 @@ TEST_F(TestMockJournalReplay, BlockedOpFinishError) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_snap_create(mock_image_ctx, &on_finish, "snap", 123); @@ -533,7 +533,7 @@ TEST_F(TestMockJournalReplay, BlockedOpFinishError) { &on_finish_ready, &on_finish_safe); ASSERT_EQ(-EBADMSG, on_resume.wait()); - on_finish->complete(-ESTALE); + wait_for_op_invoked(&on_finish, -ESTALE); ASSERT_EQ(-ESTALE, on_start_safe.wait()); ASSERT_EQ(-ESTALE, on_finish_safe.wait()); @@ -548,13 +548,14 @@ TEST_F(TestMockJournalReplay, MissingOpFinishEvent) { MockJournalReplay mock_journal_replay(mock_image_ctx); expect_op_work_queue(mock_image_ctx); + EXPECT_CALL(*mock_image_ctx.state, is_refresh_required()) + .WillRepeatedly(Return(false)); + InSequence seq; Context *on_snap_create_finish = nullptr; - expect_refresh_image(mock_image_ctx, false, 0); expect_snap_create(mock_image_ctx, &on_snap_create_finish, "snap", 123); Context *on_snap_remove_finish = nullptr; - expect_refresh_image(mock_image_ctx, false, 0); expect_snap_remove(mock_image_ctx, &on_snap_remove_finish, "snap"); C_SaferCond on_snap_remove_ready; @@ -578,7 +579,7 @@ TEST_F(TestMockJournalReplay, MissingOpFinishEvent) { when_replay_op_ready(mock_journal_replay, 123, &on_snap_create_resume); ASSERT_EQ(0, on_snap_create_resume.wait()); - on_snap_create_finish->complete(0); + wait_for_op_invoked(&on_snap_create_finish, 0); ASSERT_EQ(0, on_snap_create_ready.wait()); ASSERT_EQ(0, on_snap_create_safe.wait()); @@ -647,7 +648,7 @@ TEST_F(TestMockJournalReplay, OpEventError) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_snap_remove(mock_image_ctx, &on_finish, "snap"); @@ -662,7 +663,7 @@ TEST_F(TestMockJournalReplay, OpEventError) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(-EINVAL); + wait_for_op_invoked(&on_finish, -EINVAL); ASSERT_EQ(-EINVAL, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(-EINVAL, on_finish_safe.wait()); @@ -679,7 +680,7 @@ TEST_F(TestMockJournalReplay, SnapCreateEvent) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_snap_create(mock_image_ctx, &on_finish, "snap", 123); @@ -698,7 +699,7 @@ TEST_F(TestMockJournalReplay, SnapCreateEvent) { &on_finish_ready, &on_finish_safe); ASSERT_EQ(0, on_resume.wait()); - on_finish->complete(0); + wait_for_op_invoked(&on_finish, 0); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); @@ -749,7 +750,7 @@ TEST_F(TestMockJournalReplay, SnapRemoveEvent) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_snap_remove(mock_image_ctx, &on_finish, "snap"); @@ -764,7 +765,7 @@ TEST_F(TestMockJournalReplay, SnapRemoveEvent) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(0); + wait_for_op_invoked(&on_finish, 0); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(0, on_finish_safe.wait()); @@ -781,7 +782,7 @@ TEST_F(TestMockJournalReplay, SnapRemoveEventDNE) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_snap_remove(mock_image_ctx, &on_finish, "snap"); @@ -796,7 +797,7 @@ TEST_F(TestMockJournalReplay, SnapRemoveEventDNE) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(-ENOENT); + wait_for_op_invoked(&on_finish, -ENOENT); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(0, on_finish_safe.wait()); @@ -813,7 +814,7 @@ TEST_F(TestMockJournalReplay, SnapRenameEvent) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_snap_rename(mock_image_ctx, &on_finish, 234, "snap"); @@ -829,7 +830,7 @@ TEST_F(TestMockJournalReplay, SnapRenameEvent) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(0); + wait_for_op_invoked(&on_finish, 0); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(0, on_finish_safe.wait()); @@ -846,7 +847,7 @@ TEST_F(TestMockJournalReplay, SnapRenameEventExists) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_snap_rename(mock_image_ctx, &on_finish, 234, "snap"); @@ -862,7 +863,7 @@ TEST_F(TestMockJournalReplay, SnapRenameEventExists) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(-EEXIST); + wait_for_op_invoked(&on_finish, -EEXIST); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(0, on_finish_safe.wait()); @@ -879,7 +880,7 @@ TEST_F(TestMockJournalReplay, SnapProtectEvent) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_snap_protect(mock_image_ctx, &on_finish, "snap"); @@ -894,7 +895,7 @@ TEST_F(TestMockJournalReplay, SnapProtectEvent) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(0); + wait_for_op_invoked(&on_finish, 0); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(0, on_finish_safe.wait()); @@ -911,7 +912,7 @@ TEST_F(TestMockJournalReplay, SnapProtectEventBusy) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_snap_protect(mock_image_ctx, &on_finish, "snap"); @@ -926,7 +927,7 @@ TEST_F(TestMockJournalReplay, SnapProtectEventBusy) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(-EBUSY); + wait_for_op_invoked(&on_finish, -EBUSY); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(0, on_finish_safe.wait()); @@ -943,7 +944,7 @@ TEST_F(TestMockJournalReplay, SnapUnprotectEvent) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_snap_unprotect(mock_image_ctx, &on_finish, "snap"); @@ -958,7 +959,7 @@ TEST_F(TestMockJournalReplay, SnapUnprotectEvent) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(0); + wait_for_op_invoked(&on_finish, 0); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(0, on_finish_safe.wait()); @@ -975,7 +976,7 @@ TEST_F(TestMockJournalReplay, SnapUnprotectEventInvalid) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_snap_unprotect(mock_image_ctx, &on_finish, "snap"); @@ -990,7 +991,7 @@ TEST_F(TestMockJournalReplay, SnapUnprotectEventInvalid) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(-EINVAL); + wait_for_op_invoked(&on_finish, -EINVAL); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(0, on_finish_safe.wait()); @@ -1007,7 +1008,7 @@ TEST_F(TestMockJournalReplay, SnapRollbackEvent) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_snap_rollback(mock_image_ctx, &on_finish, "snap"); @@ -1022,7 +1023,7 @@ TEST_F(TestMockJournalReplay, SnapRollbackEvent) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(0); + wait_for_op_invoked(&on_finish, 0); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(0, on_finish_safe.wait()); @@ -1039,7 +1040,7 @@ TEST_F(TestMockJournalReplay, RenameEvent) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_rename(mock_image_ctx, &on_finish, "image"); @@ -1054,7 +1055,7 @@ TEST_F(TestMockJournalReplay, RenameEvent) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(0); + wait_for_op_invoked(&on_finish, 0); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(0, on_finish_safe.wait()); @@ -1071,7 +1072,7 @@ TEST_F(TestMockJournalReplay, RenameEventExists) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_rename(mock_image_ctx, &on_finish, "image"); @@ -1086,7 +1087,7 @@ TEST_F(TestMockJournalReplay, RenameEventExists) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(-EEXIST); + wait_for_op_invoked(&on_finish, -EEXIST); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(0, on_finish_safe.wait()); @@ -1103,7 +1104,7 @@ TEST_F(TestMockJournalReplay, ResizeEvent) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_resize(mock_image_ctx, &on_finish, 234, 123); @@ -1122,7 +1123,7 @@ TEST_F(TestMockJournalReplay, ResizeEvent) { &on_finish_ready, &on_finish_safe); ASSERT_EQ(0, on_resume.wait()); - on_finish->complete(0); + wait_for_op_invoked(&on_finish, 0); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); @@ -1140,7 +1141,7 @@ TEST_F(TestMockJournalReplay, FlattenEvent) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_flatten(mock_image_ctx, &on_finish); @@ -1155,7 +1156,7 @@ TEST_F(TestMockJournalReplay, FlattenEvent) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(0); + wait_for_op_invoked(&on_finish, 0); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(0, on_finish_safe.wait()); @@ -1172,7 +1173,7 @@ TEST_F(TestMockJournalReplay, FlattenEventInvalid) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, false, 0); expect_flatten(mock_image_ctx, &on_finish); @@ -1187,7 +1188,7 @@ TEST_F(TestMockJournalReplay, FlattenEventInvalid) { when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)}, &on_finish_ready, &on_finish_safe); - on_finish->complete(-EINVAL); + wait_for_op_invoked(&on_finish, -EINVAL); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait()); ASSERT_EQ(0, on_finish_safe.wait()); @@ -1230,7 +1231,7 @@ TEST_F(TestMockJournalReplay, RefreshImageBeforeOpStart) { expect_op_work_queue(mock_image_ctx); InSequence seq; - Context *on_finish; + Context *on_finish = nullptr; expect_refresh_image(mock_image_ctx, true, 0); expect_resize(mock_image_ctx, &on_finish, 234, 123); @@ -1249,7 +1250,7 @@ TEST_F(TestMockJournalReplay, RefreshImageBeforeOpStart) { &on_finish_ready, &on_finish_safe); ASSERT_EQ(0, on_resume.wait()); - on_finish->complete(0); + wait_for_op_invoked(&on_finish, 0); ASSERT_EQ(0, on_start_safe.wait()); ASSERT_EQ(0, on_finish_ready.wait());