librbd: refresh image if required before replaying journal ops

Fixes: #14908
Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc
index b65d51c39c14900bfd5f7971d7b3e241719a581a..501fb3d97b18d487db7e0798b39a3c673c0b62a5 100644 (file)
--- a/src/librbd/journal/Replay.cc
+++ b/src/librbd/journal/Replay.cc
@@ -54,6 +54,25 @@ struct ExecuteOp : public Context {
   }
 };
 
+template <typename I>
+struct C_RefreshIfRequired : public Context {
+  I &image_ctx;
+  Context *on_finish;
+
+  C_RefreshIfRequired(I &image_ctx, Context *on_finish)
+    : image_ctx(image_ctx), on_finish(on_finish) {
+  }
+
+  virtual void finish(int r) override {
+    if (image_ctx.state->is_refresh_required()) {
+      image_ctx.state->refresh(on_finish);
+      return;
+    }
+
+    on_finish->complete(0);
+  }
+};
+
 } // anonymous namespace
 
 template <typename I>
@@ -323,9 +342,10 @@ void Replay<I>::handle_event(const journal::SnapCreateEvent &event,
   op_event->ignore_error_codes = {-EEXIST};
 
   // avoid lock cycles
-  m_image_ctx.op_work_queue->queue(
-    new ExecuteOp<I, journal::SnapCreateEvent>(m_image_ctx, event,
-                                               on_op_complete), 0);
+  m_image_ctx.op_work_queue->queue(new C_RefreshIfRequired<I>(
+    m_image_ctx, 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
@@ -343,11 +363,12 @@ void Replay<I>::handle_event(const journal::SnapRemoveEvent &event,
   OpEvent *op_event;
   Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe,
                                                        &op_event);
-  op_event->on_op_finish_event = new FunctionContext(
-    [this, event, on_op_complete](int r) {
-      m_image_ctx.operations->snap_remove(event.snap_name.c_str(),
-                                          on_op_complete);
-    });
+  op_event->on_op_finish_event = new C_RefreshIfRequired<I>(
+    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);
+      }));
 
   // ignore errors caused due to replay
   op_event->ignore_error_codes = {-ENOENT};
@@ -365,12 +386,13 @@ void Replay<I>::handle_event(const journal::SnapRenameEvent &event,
   OpEvent *op_event;
   Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe,
                                                        &op_event);
-  op_event->on_op_finish_event = 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);
-    });
+  op_event->on_op_finish_event = new C_RefreshIfRequired<I>(
+    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);
+      }));
 
   // ignore errors caused due to replay
   op_event->ignore_error_codes = {-EEXIST};
@@ -388,11 +410,12 @@ void Replay<I>::handle_event(const journal::SnapProtectEvent &event,
   OpEvent *op_event;
   Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe,
                                                        &op_event);
-  op_event->on_op_finish_event = new FunctionContext(
-    [this, event, on_op_complete](int r) {
-      m_image_ctx.operations->snap_protect(event.snap_name.c_str(),
-                                           on_op_complete);
-    });
+  op_event->on_op_finish_event = new C_RefreshIfRequired<I>(
+    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);
+      }));
 
   // ignore errors caused due to replay
   op_event->ignore_error_codes = {-EBUSY};
@@ -411,11 +434,12 @@ void Replay<I>::handle_event(const journal::SnapUnprotectEvent &event,
   OpEvent *op_event;
   Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe,
                                                        &op_event);
-  op_event->on_op_finish_event = new FunctionContext(
-    [this, event, on_op_complete](int r) {
-      m_image_ctx.operations->snap_unprotect(event.snap_name.c_str(),
-                                             on_op_complete);
-    });
+  op_event->on_op_finish_event = new C_RefreshIfRequired<I>(
+    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);
+      }));
 
   // ignore errors caused due to replay
   op_event->ignore_error_codes = {-EINVAL};
@@ -434,12 +458,13 @@ void Replay<I>::handle_event(const journal::SnapRollbackEvent &event,
   OpEvent *op_event;
   Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe,
                                                        &op_event);
-  op_event->on_op_finish_event = 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);
-    });
+  op_event->on_op_finish_event = new C_RefreshIfRequired<I>(
+    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);
+      }));
 
   on_ready->complete(0);
 }
