From fef89753d8b086db3e81e977b66f60fe0a2fa35c Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Fri, 5 Jan 2018 13:54:24 -0500 Subject: [PATCH] librbd: retrieve the op features on image refresh Signed-off-by: Jason Dillaman --- src/librbd/ImageCtx.h | 2 + src/librbd/image/RefreshRequest.cc | 48 ++++++++ src/librbd/image/RefreshRequest.h | 7 ++ .../librbd/image/test_mock_RefreshRequest.cc | 112 +++++++++++++++--- src/test/librbd/mock/MockImageCtx.h | 4 + 5 files changed, 155 insertions(+), 18 deletions(-) diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h index a7ec2989d72..d6978c23dba 100644 --- a/src/librbd/ImageCtx.h +++ b/src/librbd/ImageCtx.h @@ -129,6 +129,8 @@ namespace librbd { cls::rbd::GroupSpec group_spec; uint64_t stripe_unit, stripe_count; uint64_t flags; + uint64_t op_features = 0; + bool operations_disabled = false; utime_t create_timestamp; file_layout_t layout; diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc index 3dff35fe6d3..fc5cb475214 100644 --- a/src/librbd/image/RefreshRequest.cc +++ b/src/librbd/image/RefreshRequest.cc @@ -403,6 +403,49 @@ Context *RefreshRequest::handle_v2_get_flags(int *result) { return m_on_finish; } + send_v2_get_op_features(); + return nullptr; +} + +template +void RefreshRequest::send_v2_get_op_features() { + if ((m_features & RBD_FEATURE_OPERATIONS) == 0LL) { + send_v2_get_group(); + return; + } + + CephContext *cct = m_image_ctx.cct; + ldout(cct, 10) << this << " " << __func__ << dendl; + + librados::ObjectReadOperation op; + cls_client::op_features_get_start(&op); + + librados::AioCompletion *comp = create_rados_callback< + RefreshRequest, &RefreshRequest::handle_v2_get_op_features>(this); + m_out_bl.clear(); + int r = m_image_ctx.md_ctx.aio_operate(m_image_ctx.header_oid, comp, &op, + &m_out_bl); + assert(r == 0); + comp->release(); +} + +template +Context *RefreshRequest::handle_v2_get_op_features(int *result) { + CephContext *cct = m_image_ctx.cct; + ldout(cct, 10) << this << " " << __func__ << ": " + << "r=" << *result << dendl; + + // -EOPNOTSUPP handler not required since feature bit implies OSD + // supports the method + if (*result == 0) { + bufferlist::iterator it = m_out_bl.begin(); + cls_client::op_features_get_finish(&it, &m_op_features); + } else if (*result < 0) { + lderr(cct) << "failed to retrieve op features: " << cpp_strerror(*result) + << dendl; + return m_on_finish; + } + send_v2_get_group(); return nullptr; } @@ -1079,11 +1122,16 @@ void RefreshRequest::apply() { m_image_ctx.order = m_order; m_image_ctx.features = 0; m_image_ctx.flags = 0; + m_image_ctx.op_features = 0; + m_image_ctx.operations_disabled = false; m_image_ctx.object_prefix = std::move(m_object_prefix); m_image_ctx.init_layout(); } else { m_image_ctx.features = m_features; m_image_ctx.flags = m_flags; + m_image_ctx.op_features = m_op_features; + m_image_ctx.operations_disabled = ( + (m_op_features & ~RBD_OPERATION_FEATURES_ALL) != 0ULL); m_image_ctx.group_spec = m_group_spec; m_image_ctx.parent_md = m_parent_md; } diff --git a/src/librbd/image/RefreshRequest.h b/src/librbd/image/RefreshRequest.h index 33f4d2568af..430b351c708 100644 --- a/src/librbd/image/RefreshRequest.h +++ b/src/librbd/image/RefreshRequest.h @@ -57,6 +57,9 @@ private: * v | * V2_GET_FLAGS | * | | + * v (skip if not enabled) | + * V2_GET_OP_FEATURES | + * | | * v | * V2_GET_GROUP | * | | @@ -130,6 +133,7 @@ private: uint64_t m_features = 0; uint64_t m_incompatible_features = 0; uint64_t m_flags = 0; + uint64_t m_op_features = 0; std::string m_last_metadata_key; std::map m_metadata; @@ -176,6 +180,9 @@ private: void send_v2_get_flags(); Context *handle_v2_get_flags(int *result); + void send_v2_get_op_features(); + Context *handle_v2_get_op_features(int *result); + void send_v2_get_group(); Context *handle_v2_get_group(int *result); diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc index 7747956e34b..97ffa7e2157 100644 --- a/src/test/librbd/image/test_mock_RefreshRequest.cc +++ b/src/test/librbd/image/test_mock_RefreshRequest.cc @@ -89,6 +89,7 @@ using ::testing::_; using ::testing::DoAll; using ::testing::DoDefault; using ::testing::InSequence; +using ::testing::Invoke; using ::testing::Return; using ::testing::WithArg; using ::testing::StrEq; @@ -134,16 +135,25 @@ public: } } - void expect_get_mutable_metadata(MockRefreshImageCtx &mock_image_ctx, int r) { + void expect_get_mutable_metadata(MockRefreshImageCtx &mock_image_ctx, + uint64_t features, 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_size"), _, _, _)); if (r < 0) { expect.WillOnce(Return(r)); } else { + uint64_t incompatible = ( + mock_image_ctx.read_only ? features & RBD_FEATURES_INCOMPATIBLE : + features & RBD_FEATURES_RW_INCOMPATIBLE); + expect.WillOnce(DoDefault()); EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(mock_image_ctx.header_oid, _, StrEq("rbd"), StrEq("get_features"), _, _, _)) - .WillOnce(DoDefault()); + .WillOnce(WithArg<5>(Invoke([features, incompatible](bufferlist* out_bl) { + encode(features, *out_bl); + encode(incompatible, *out_bl); + return 0; + }))); EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(mock_image_ctx.header_oid, _, StrEq("rbd"), StrEq("get_snapcontext"), _, _, _)) .WillOnce(DoDefault()); @@ -178,6 +188,17 @@ public: } } + void expect_get_op_features(MockRefreshImageCtx &mock_image_ctx, + uint64_t op_features, int r) { + EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), + exec(mock_image_ctx.header_oid, _, StrEq("rbd"), + StrEq("op_features_get"), _, _, _)) + .WillOnce(WithArg<5>(Invoke([op_features, r](bufferlist* out_bl) { + encode(op_features, *out_bl); + return r; + }))); + } + void expect_get_group(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"), @@ -416,7 +437,7 @@ TEST_F(TestMockImageRefreshRequest, SuccessV2) { expect_test_features(mock_image_ctx); InSequence seq; - expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -447,7 +468,7 @@ TEST_F(TestMockImageRefreshRequest, SuccessSnapshotV2) { expect_test_features(mock_image_ctx); InSequence seq; - expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -484,7 +505,7 @@ TEST_F(TestMockImageRefreshRequest, SuccessSetSnapshotV2) { expect_test_features(mock_image_ctx); InSequence seq; - expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -540,7 +561,7 @@ TEST_F(TestMockImageRefreshRequest, SuccessChild) { expect_test_features(mock_image_ctx); InSequence seq; - expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -592,7 +613,7 @@ TEST_F(TestMockImageRefreshRequest, SuccessChildDontOpenParent) { expect_test_features(mock_image_ctx); InSequence seq; - expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -608,6 +629,41 @@ TEST_F(TestMockImageRefreshRequest, SuccessChildDontOpenParent) { ASSERT_EQ(0, ctx.wait()); } +TEST_F(TestMockImageRefreshRequest, SuccessOpFeatures) { + 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); + + mock_image_ctx.features |= RBD_FEATURE_OPERATIONS; + + InSequence seq; + expect_get_mutable_metadata(mock_image_ctx, mock_image_ctx.features, 0); + expect_get_metadata(mock_image_ctx, 0); + expect_apply_metadata(mock_image_ctx, 0); + expect_get_flags(mock_image_ctx, 0); + expect_get_op_features(mock_image_ctx, 4096, 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()); + ASSERT_EQ(4096, mock_image_ctx.op_features); + ASSERT_TRUE(mock_image_ctx.operations_disabled); +} + TEST_F(TestMockImageRefreshRequest, DisableExclusiveLock) { REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK); @@ -650,13 +706,15 @@ TEST_F(TestMockImageRefreshRequest, DisableExclusiveLock) { false)); } + ASSERT_EQ(0, ictx->state->refresh()); + expect_op_work_queue(mock_image_ctx); expect_test_features(mock_image_ctx); // verify that exclusive lock is properly handled when object map // and journaling were never enabled (or active) InSequence seq; - expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -703,13 +761,15 @@ TEST_F(TestMockImageRefreshRequest, DisableExclusiveLockWhileAcquiringLock) { false)); } + ASSERT_EQ(0, ictx->state->refresh()); + expect_op_work_queue(mock_image_ctx); expect_test_features(mock_image_ctx); // verify that exclusive lock is properly handled when object map // and journaling were never enabled (or active) InSequence seq; - expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -739,6 +799,8 @@ TEST_F(TestMockImageRefreshRequest, JournalDisabledByPolicy) { false)); } + ASSERT_EQ(0, ictx->state->refresh()); + MockRefreshImageCtx mock_image_ctx(*ictx); MockRefreshParentRequest mock_refresh_parent_request; @@ -752,7 +814,7 @@ TEST_F(TestMockImageRefreshRequest, JournalDisabledByPolicy) { expect_is_exclusive_lock_owner(mock_exclusive_lock, true); InSequence seq; - expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -786,6 +848,8 @@ TEST_F(TestMockImageRefreshRequest, EnableJournalWithExclusiveLock) { false)); } + ASSERT_EQ(0, ictx->state->refresh()); + MockRefreshImageCtx mock_image_ctx(*ictx); MockRefreshParentRequest mock_refresh_parent_request; @@ -800,7 +864,7 @@ 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_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -835,6 +899,8 @@ TEST_F(TestMockImageRefreshRequest, EnableJournalWithoutExclusiveLock) { false)); } + ASSERT_EQ(0, ictx->state->refresh()); + MockRefreshImageCtx mock_image_ctx(*ictx); MockRefreshParentRequest mock_refresh_parent_request; @@ -847,7 +913,7 @@ 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_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -887,12 +953,14 @@ TEST_F(TestMockImageRefreshRequest, DisableJournal) { false)); } + ASSERT_EQ(0, ictx->state->refresh()); + expect_op_work_queue(mock_image_ctx); expect_test_features(mock_image_ctx); // verify journal is closed if feature disabled InSequence seq; - expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -923,6 +991,8 @@ TEST_F(TestMockImageRefreshRequest, EnableObjectMapWithExclusiveLock) { false)); } + ASSERT_EQ(0, ictx->state->refresh()); + MockRefreshImageCtx mock_image_ctx(*ictx); MockRefreshParentRequest mock_refresh_parent_request; @@ -937,7 +1007,7 @@ 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_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -963,6 +1033,8 @@ TEST_F(TestMockImageRefreshRequest, EnableObjectMapWithoutExclusiveLock) { false)); } + ASSERT_EQ(0, ictx->state->refresh()); + MockRefreshImageCtx mock_image_ctx(*ictx); MockRefreshParentRequest mock_refresh_parent_request; @@ -975,7 +1047,7 @@ 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_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -1019,12 +1091,14 @@ TEST_F(TestMockImageRefreshRequest, DisableObjectMap) { false)); } + ASSERT_EQ(0, ictx->state->refresh()); + expect_op_work_queue(mock_image_ctx); expect_test_features(mock_image_ctx); // verify object map is closed if feature disabled InSequence seq; - expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -1050,6 +1124,8 @@ TEST_F(TestMockImageRefreshRequest, OpenObjectMapError) { false)); } + ASSERT_EQ(0, ictx->state->refresh()); + MockRefreshImageCtx mock_image_ctx(*ictx); MockRefreshParentRequest mock_refresh_parent_request; @@ -1064,7 +1140,7 @@ 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_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, 0); expect_get_flags(mock_image_ctx, 0); @@ -1093,7 +1169,7 @@ TEST_F(TestMockImageRefreshRequest, ApplyMetadataError) { expect_test_features(mock_image_ctx); InSequence seq; - expect_get_mutable_metadata(mock_image_ctx, 0); + expect_get_mutable_metadata(mock_image_ctx, ictx->features, 0); expect_get_metadata(mock_image_ctx, 0); expect_apply_metadata(mock_image_ctx, -EINVAL); expect_get_flags(mock_image_ctx, 0); diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h index 1d074fc8893..9da47beb3e7 100644 --- a/src/test/librbd/mock/MockImageCtx.h +++ b/src/test/librbd/mock/MockImageCtx.h @@ -71,6 +71,8 @@ struct MockImageCtx { size(image_ctx.size), features(image_ctx.features), flags(image_ctx.flags), + op_features(image_ctx.op_features), + operations_disabled(image_ctx.operations_disabled), stripe_unit(image_ctx.stripe_unit), stripe_count(image_ctx.stripe_count), object_prefix(image_ctx.object_prefix), @@ -262,6 +264,8 @@ struct MockImageCtx { uint64_t size; uint64_t features; uint64_t flags; + uint64_t op_features; + bool operations_disabled; uint64_t stripe_unit; uint64_t stripe_count; std::string object_prefix;