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 <dillaman@redhat.com>

diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc
index 0db4bb3da6c00b731cdff11d82638ab7579f7348..bd6e081d4df2ba2345223cce48774c07baac0ab3 100644 (file)
--- 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 <typename I, typename E>
+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 <typename I>
@@ -254,8 +282,10 @@ void Replay<I>::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<I, journal::SnapCreateEvent>(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<I>::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<I, journal::ResizeEvent>(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 d3e8bf6ed89feb1d1e49711e4a45c334b6aef15b..216da1a983aef208262c70ab74f0350fc23cd480 100644 (file)
--- 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<I>::commit_op_event(int r) {
   }
 }
 
+template <typename I>
+void Request<I>::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 <typename I>
 void Request<I>::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 44ff5e2eab21e60d6988c4b1bbca6cfe3790d195..be4d174be5d710153ef32e48232f927d49cd4587 100644 (file)
--- 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<T, MF>(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 be121cff90a95ecfd940381c39fe9eb71ec96f3e..93aeddcbd43732e13607e4f11a9661d1cba94fd8 100644 (file)
--- 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, &current);
   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, &current);
   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, &current);
   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, &current);
+  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, &current);
+  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, &current);
+  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, &current);
+  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, &current);
+  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, &current);
+  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, &current);
+  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, &current);
+  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, &current);
+  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) {