@@ -454,10 +479,12 @@ void Replay<I>::handle_event(const journal::RenameEvent &event,
   OpEvent *op_event;
   Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe,
                                                        &op_event);
-  op_event->on_op_finish_event = new FunctionContext(
-    [this, event, on_op_complete](int r) {
-      m_image_ctx.operations->rename(event.image_name.c_str(), on_op_complete);
-    });
+  op_event->on_op_finish_event = new C_RefreshIfRequired<I>(
+    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);
+      }));
 
   // ignore errors caused due to replay
   op_event->ignore_error_codes = {-EEXIST};
@@ -477,9 +504,9 @@ void Replay<I>::handle_event(const journal::ResizeEvent &event,
                                                        &op_event);
 
   // avoid lock cycles
-  m_image_ctx.op_work_queue->queue(
-    new ExecuteOp<I, journal::ResizeEvent>(m_image_ctx, event,
-                                               on_op_complete), 0);
+  m_image_ctx.op_work_queue->queue(new C_RefreshIfRequired<I>(
+    m_image_ctx, 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
@@ -497,10 +524,12 @@ void Replay<I>::handle_event(const journal::FlattenEvent &event,
   OpEvent *op_event;
   Context *on_op_complete = create_op_context_callback(event.op_tid, on_safe,
                                                        &op_event);
-  op_event->on_op_finish_event = new FunctionContext(
-    [this, event, on_op_complete](int r) {
-      m_image_ctx.operations->flatten(no_op_progress_callback, on_op_complete);
-    });
+  op_event->on_op_finish_event = new C_RefreshIfRequired<I>(
+    m_image_ctx, new FunctionContext(
+      [this, event, on_op_complete](int r) {
+        m_image_ctx.operations->flatten(no_op_progress_callback,
+                                        on_op_complete);
+      }));
 
   // ignore errors caused due to replay
   op_event->ignore_error_codes = {-EINVAL};

diff --git a/src/test/Makefile-client.am b/src/test/Makefile-client.am
index 987e773d04e9326d025d40c0bae809159675618a..2b0e804e5265a207dfefb67bf7e01d425d842d42 100644 (file)
--- a/src/test/Makefile-client.am
+++ b/src/test/Makefile-client.am
@@ -425,6 +425,7 @@ noinst_HEADERS += \
        test/librbd/mock/MockContextWQ.h \
        test/librbd/mock/MockExclusiveLock.h \
        test/librbd/mock/MockImageCtx.h \
+       test/librbd/mock/MockImageState.h \
        test/librbd/mock/MockImageWatcher.h \
        test/librbd/mock/MockJournal.h \
        test/librbd/mock/MockObjectMap.h \

diff --git a/src/test/librbd/journal/test_mock_Replay.cc b/src/test/librbd/journal/test_mock_Replay.cc
index 539599538cf727c48bd2b72491cb36ab61834acc..ebb3123490d707a5caddd2aefcdd4005329467f6 100644 (file)
--- a/src/test/librbd/journal/test_mock_Replay.cc
+++ b/src/test/librbd/journal/test_mock_Replay.cc
@@ -188,6 +188,16 @@ public:
                                   NotifyInvoke(&m_invoke_lock, &m_invoke_cond)));
   }
 
