From caf17a876e75cd448947b7b729a8e39689a304df Mon Sep 17 00:00:00 2001 From: Dongsheng Yang Date: Fri, 11 Aug 2017 17:44:19 +0800 Subject: [PATCH] librbd: notify watcher when updating image metadata Signed-off-by: Dongsheng Yang (cherry picked from commit b10d26dfa84627b2622d405d272b1133bb773245) Conflicts: src/librbd/image/RefreshRequest.h (luminous does not have 233482cf708aa7b4723c529d387c4605bdf0722f so we leave the following variables uninitialized: uint8_t m_order; uint64_t m_size; uint64_t m_features; uint64_t m_incompatible_features; uint64_t m_flags; ) --- src/librbd/Operations.cc | 8 +- src/librbd/image/RefreshRequest.cc | 62 +++++++++++++++ src/librbd/image/RefreshRequest.h | 10 +++ .../librbd/image/test_mock_RefreshRequest.cc | 76 +++++++++++++++++++ src/test/librbd/mock/MockImageCtx.h | 1 + 5 files changed, 155 insertions(+), 2 deletions(-) diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc index 3fc256d72ceeb..15dff8224e5ac 100644 --- a/src/librbd/Operations.cc +++ b/src/librbd/Operations.cc @@ -1406,7 +1406,9 @@ void Operations::execute_metadata_set(const std::string &key, << value << dendl; operation::MetadataSetRequest *request = - new operation::MetadataSetRequest(m_image_ctx, on_finish, key, value); + new operation::MetadataSetRequest(m_image_ctx, + new C_NotifyUpdate(m_image_ctx, on_finish), + key, value); request->send(); } @@ -1464,7 +1466,9 @@ void Operations::execute_metadata_remove(const std::string &key, ldout(cct, 5) << this << " " << __func__ << ": key=" << key << dendl; operation::MetadataRemoveRequest *request = - new operation::MetadataRemoveRequest(m_image_ctx, on_finish, key); + new operation::MetadataRemoveRequest( + m_image_ctx, + new C_NotifyUpdate(m_image_ctx, on_finish), key); request->send(); } diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc index f8448dc0e0159..4ec161698f5dc 100644 --- a/src/librbd/image/RefreshRequest.cc +++ b/src/librbd/image/RefreshRequest.cc @@ -1,6 +1,9 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab +#include +#include "include/assert.h" + #include "librbd/image/RefreshRequest.h" #include "common/dout.h" #include "common/errno.h" @@ -22,6 +25,12 @@ namespace librbd { namespace image { +namespace { + +const uint64_t MAX_METADATA_ITEMS = 128; + +} + using util::create_rados_callback; using util::create_async_context_callback; using util::create_context_callback; @@ -285,6 +294,59 @@ Context *RefreshRequest::handle_v2_get_mutable_metadata(int *result) { m_incomplete_update = true; } + send_v2_get_metadata(); + return nullptr; +} + +template +void RefreshRequest::send_v2_get_metadata() { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 10) << this << " " << __func__ << ": " + << "start_key=" << m_last_metadata_key << dendl; + + librados::ObjectReadOperation op; + cls_client::metadata_list_start(&op, m_last_metadata_key, MAX_METADATA_ITEMS); + + using klass = RefreshRequest; + librados::AioCompletion *comp = + create_rados_callback(this); + m_out_bl.clear(); + m_image_ctx.md_ctx.aio_operate(m_image_ctx.header_oid, comp, &op, + &m_out_bl); + comp->release(); +} + +template +Context *RefreshRequest::handle_v2_get_metadata(int *result) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 10) << this << " " << __func__ << ": r=" << *result << dendl; + + std::map metadata; + if (*result == 0) { + bufferlist::iterator it = m_out_bl.begin(); + *result = cls_client::metadata_list_finish(&it, &metadata); + } + + if (*result == -EOPNOTSUPP || *result == -EIO) { + ldout(cct, 10) << "config metadata not supported by OSD" << dendl; + } else if (*result < 0) { + lderr(cct) << "failed to retrieve metadata: " << cpp_strerror(*result) + << dendl; + return m_on_finish; + } + + if (!metadata.empty()) { + m_metadata.insert(metadata.begin(), metadata.end()); + m_last_metadata_key = metadata.rbegin()->first; + if (boost::starts_with(m_last_metadata_key, + ImageCtx::METADATA_CONF_PREFIX)) { + send_v2_get_metadata(); + return nullptr; + } + } + + m_image_ctx.apply_metadata(m_metadata); + send_v2_get_flags(); return nullptr; } diff --git a/src/librbd/image/RefreshRequest.h b/src/librbd/image/RefreshRequest.h index 286703e39d57d..891fbf4565352 100644 --- a/src/librbd/image/RefreshRequest.h +++ b/src/librbd/image/RefreshRequest.h @@ -52,6 +52,9 @@ private: * \-----> V2_GET_MUTABLE_METADATA * | | * v | + * V2_GET_METADATA | + * | | + * v | * V2_GET_FLAGS | * | | * v | @@ -127,6 +130,10 @@ private: uint64_t m_features; uint64_t m_incompatible_features; uint64_t m_flags; + + std::string m_last_metadata_key; + std::map m_metadata; + std::string m_object_prefix; ParentInfo m_parent_md; cls::rbd::GroupSpec m_group_spec; @@ -163,6 +170,9 @@ private: void send_v2_get_mutable_metadata(); Context *handle_v2_get_mutable_metadata(int *result); + void send_v2_get_metadata(); + Context *handle_v2_get_metadata(int *result); + void send_v2_get_flags(); Context *handle_v2_get_flags(int *result); diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc index 5c50d8131a691..07490146a56b0 100644 --- a/src/test/librbd/image/test_mock_RefreshRequest.cc +++ b/src/test/librbd/image/test_mock_RefreshRequest.cc @@ -155,6 +155,16 @@ public: } } + void expect_get_metadata(MockRefreshImageCtx &mock_image_ctx, int r) { + auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), + exec(mock_image_ctx.header_oid, _, StrEq("rbd"), StrEq("metadata_list"), _, _, _)); + if (r < 0) { + expect.WillOnce(Return(r)); + } else { + expect.WillOnce(DoDefault()); + } + } + void expect_get_flags(MockRefreshImageCtx &mock_image_ctx, int r) { auto &expect = EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(mock_image_ctx.header_oid, _, StrEq("rbd"), StrEq("get_flags"), _, _, _)); @@ -216,6 +226,12 @@ public: } } + void expect_apply_metadata(MockRefreshImageCtx &mock_image_ctx, + int r) { + EXPECT_CALL(mock_image_ctx, apply_metadata(_)) + .WillOnce(Return(r)); + } + void expect_add_snap(MockRefreshImageCtx &mock_image_ctx, const std::string &snap_name, uint64_t snap_id) { EXPECT_CALL(mock_image_ctx, add_snap(_, snap_name, snap_id, _, _, _, _, _)); @@ -398,6 +414,8 @@ TEST_F(TestMockImageRefreshRequest, SuccessV2) { InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); @@ -427,6 +445,8 @@ TEST_F(TestMockImageRefreshRequest, SuccessSnapshotV2) { InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); @@ -462,6 +482,8 @@ TEST_F(TestMockImageRefreshRequest, SuccessSetSnapshotV2) { InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); @@ -516,6 +538,8 @@ TEST_F(TestMockImageRefreshRequest, SuccessChild) { InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_refresh_parent_is_required(*mock_refresh_parent_request, true); @@ -566,6 +590,8 @@ TEST_F(TestMockImageRefreshRequest, SuccessChildDontOpenParent) { InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) { @@ -628,6 +654,8 @@ TEST_F(TestMockImageRefreshRequest, DisableExclusiveLock) { // and journaling were never enabled (or active) InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); @@ -679,6 +707,8 @@ TEST_F(TestMockImageRefreshRequest, DisableExclusiveLockWhileAcquiringLock) { // and journaling were never enabled (or active) InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); @@ -720,6 +750,8 @@ TEST_F(TestMockImageRefreshRequest, JournalDisabledByPolicy) { InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); @@ -766,6 +798,8 @@ TEST_F(TestMockImageRefreshRequest, EnableJournalWithExclusiveLock) { // journal should be immediately opened if exclusive lock owned InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); @@ -811,6 +845,8 @@ TEST_F(TestMockImageRefreshRequest, EnableJournalWithoutExclusiveLock) { // do not open the journal if exclusive lock is not owned InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); @@ -854,6 +890,8 @@ TEST_F(TestMockImageRefreshRequest, DisableJournal) { // verify journal is closed if feature disabled InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); @@ -897,6 +935,8 @@ TEST_F(TestMockImageRefreshRequest, EnableObjectMapWithExclusiveLock) { // object map should be immediately opened if exclusive lock owned InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); @@ -933,6 +973,8 @@ TEST_F(TestMockImageRefreshRequest, EnableObjectMapWithoutExclusiveLock) { // do not open the object map if exclusive lock is not owned InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); @@ -980,6 +1022,8 @@ TEST_F(TestMockImageRefreshRequest, DisableObjectMap) { // verify object map is closed if feature disabled InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); @@ -1018,6 +1062,8 @@ TEST_F(TestMockImageRefreshRequest, OpenObjectMapError) { // object map should be immediately opened if exclusive lock owned InSequence seq; expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_refresh_parent_is_required(mock_refresh_parent_request, false); @@ -1031,5 +1077,35 @@ TEST_F(TestMockImageRefreshRequest, OpenObjectMapError) { ASSERT_EQ(nullptr, mock_image_ctx.object_map); } +TEST_F(TestMockImageRefreshRequest, ApplyMetadataError) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockRefreshImageCtx mock_image_ctx(*ictx); + MockRefreshParentRequest mock_refresh_parent_request; + MockExclusiveLock mock_exclusive_lock; + expect_op_work_queue(mock_image_ctx); + expect_test_features(mock_image_ctx); + + InSequence seq; + expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, -EINVAL); + expect_get_flags(mock_image_ctx, 0); + expect_get_group(mock_image_ctx, 0); + expect_refresh_parent_is_required(mock_refresh_parent_request, false); + if (ictx->test_features(RBD_FEATURE_EXCLUSIVE_LOCK)) { + expect_init_exclusive_lock(mock_image_ctx, mock_exclusive_lock, 0); + } + + C_SaferCond ctx; + MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, false, false, &ctx); + req->send(); + + ASSERT_EQ(0, ctx.wait()); +} + } // namespace image } // namespace librbd diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h index 92330a1ddafd1..f651574d16973 100644 --- a/src/test/librbd/mock/MockImageCtx.h +++ b/src/test/librbd/mock/MockImageCtx.h @@ -205,6 +205,7 @@ struct MockImageCtx { size_t, uint64_t, Context *, int, ZTracer::Trace *)); MOCK_METHOD8(write_to_cache, void(object_t, const bufferlist&, size_t, uint64_t, Context *, int, uint64_t, ZTracer::Trace *)); + MOCK_METHOD1(apply_metadata, int(const std::map &)); MOCK_CONST_METHOD0(get_stripe_count, uint64_t()); MOCK_CONST_METHOD0(get_stripe_period, uint64_t()); -- 2.39.5