From ad01a883c5d9139d3ad3176a39a538bb8b247388 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Fri, 6 Oct 2017 15:03:34 -0400 Subject: [PATCH] librbd: refresh image after applying new/removing old metadata Fixes: http://tracker.ceph.com/issues/21711 Signed-off-by: Jason Dillaman --- src/librbd/Operations.cc | 21 +++++++++++++++------ src/librbd/Utils.cc | 16 +++++++++++++++- src/librbd/Utils.h | 3 +++ src/librbd/journal/Replay.cc | 6 ++++-- src/test/librbd/journal/test_Replay.cc | 21 +++++++++++++++------ src/test/librbd/journal/test_mock_Replay.cc | 5 ++--- 6 files changed, 54 insertions(+), 18 deletions(-) diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc index 15dff8224e5ac..65dea8f8c41ee 100644 --- a/src/librbd/Operations.cc +++ b/src/librbd/Operations.cc @@ -1352,13 +1352,11 @@ int Operations::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::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::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; } diff --git a/src/librbd/Utils.cc b/src/librbd/Utils.cc index dc6e7988e69c6..6f71841d7ee77 100644 --- a/src/librbd/Utils.cc +++ b/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 diff --git a/src/librbd/Utils.h b/src/librbd/Utils.h index 19c6344794339..1bdf1b6714a21 100644 --- a/src/librbd/Utils.h +++ b/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 diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc index a585208849297..e2e4f0e8e162b 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -816,7 +816,8 @@ void Replay::handle_event(const journal::MetadataSetEvent &event, return; } - op_event->on_op_finish_event = new C_RefreshIfRequired( + on_op_complete = new C_RefreshIfRequired(m_image_ctx, on_op_complete); + op_event->on_op_finish_event = util::create_async_context_callback( m_image_ctx, new ExecuteOp( m_image_ctx, event, on_op_complete)); @@ -837,7 +838,8 @@ void Replay::handle_event(const journal::MetadataRemoveEvent &event, return; } - op_event->on_op_finish_event = new C_RefreshIfRequired( + on_op_complete = new C_RefreshIfRequired(m_image_ctx, on_op_complete); + op_event->on_op_finish_event = util::create_async_context_callback( m_image_ctx, new ExecuteOp( m_image_ctx, event, on_op_complete)); diff --git a/src/test/librbd/journal/test_Replay.cc b/src/test/librbd/journal/test_Replay.cc index 0b7180e87cd62..01d510b610002 100644 --- a/src/test/librbd/journal/test_Replay.cc +++ b/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, ¤t_tag, ¤t_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")); diff --git a/src/test/librbd/journal/test_mock_Replay.cc b/src/test/librbd/journal/test_mock_Replay.cc index 8c39576905d7f..e318cc3916b59 100644 --- a/src/test/librbd/journal/test_mock_Replay.cc +++ b/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; -- 2.39.5