+  void expect_refresh_image(MockReplayImageCtx &mock_image_ctx, bool required,
+                            int r) {
+    EXPECT_CALL(*mock_image_ctx.state, is_refresh_required())
+                  .WillOnce(Return(required));
+    if (required) {
+      EXPECT_CALL(*mock_image_ctx.state, refresh(_))
+                    .WillOnce(CompleteContext(r, mock_image_ctx.image_ctx->op_work_queue));
+    }
+  }
+
   void when_process(MockJournalReplay &mock_journal_replay,
                     EventEntry &&event_entry, Context *on_ready,
                     Context *on_safe) {
@@ -505,6 +515,7 @@ TEST_F(TestMockJournalReplay, BlockedOpFinishError) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_snap_create(mock_image_ctx, &on_finish, "snap", 123);
 
   C_SaferCond on_start_ready;
@@ -539,9 +550,11 @@ TEST_F(TestMockJournalReplay, MissingOpFinishEvent) {
 
   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;
@@ -582,6 +595,7 @@ TEST_F(TestMockJournalReplay, MissingOpFinishEventCancelOps) {
 
   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);
 
   C_SaferCond on_snap_remove_ready;
@@ -634,6 +648,7 @@ TEST_F(TestMockJournalReplay, OpEventError) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_snap_remove(mock_image_ctx, &on_finish, "snap");
 
   C_SaferCond on_start_ready;
@@ -665,6 +680,7 @@ TEST_F(TestMockJournalReplay, SnapCreateEvent) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_snap_create(mock_image_ctx, &on_finish, "snap", 123);
 
   C_SaferCond on_start_ready;
@@ -701,6 +717,7 @@ TEST_F(TestMockJournalReplay, SnapCreateEventExists) {
 
   InSequence seq;
   Context *on_finish = nullptr;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_snap_create(mock_image_ctx, &on_finish, "snap", 123);
 
   C_SaferCond on_start_ready;
@@ -733,6 +750,7 @@ TEST_F(TestMockJournalReplay, SnapRemoveEvent) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_snap_remove(mock_image_ctx, &on_finish, "snap");
 
   C_SaferCond on_start_ready;
@@ -764,6 +782,7 @@ TEST_F(TestMockJournalReplay, SnapRemoveEventDNE) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_snap_remove(mock_image_ctx, &on_finish, "snap");
 
   C_SaferCond on_start_ready;
@@ -795,6 +814,7 @@ TEST_F(TestMockJournalReplay, SnapRenameEvent) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_snap_rename(mock_image_ctx, &on_finish, 234, "snap");
 
   C_SaferCond on_start_ready;
@@ -827,6 +847,7 @@ TEST_F(TestMockJournalReplay, SnapRenameEventExists) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_snap_rename(mock_image_ctx, &on_finish, 234, "snap");
 
   C_SaferCond on_start_ready;
@@ -859,6 +880,7 @@ TEST_F(TestMockJournalReplay, SnapProtectEvent) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_snap_protect(mock_image_ctx, &on_finish, "snap");
 
   C_SaferCond on_start_ready;
@@ -890,6 +912,7 @@ TEST_F(TestMockJournalReplay, SnapProtectEventBusy) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_snap_protect(mock_image_ctx, &on_finish, "snap");
 
   C_SaferCond on_start_ready;
@@ -921,6 +944,7 @@ TEST_F(TestMockJournalReplay, SnapUnprotectEvent) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_snap_unprotect(mock_image_ctx, &on_finish, "snap");
 
   C_SaferCond on_start_ready;
@@ -952,6 +976,7 @@ TEST_F(TestMockJournalReplay, SnapUnprotectEventInvalid) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_snap_unprotect(mock_image_ctx, &on_finish, "snap");
 
   C_SaferCond on_start_ready;
@@ -983,6 +1008,7 @@ TEST_F(TestMockJournalReplay, SnapRollbackEvent) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_snap_rollback(mock_image_ctx, &on_finish, "snap");
 
   C_SaferCond on_start_ready;
@@ -1014,6 +1040,7 @@ TEST_F(TestMockJournalReplay, RenameEvent) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_rename(mock_image_ctx, &on_finish, "image");
 
   C_SaferCond on_start_ready;
@@ -1045,6 +1072,7 @@ TEST_F(TestMockJournalReplay, RenameEventExists) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_rename(mock_image_ctx, &on_finish, "image");
 
   C_SaferCond on_start_ready;
@@ -1076,6 +1104,7 @@ TEST_F(TestMockJournalReplay, ResizeEvent) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_resize(mock_image_ctx, &on_finish, 234, 123);
 
   C_SaferCond on_start_ready;
@@ -1112,6 +1141,7 @@ TEST_F(TestMockJournalReplay, FlattenEvent) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_flatten(mock_image_ctx, &on_finish);
 
   C_SaferCond on_start_ready;
@@ -1143,6 +1173,7 @@ TEST_F(TestMockJournalReplay, FlattenEventInvalid) {
 
   InSequence seq;
   Context *on_finish;
+  expect_refresh_image(mock_image_ctx, false, 0);
   expect_flatten(mock_image_ctx, &on_finish);
 
   C_SaferCond on_start_ready;
@@ -1188,5 +1219,42 @@ TEST_F(TestMockJournalReplay, UnknownEvent) {
   ASSERT_EQ(0, on_ready.wait());
 }
 
+TEST_F(TestMockJournalReplay, RefreshImageBeforeOpStart) {
+  REQUIRE_FEATURE(RBD_FEATURE_JOURNALING);
+
+  librbd::ImageCtx *ictx;
+  ASSERT_EQ(0, open_image(m_image_name, &ictx));
+
+  MockReplayImageCtx mock_image_ctx(*ictx);
+  MockJournalReplay mock_journal_replay(mock_image_ctx);
+  expect_op_work_queue(mock_image_ctx);
+
+  InSequence seq;
+  Context *on_finish;
+  expect_refresh_image(mock_image_ctx, true, 0);
+  expect_resize(mock_image_ctx, &on_finish, 234, 123);
+
+  C_SaferCond on_start_ready;
+  C_SaferCond on_start_safe;
+  when_process(mock_journal_replay, EventEntry{ResizeEvent(123, 234)},
+               &on_start_ready, &on_start_safe);
+
+  C_SaferCond on_resume;
+  when_replay_op_ready(mock_journal_replay, 123, &on_resume);
+  ASSERT_EQ(0, on_start_ready.wait());
+
+  C_SaferCond on_finish_ready;
+  C_SaferCond on_finish_safe;
+  when_process(mock_journal_replay, EventEntry{OpFinishEvent(123, 0)},
+               &on_finish_ready, &on_finish_safe);
+
+  ASSERT_EQ(0, on_resume.wait());
+  on_finish->complete(0);
+
+  ASSERT_EQ(0, on_start_safe.wait());
+  ASSERT_EQ(0, on_finish_ready.wait());
+  ASSERT_EQ(0, on_finish_safe.wait());
+}
+
 } // namespace journal
 } // namespace librbd

diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h
index c7f2abfad3222b23054904763a974aaee67d9a9e..b6e8e9e7362d186b0712e2bf8f62c8d8977ebafd 100644 (file)
--- a/src/test/librbd/mock/MockImageCtx.h
+++ b/src/test/librbd/mock/MockImageCtx.h
@@ -7,6 +7,7 @@
 #include "test/librbd/mock/MockAioImageRequestWQ.h"
 #include "test/librbd/mock/MockContextWQ.h"
 #include "test/librbd/mock/MockExclusiveLock.h"
+#include "test/librbd/mock/MockImageState.h"
 #include "test/librbd/mock/MockImageWatcher.h"
 #include "test/librbd/mock/MockJournal.h"
 #include "test/librbd/mock/MockObjectMap.h"
@@ -59,6 +60,7 @@ struct MockImageCtx {
       aio_work_queue(new MockAioImageRequestWQ()),
       op_work_queue(new MockContextWQ()),
       parent(NULL), operations(new MockOperations()),
+      state(new MockImageState()),
       image_watcher(NULL), object_map(NULL),
       exclusive_lock(NULL), journal(NULL),
       concurrent_management_ops(image_ctx.concurrent_management_ops),
@@ -85,6 +87,7 @@ struct MockImageCtx {
     image_ctx->md_ctx.aio_flush();
     image_ctx->data_ctx.aio_flush();
     image_ctx->op_work_queue->drain();
+    delete state;
     delete operations;
     delete image_watcher;
     delete op_work_queue;
@@ -200,6 +203,7 @@ struct MockImageCtx {
 
   MockImageCtx *parent;
   MockOperations *operations;
+  MockImageState *state;
 
   MockImageWatcher *image_watcher;
   MockObjectMap *object_map;

diff --git a/src/test/librbd/mock/MockImageState.h b/src/test/librbd/mock/MockImageState.h
new file mode 100644 (file)
index 0000000..6f75ecd
--- /dev/null
+++ b/src/test/librbd/mock/MockImageState.h
@@ -0,0 +1,20 @@
+// -*- mode:C; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+
+#ifndef CEPH_TEST_LIBRBD_MOCK_IMAGE_STATE_H
+#define CEPH_TEST_LIBRBD_MOCK_IMAGE_STATE_H
+
+#include <gmock/gmock.h>
+
+class Context;
+
+namespace librbd {
+
+struct MockImageState {
+  MOCK_CONST_METHOD0(is_refresh_required, bool());
+  MOCK_METHOD1(refresh, void(Context*));
+};
+
+} // namespace librbd
+
+#endif // CEPH_TEST_LIBRBD_MOCK_IMAGE_STATE_H