Merge pull request #18158 from dillaman/wip-21711

librbd: refresh image after applying new metadata

Reviewed-by: Mykola Golub <to.my.trociny@gmail.com>

src/librbd/Operations.cc

@@ -1352,13 +1352,11 @@ int Operations<I>::metadata_set(const std::string &key,
   ldout(cct, 5) << this << " " << __func__ << ": key=" << key << ", value="
                 << value << dendl;
 
-  string start = m_image_ctx.METADATA_CONF_PREFIX;
-  size_t conf_prefix_len = start.size();
-
-  if (key.size() > conf_prefix_len && !key.compare(0, conf_prefix_len, start)) {
+  std::string config_key;
+  bool config_override = util::is_metadata_config_override(key, &config_key);
+  if (config_override) {
     // validate config setting
-    string subkey = key.substr(conf_prefix_len, key.size() - conf_prefix_len);
-    int r = md_config_t().set_val(subkey.c_str(), value);
+    int r = md_config_t().set_val(config_key.c_str(), value);
     if (r < 0) {
       return r;
     }
@@ -1392,6 +1390,11 @@ int Operations<I>::metadata_set(const std::string &key,
     r = metadata_ctx.wait();
   }
 
+  if (config_override && r >= 0) {
+    // apply new config key immediately
+    r = m_image_ctx.state->refresh_if_required();
+  }
+
   return r;
 }
 
@@ -1454,6 +1457,12 @@ int Operations<I>::metadata_remove(const std::string &key) {
     r = metadata_ctx.wait();
   }
 
+  std::string config_key;
+  if (util::is_metadata_config_override(key, &config_key) && r >= 0) {
+    // apply new config key immediately
+    r = m_image_ctx.state->refresh_if_required();
+  }
+
   return r;
 }
 

src/librbd/Utils.cc

@@ -9,6 +9,7 @@
 #include "include/stringify.h"
 #include "include/rbd/features.h"
 #include "common/dout.h"
+#include "librbd/ImageCtx.h"
 
 #define dout_subsys ceph_subsys_rbd
 #undef dout_prefix
@@ -98,6 +99,19 @@ bool calc_sparse_extent(const bufferptr &bp,
   }
   return false;
 }
-} // namespace util
 
+bool is_metadata_config_override(const std::string& metadata_key,
+                                 std::string* config_key) {
+  size_t prefix_len = librbd::ImageCtx::METADATA_CONF_PREFIX.size();
+  if (metadata_key.size() > prefix_len &&
+      metadata_key.compare(0, prefix_len,
+                           librbd::ImageCtx::METADATA_CONF_PREFIX) == 0) {
+    *config_key = metadata_key.substr(prefix_len,
+                                      metadata_key.size() - prefix_len);
+    return true;
+  }
+  return false;
+}
+
+} // namespace util
 } // namespace librbd

src/librbd/Utils.h

@@ -208,6 +208,9 @@ inline ZTracer::Trace create_trace(const I &image_ctx, const char *trace_name,
   return ZTracer::Trace();
 }
 
+bool is_metadata_config_override(const std::string& metadata_key,
+                                 std::string* config_key);
+
 } // namespace util
 
 } // namespace librbd

src/librbd/journal/Replay.cc

@@ -816,7 +816,8 @@ void Replay<I>::handle_event(const journal::MetadataSetEvent &event,
     return;
   }
 
