From 8dc6a97b2ad7740963b2d746621db6eed2433e9b Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 9 May 2019 15:37:26 -0400 Subject: [PATCH] librbd: fix issues with object-map/fast-diff feature interlock Enabling/disabling one should enable/disable the other. Fixes: http://tracker.ceph.com/issues/39521 Signed-off-by: Jason Dillaman --- .../operation/DisableFeaturesRequest.cc | 8 +++++ src/librbd/operation/EnableFeaturesRequest.cc | 12 ++++++-- .../librbd/image/test_mock_RefreshRequest.cc | 30 ------------------- 3 files changed, 18 insertions(+), 32 deletions(-) diff --git a/src/librbd/operation/DisableFeaturesRequest.cc b/src/librbd/operation/DisableFeaturesRequest.cc index a248ec7c427f5..dc58a989ab952 100644 --- a/src/librbd/operation/DisableFeaturesRequest.cc +++ b/src/librbd/operation/DisableFeaturesRequest.cc @@ -175,6 +175,13 @@ Context *DisableFeaturesRequest::handle_acquire_exclusive_lock(int *result) { do { m_features &= image_ctx.features; + + // interlock object-map and fast-diff together + if (((m_features & RBD_FEATURE_OBJECT_MAP) != 0) || + ((m_features & RBD_FEATURE_FAST_DIFF) != 0)) { + m_features |= (RBD_FEATURE_OBJECT_MAP | RBD_FEATURE_FAST_DIFF); + } + m_new_features = image_ctx.features & ~m_features; m_features_mask = m_features; @@ -188,6 +195,7 @@ Context *DisableFeaturesRequest::handle_acquire_exclusive_lock(int *result) { break; } m_features_mask |= (RBD_FEATURE_OBJECT_MAP | + RBD_FEATURE_FAST_DIFF | RBD_FEATURE_JOURNALING); } if ((m_features & RBD_FEATURE_FAST_DIFF) != 0) { diff --git a/src/librbd/operation/EnableFeaturesRequest.cc b/src/librbd/operation/EnableFeaturesRequest.cc index 44938ffee66d5..e2c1113d0e9fd 100644 --- a/src/librbd/operation/EnableFeaturesRequest.cc +++ b/src/librbd/operation/EnableFeaturesRequest.cc @@ -175,6 +175,13 @@ Context *EnableFeaturesRequest::handle_get_mirror_mode(int *result) { } m_features &= ~image_ctx.features; + + // interlock object-map and fast-diff together + if (((m_features & RBD_FEATURE_OBJECT_MAP) != 0) || + ((m_features & RBD_FEATURE_FAST_DIFF) != 0)) { + m_features |= (RBD_FEATURE_OBJECT_MAP | RBD_FEATURE_FAST_DIFF); + } + m_new_features = image_ctx.features | m_features; m_features_mask = m_features; @@ -186,12 +193,13 @@ Context *EnableFeaturesRequest::handle_get_mirror_mode(int *result) { break; } m_enable_flags |= RBD_FLAG_OBJECT_MAP_INVALID; - m_features_mask |= RBD_FEATURE_EXCLUSIVE_LOCK; + m_features_mask |= (RBD_FEATURE_EXCLUSIVE_LOCK | RBD_FEATURE_FAST_DIFF); } if ((m_features & RBD_FEATURE_FAST_DIFF) != 0) { m_enable_flags |= RBD_FLAG_FAST_DIFF_INVALID; - m_features_mask |= (RBD_FEATURE_OBJECT_MAP | RBD_FEATURE_EXCLUSIVE_LOCK); + m_features_mask |= (RBD_FEATURE_EXCLUSIVE_LOCK | RBD_FEATURE_OBJECT_MAP); } + if ((m_features & RBD_FEATURE_JOURNALING) != 0) { if ((m_new_features & RBD_FEATURE_EXCLUSIVE_LOCK) == 0) { lderr(cct) << "cannot enable journaling. exclusive-lock must be " diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc index 4a6cea16e0412..8274e04b1086d 100644 --- a/src/test/librbd/image/test_mock_RefreshRequest.cc +++ b/src/test/librbd/image/test_mock_RefreshRequest.cc @@ -829,11 +829,6 @@ TEST_F(TestMockImageRefreshRequest, DisableExclusiveLock) { false)); } - if (ictx->test_features(RBD_FEATURE_FAST_DIFF)) { - ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_FAST_DIFF, - false)); - } - if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) { ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_OBJECT_MAP, false)); @@ -884,11 +879,6 @@ TEST_F(TestMockImageRefreshRequest, DisableExclusiveLockWhileAcquiringLock) { false)); } - if (ictx->test_features(RBD_FEATURE_FAST_DIFF)) { - ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_FAST_DIFF, - false)); - } - if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) { ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_OBJECT_MAP, false)); @@ -932,11 +922,6 @@ TEST_F(TestMockImageRefreshRequest, JournalDisabledByPolicy) { false)); } - if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) { - ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_OBJECT_MAP, - false)); - } - ASSERT_EQ(0, ictx->state->refresh()); MockRefreshImageCtx mock_image_ctx(*ictx); @@ -981,11 +966,6 @@ TEST_F(TestMockImageRefreshRequest, EnableJournalWithExclusiveLock) { false)); } - if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) { - ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_OBJECT_MAP, - false)); - } - ASSERT_EQ(0, ictx->state->refresh()); MockRefreshImageCtx mock_image_ctx(*ictx); @@ -1027,11 +1007,6 @@ TEST_F(TestMockImageRefreshRequest, EnableJournalWithoutExclusiveLock) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); - if (ictx->test_features(RBD_FEATURE_FAST_DIFF)) { - ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_FAST_DIFF, - false)); - } - if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) { ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_OBJECT_MAP, false)); @@ -1224,11 +1199,6 @@ TEST_F(TestMockImageRefreshRequest, DisableObjectMap) { false)); } - if (ictx->test_features(RBD_FEATURE_OBJECT_MAP)) { - ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_OBJECT_MAP, - false)); - } - ASSERT_EQ(0, ictx->state->refresh()); expect_op_work_queue(mock_image_ctx); -- 2.39.5