From e01b406762d1ac27a2912a5d5da663633d3512ec Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Tue, 16 Feb 2016 09:07:57 -0500 Subject: [PATCH] librbd: journal replay should execute ops in clean context lockdep will complain about loop cycles that won't cause an issue in reality as replay and record are two different journal states. Signed-off-by: Jason Dillaman --- src/librbd/journal/Replay.cc | 40 ++- src/librbd/operation/Request.cc | 13 + src/librbd/operation/Request.h | 5 +- src/test/librbd/journal/test_Replay.cc | 340 +++++++++++++++++++++++++ 4 files changed, 391 insertions(+), 7 deletions(-) diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc index 0db4bb3da6c0..bd6e081d4df2 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -8,6 +8,8 @@ #include "librbd/AioCompletion.h" #include "librbd/AioImageRequest.h" #include "librbd/ImageCtx.h" +#include "librbd/ImageState.h" +#include "librbd/ImageWatcher.h" #include "librbd/internal.h" #include "librbd/Operations.h" #include "librbd/Utils.h" @@ -26,6 +28,32 @@ static const uint64_t IN_FLIGHT_IO_HIGH_WATER_MARK(64); static NoOpProgressContext no_op_progress_callback; +template +struct ExecuteOp : public Context { + I &image_ctx; + E event; + Context *on_op_complete; + + ExecuteOp(I &image_ctx, const E &event, Context *on_op_complete) + : image_ctx(image_ctx), event(event), on_op_complete(on_op_complete) { + } + + void execute(const journal::SnapCreateEvent &_) { + image_ctx.operations->snap_create(event.snap_name.c_str(), on_op_complete, + event.op_tid); + } + + void execute(const journal::ResizeEvent &_) { + image_ctx.operations->resize(event.size, no_op_progress_callback, + on_op_complete, event.op_tid); + } + + virtual void finish(int r) override { + RWLock::RLocker owner_locker(image_ctx.owner_lock); + execute(event); + } +}; + } // anonymous namespace template @@ -254,8 +282,10 @@ void Replay::handle_event(const journal::SnapCreateEvent &event, Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe, &op_event); - m_image_ctx.operations->snap_create(event.snap_name.c_str(), on_op_complete, - event.op_tid); + // avoid lock cycles + m_image_ctx.op_work_queue->queue( + new ExecuteOp(m_image_ctx, event, + on_op_complete), 0); // do not process more events until the state machine is ready // since it will affect IO @@ -391,8 +421,10 @@ void Replay::handle_event(const journal::ResizeEvent &event, Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe, &op_event); - m_image_ctx.operations->resize(event.size, no_op_progress_callback, - on_op_complete, event.op_tid); + // avoid lock cycles + m_image_ctx.op_work_queue->queue( + new ExecuteOp(m_image_ctx, event, + on_op_complete), 0); // do not process more events until the state machine is ready // since it will affect IO diff --git a/src/librbd/operation/Request.cc b/src/librbd/operation/Request.cc index d3e8bf6ed89f..216da1a983ae 100644 --- a/src/librbd/operation/Request.cc +++ b/src/librbd/operation/Request.cc @@ -4,6 +4,7 @@ #include "librbd/operation/Request.h" #include "common/dout.h" #include "common/errno.h" +#include "common/WorkQueue.h" #include "librbd/ImageCtx.h" #include "librbd/Journal.h" #include "librbd/Utils.h" @@ -83,6 +84,18 @@ void Request::commit_op_event(int r) { } } +template +void Request::replay_op_ready(Context *on_safe) { + I &image_ctx = this->m_image_ctx; + assert(image_ctx.owner_lock.is_locked()); + assert(image_ctx.snap_lock.is_locked()); + assert(m_op_tid != 0); + + m_appended_op_event = true; + image_ctx.journal->replay_op_ready( + m_op_tid, util::create_async_context_callback(image_ctx, on_safe)); +} + template void Request::append_op_event(Context *on_safe) { I &image_ctx = this->m_image_ctx; diff --git a/src/librbd/operation/Request.h b/src/librbd/operation/Request.h index 44ff5e2eab21..be4d174be5d7 100644 --- a/src/librbd/operation/Request.h +++ b/src/librbd/operation/Request.h @@ -44,9 +44,7 @@ protected: if (image_ctx.journal != NULL) { Context *ctx = util::create_context_callback(request); if (image_ctx.journal->is_journal_replaying()) { - assert(m_op_tid != 0); - m_appended_op_event = true; - image_ctx.journal->replay_op_ready(m_op_tid, ctx); + replay_op_ready(ctx); } else { append_op_event(ctx); } @@ -83,6 +81,7 @@ private: bool m_appended_op_event = false; bool m_committed_op_event = false; + void replay_op_ready(Context *on_safe); void append_op_event(Context *on_safe); void handle_op_event_safe(int r); diff --git a/src/test/librbd/journal/test_Replay.cc b/src/test/librbd/journal/test_Replay.cc index be121cff90a9..93aeddcbd437 100644 --- a/src/test/librbd/journal/test_Replay.cc +++ b/src/test/librbd/journal/test_Replay.cc @@ -12,7 +12,9 @@ #include "librbd/ExclusiveLock.h" #include "librbd/ImageCtx.h" #include "librbd/ImageWatcher.h" +#include "librbd/internal.h" #include "librbd/Journal.h" +#include "librbd/Operations.h" #include "librbd/journal/Types.h" void register_test_journal_replay() { @@ -153,6 +155,12 @@ TEST_F(TestJournalReplay, AioDiscardEvent) { ASSERT_EQ(0, when_acquired_lock(ictx)); get_journal_commit_position(ictx, ¤t); ASSERT_EQ(current, initial + 3); + + // verify lock ordering constraints + aio_comp = new librbd::AioCompletion(); + ictx->aio_work_queue->aio_discard(aio_comp, 0, read_payload.size()); + ASSERT_EQ(0, aio_comp->wait_for_complete()); + aio_comp->release(); } TEST_F(TestJournalReplay, AioWriteEvent) { @@ -199,6 +207,13 @@ TEST_F(TestJournalReplay, AioWriteEvent) { ASSERT_EQ(0, when_acquired_lock(ictx)); get_journal_commit_position(ictx, ¤t); ASSERT_EQ(current, initial + 3); + + // verify lock ordering constraints + aio_comp = new librbd::AioCompletion(); + ictx->aio_work_queue->aio_write(aio_comp, 0, payload.size(), payload.c_str(), + 0); + ASSERT_EQ(0, aio_comp->wait_for_complete()); + aio_comp->release(); } TEST_F(TestJournalReplay, AioFlushEvent) { @@ -257,6 +272,331 @@ TEST_F(TestJournalReplay, AioFlushEvent) { ASSERT_EQ(0, when_acquired_lock(ictx)); get_journal_commit_position(ictx, ¤t); ASSERT_EQ(current, initial + 3); + + // verify lock ordering constraints + aio_comp = new librbd::AioCompletion(); + ictx->aio_work_queue->aio_flush(aio_comp); + ASSERT_EQ(0, aio_comp->wait_for_complete()); + aio_comp->release(); +} + +TEST_F(TestJournalReplay, SnapCreate) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::SnapCreateEvent(1, "snap")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + { + RWLock::RLocker snap_locker(ictx->snap_lock); + ASSERT_NE(CEPH_NOSNAP, ictx->get_snap_id("snap")); + } + + // verify lock ordering constraints + ASSERT_EQ(0, ictx->operations->snap_create("snap2")); +} + +TEST_F(TestJournalReplay, SnapProtect) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::SnapProtectEvent(1, "snap")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + bool is_protected; + ASSERT_EQ(0, librbd::snap_is_protected(ictx, "snap", &is_protected)); + ASSERT_TRUE(is_protected); + + // verify lock ordering constraints + ASSERT_EQ(0, ictx->operations->snap_create("snap2")); + ASSERT_EQ(0, ictx->operations->snap_protect("snap2")); +} + +TEST_F(TestJournalReplay, SnapUnprotect) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + uint64_t snap_id; + { + RWLock::RLocker snap_locker(ictx->snap_lock); + snap_id = ictx->get_snap_id("snap"); + ASSERT_NE(CEPH_NOSNAP, snap_id); + } + ASSERT_EQ(0, ictx->operations->snap_protect("snap")); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::SnapUnprotectEvent(1, "snap")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + bool is_protected; + ASSERT_EQ(0, librbd::snap_is_protected(ictx, "snap", &is_protected)); + ASSERT_FALSE(is_protected); + + // verify lock ordering constraints + ASSERT_EQ(0, ictx->operations->snap_create("snap2")); + ASSERT_EQ(0, ictx->operations->snap_protect("snap2")); + ASSERT_EQ(0, ictx->operations->snap_unprotect("snap2")); +} + +TEST_F(TestJournalReplay, SnapRename) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + uint64_t snap_id; + { + RWLock::RLocker snap_locker(ictx->snap_lock); + snap_id = ictx->get_snap_id("snap"); + ASSERT_NE(CEPH_NOSNAP, snap_id); + } + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::SnapRenameEvent(1, snap_id, "snap2")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + { + RWLock::RLocker snap_locker(ictx->snap_lock); + snap_id = ictx->get_snap_id("snap2"); + ASSERT_NE(CEPH_NOSNAP, snap_id); + } + + // verify lock ordering constraints + ASSERT_EQ(0, ictx->operations->snap_rename("snap2", "snap3")); +} + +TEST_F(TestJournalReplay, SnapRollback) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::SnapRollbackEvent(1, "snap")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + // verify lock ordering constraints + librbd::NoOpProgressContext no_op_progress; + ASSERT_EQ(0, ictx->operations->snap_rollback("snap", no_op_progress)); +} + +TEST_F(TestJournalReplay, SnapRemove) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::SnapRemoveEvent(1, "snap")); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + { + RWLock::RLocker snap_locker(ictx->snap_lock); + uint64_t snap_id = ictx->get_snap_id("snap"); + ASSERT_EQ(CEPH_NOSNAP, snap_id); + } + + // verify lock ordering constraints + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + ASSERT_EQ(0, ictx->operations->snap_remove("snap")); +} + +TEST_F(TestJournalReplay, Rename) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + std::string new_image_name(get_temp_image_name()); + inject_into_journal(ictx, librbd::journal::RenameEvent(1, new_image_name)); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + // verify lock ordering constraints + librbd::RBD rbd; + ASSERT_EQ(0, rbd.rename(m_ioctx, new_image_name.c_str(), m_image_name.c_str())); +} + +TEST_F(TestJournalReplay, Resize) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx, librbd::journal::ResizeEvent(1, 16)); + inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, when_acquired_lock(ictx)); + + int64_t current; + get_journal_commit_position(ictx, ¤t); + ASSERT_EQ(current, initial + 2); + + // verify lock ordering constraints + librbd::NoOpProgressContext no_op_progress; + ASSERT_EQ(0, ictx->operations->resize(0, no_op_progress)); +} + +TEST_F(TestJournalReplay, Flatten) { + REQUIRE_FEATURE(RBD_FEATURE_LAYERING | RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, ictx->operations->snap_create("snap")); + ASSERT_EQ(0, ictx->operations->snap_protect("snap")); + + std::string clone_name = get_temp_image_name(); + int order = ictx->order; + ASSERT_EQ(0, librbd::clone(m_ioctx, m_image_name.c_str(), "snap", m_ioctx, + clone_name.c_str(), ictx->features, &order, 0, 0)); + + librbd::ImageCtx *ictx2; + ASSERT_EQ(0, open_image(clone_name, &ictx2)); + ASSERT_EQ(0, when_acquired_lock(ictx2)); + + // get current commit position + int64_t initial; + get_journal_commit_position(ictx2, &initial); + + // inject snapshot ops into journal + inject_into_journal(ictx2, librbd::journal::FlattenEvent(1)); + inject_into_journal(ictx2, librbd::journal::OpFinishEvent(1, 0)); + + // replay journal + ASSERT_EQ(0, open_image(clone_name, &ictx2)); + ASSERT_EQ(0, when_acquired_lock(ictx2)); + + int64_t current; + get_journal_commit_position(ictx2, ¤t); + ASSERT_EQ(current, initial + 2); + ASSERT_EQ(0, ictx->operations->snap_unprotect("snap")); + + // verify lock ordering constraints + librbd::NoOpProgressContext no_op; + ASSERT_EQ(-EINVAL, ictx2->operations->flatten(no_op)); } TEST_F(TestJournalReplay, EntryPosition) { -- 2.47.3