-  op_event->on_op_finish_event = new C_RefreshIfRequired<I>(
+  on_op_complete = new C_RefreshIfRequired<I>(m_image_ctx, on_op_complete);
+  op_event->on_op_finish_event = util::create_async_context_callback(
     m_image_ctx, new ExecuteOp<I, journal::MetadataSetEvent>(
       m_image_ctx, event, on_op_complete));
 
@@ -837,7 +838,8 @@ void Replay<I>::handle_event(const journal::MetadataRemoveEvent &event,
     return;
   }
 
-  op_event->on_op_finish_event = new C_RefreshIfRequired<I>(
+  on_op_complete = new C_RefreshIfRequired<I>(m_image_ctx, on_op_complete);
+  op_event->on_op_finish_event = util::create_async_context_callback(
     m_image_ctx, new ExecuteOp<I, journal::MetadataRemoveEvent>(
       m_image_ctx, event, on_op_complete));
 

src/test/librbd/journal/test_Replay.cc

@@ -741,7 +741,8 @@ TEST_F(TestJournalReplay, MetadataSet) {
   get_journal_commit_position(ictx, &initial_tag, &initial_entry);
 
   // inject metadata_set op into journal
-  inject_into_journal(ictx, librbd::journal::MetadataSetEvent(1, "key", "value"));
+  inject_into_journal(ictx, librbd::journal::MetadataSetEvent(
+    1, "conf_rbd_mirroring_replay_delay", "9876"));
   inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0));
   close_image(ictx);
 
@@ -755,9 +756,12 @@ TEST_F(TestJournalReplay, MetadataSet) {
   ASSERT_EQ(initial_tag + 1, current_tag);
   ASSERT_EQ(1, current_entry);
 
+  ASSERT_EQ(9876, ictx->mirroring_replay_delay);
+
   std::string value;
-  ASSERT_EQ(0, librbd::metadata_get(ictx, "key", &value));
-  ASSERT_EQ("value", value);
+  ASSERT_EQ(0, librbd::metadata_get(ictx, "conf_rbd_mirroring_replay_delay",
+                                    &value));
+  ASSERT_EQ("9876", value);
 
   // verify lock ordering constraints
   ASSERT_EQ(0, ictx->operations->metadata_set("key2", "value"));
@@ -771,7 +775,8 @@ TEST_F(TestJournalReplay, MetadataRemove) {
   ASSERT_EQ(0, open_image(m_image_name, &ictx));
   ASSERT_EQ(0, when_acquired_lock(ictx));
 
-  ASSERT_EQ(0, ictx->operations->metadata_set("key", "value"));
+  ASSERT_EQ(0, ictx->operations->metadata_set(
+    "conf_rbd_mirroring_replay_delay", "9876"));
 
   // get current commit position
   int64_t initial_tag;
@@ -779,7 +784,8 @@ TEST_F(TestJournalReplay, MetadataRemove) {
   get_journal_commit_position(ictx, &initial_tag, &initial_entry);
 
   // inject metadata_remove op into journal
-  inject_into_journal(ictx, librbd::journal::MetadataRemoveEvent(1, "key"));
+  inject_into_journal(ictx, librbd::journal::MetadataRemoveEvent(
+    1, "conf_rbd_mirroring_replay_delay"));
   inject_into_journal(ictx, librbd::journal::OpFinishEvent(1, 0));
   close_image(ictx);
 
@@ -792,9 +798,12 @@ TEST_F(TestJournalReplay, MetadataRemove) {
   get_journal_commit_position(ictx, &current_tag, &current_entry);
   ASSERT_EQ(initial_tag, current_tag);
   ASSERT_EQ(initial_entry + 2, current_entry);
+  ASSERT_EQ(0, ictx->mirroring_replay_delay);
 
   std::string value;
-  ASSERT_EQ(-ENOENT, librbd::metadata_get(ictx, "key", &value));
+  ASSERT_EQ(-ENOENT,
+            librbd::metadata_get(ictx, "conf_rbd_mirroring_replay_delay",
+                                 &value));
 
   // verify lock ordering constraints
   ASSERT_EQ(0, ictx->operations->metadata_set("key", "value"));

src/test/librbd/journal/test_mock_Replay.cc

@@ -1700,8 +1700,8 @@ TEST_F(TestMockJournalReplay, MetadataSetEvent) {
 
   InSequence seq;
   Context *on_finish = nullptr;
-  expect_refresh_image(mock_image_ctx, false, 0);
   expect_metadata_set(mock_image_ctx, &on_finish, "key", "value");
+  expect_refresh_image(mock_image_ctx, false, 0);
 
   C_SaferCond on_start_ready;
   C_SaferCond on_start_safe;
@@ -1737,8 +1737,8 @@ TEST_F(TestMockJournalReplay, MetadataRemoveEvent) {
 
   InSequence seq;
   Context *on_finish = nullptr;
-  expect_refresh_image(mock_image_ctx, false, 0);
   expect_metadata_remove(mock_image_ctx, &on_finish, "key");
+  expect_refresh_image(mock_image_ctx, false, 0);
 
   C_SaferCond on_start_ready;
   C_SaferCond on_start_safe;
@@ -1774,7 +1774,6 @@ TEST_F(TestMockJournalReplay, MetadataRemoveEventDNE) {
 
   InSequence seq;
   Context *on_finish = nullptr;
-  expect_refresh_image(mock_image_ctx, false, 0);
   expect_metadata_remove(mock_image_ctx, &on_finish, "key");
 
   C_SaferCond on_start_ready;