From d1a67c3bda09430bb21f768fb743b3e41c7d411e Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 2 May 2019 16:55:44 -0400 Subject: [PATCH] librbd: re-add support for nautilus clients talking to jewel clusters We want to support N - 3 client backward compatibility (special case to support Jewel since it was a LTS release). The "get_snapshot_timestamp" cls method does not exist in Jewel clusters so librbd should fallback to excluding the op if it fails. Note that this N - 3 also needs to apply for downstream releases as well, which implies we still need Jewel for the time being. Fixes: http://tracker.ceph.com/issues/39450 Signed-off-by: Jason Dillaman (cherry picked from commit c644121820b83c97e68f9896393a45cd34787672) Conflicts: src/test/librbd/image/test_mock_RefreshRequest.cc: tweaked to support pool configs --- src/librbd/image/RefreshRequest.cc | 23 +++++--- src/librbd/image/RefreshRequest.h | 8 ++- .../librbd/image/test_mock_RefreshRequest.cc | 54 +++++++++++++++++-- 3 files changed, 73 insertions(+), 12 deletions(-) diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc index 8a93ee109b158..560b22ee6106b 100644 --- a/src/librbd/image/RefreshRequest.cc +++ b/src/librbd/image/RefreshRequest.cc @@ -670,11 +670,13 @@ void RefreshRequest::send_v2_get_snapshots() { librados::ObjectReadOperation op; for (auto snap_id : m_snapc.snaps) { - if (m_legacy_snapshot) { + if (m_legacy_snapshot != LEGACY_SNAPSHOT_DISABLED) { /// NOTE: remove after Luminous is retired cls_client::get_snapshot_name_start(&op, snap_id); cls_client::get_size_start(&op, snap_id); - cls_client::get_snapshot_timestamp_start(&op, snap_id); + if (m_legacy_snapshot != LEGACY_SNAPSHOT_ENABLED_NO_TIMESTAMP) { + cls_client::get_snapshot_timestamp_start(&op, snap_id); + } } else { cls_client::snapshot_get_start(&op, snap_id); } @@ -706,7 +708,7 @@ Context *RefreshRequest::handle_v2_get_snapshots(int *result) { auto it = m_out_bl.cbegin(); for (size_t i = 0; i < m_snapc.snaps.size(); ++i) { - if (m_legacy_snapshot) { + if (m_legacy_snapshot != LEGACY_SNAPSHOT_DISABLED) { /// NOTE: remove after Luminous is retired std::string snap_name; if (*result >= 0) { @@ -720,7 +722,9 @@ Context *RefreshRequest::handle_v2_get_snapshots(int *result) { } utime_t snap_timestamp; - if (*result >= 0) { + if (*result >= 0 && + m_legacy_snapshot != LEGACY_SNAPSHOT_ENABLED_NO_TIMESTAMP) { + /// NOTE: remove after Jewel is retired *result = cls_client::get_snapshot_timestamp_finish(&it, &snap_timestamp); } @@ -766,9 +770,16 @@ Context *RefreshRequest::handle_v2_get_snapshots(int *result) { ldout(cct, 10) << "out-of-sync snapshot state detected" << dendl; send_v2_get_mutable_metadata(); return nullptr; - } else if (!m_legacy_snapshot && *result == -EOPNOTSUPP) { + } else if (m_legacy_snapshot == LEGACY_SNAPSHOT_DISABLED && + *result == -EOPNOTSUPP) { ldout(cct, 10) << "retrying using legacy snapshot methods" << dendl; - m_legacy_snapshot = true; + m_legacy_snapshot = LEGACY_SNAPSHOT_ENABLED; + send_v2_get_snapshots(); + return nullptr; + } else if (m_legacy_snapshot == LEGACY_SNAPSHOT_ENABLED && + *result == -EOPNOTSUPP) { + ldout(cct, 10) << "retrying using legacy snapshot methods (jewel)" << dendl; + m_legacy_snapshot = LEGACY_SNAPSHOT_ENABLED_NO_TIMESTAMP; send_v2_get_snapshots(); return nullptr; } else if (*result < 0) { diff --git a/src/librbd/image/RefreshRequest.h b/src/librbd/image/RefreshRequest.h index 74e80cf98b159..4e314c7e6980c 100644 --- a/src/librbd/image/RefreshRequest.h +++ b/src/librbd/image/RefreshRequest.h @@ -120,6 +120,12 @@ private: * @endverbatim */ + enum LegacySnapshot { + LEGACY_SNAPSHOT_DISABLED, + LEGACY_SNAPSHOT_ENABLED, + LEGACY_SNAPSHOT_ENABLED_NO_TIMESTAMP + }; + ImageCtxT &m_image_ctx; bool m_acquiring_lock; bool m_skip_open_parent_image; @@ -136,7 +142,7 @@ private: bufferlist m_out_bl; bool m_legacy_parent = false; - bool m_legacy_snapshot = false; + LegacySnapshot m_legacy_snapshot = LEGACY_SNAPSHOT_DISABLED; uint8_t m_order = 0; uint64_t m_size = 0; diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc index 9e4fea44b3c9e..f9a076e3229c3 100644 --- a/src/test/librbd/image/test_mock_RefreshRequest.cc +++ b/src/test/librbd/image/test_mock_RefreshRequest.cc @@ -359,7 +359,8 @@ public: } } - void expect_get_snapshots_legacy(MockRefreshImageCtx &mock_image_ctx, int r) { + void expect_get_snapshots_legacy(MockRefreshImageCtx &mock_image_ctx, + bool include_timestamp, 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_snapshot_name"), _, _, _)); if (r < 0) { @@ -369,9 +370,11 @@ public: EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(mock_image_ctx.header_oid, _, StrEq("rbd"), StrEq("get_size"), _, _, _)) .WillOnce(DoDefault()); - EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), - exec(mock_image_ctx.header_oid, _, StrEq("rbd"), StrEq("get_snapshot_timestamp"), _, _, _)) - .WillOnce(DoDefault()); + if (include_timestamp) { + EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), + exec(mock_image_ctx.header_oid, _, StrEq("rbd"), StrEq("get_snapshot_timestamp"), _, _, _)) + .WillOnce(DoDefault()); + } EXPECT_CALL(get_mock_io_ctx(mock_image_ctx.md_ctx), exec(mock_image_ctx.header_oid, _, StrEq("rbd"), StrEq("get_parent"), _, _, _)) .WillOnce(DoDefault()); @@ -663,7 +666,7 @@ TEST_F(TestMockImageRefreshRequest, SuccessLegacySnapshotV2) { expect_apply_metadata(mock_image_ctx, 0); expect_get_group(mock_image_ctx, 0); expect_get_snapshots(mock_image_ctx, true, -EOPNOTSUPP); - expect_get_snapshots_legacy(mock_image_ctx, 0); + expect_get_snapshots_legacy(mock_image_ctx, true, 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); @@ -677,6 +680,47 @@ TEST_F(TestMockImageRefreshRequest, SuccessLegacySnapshotV2) { ASSERT_EQ(0, ctx.wait()); } +TEST_F(TestMockImageRefreshRequest, SuccessLegacySnapshotNoTimestampV2) { + REQUIRE_FORMAT_V2(); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + ASSERT_EQ(0, snap_create(*ictx, "snap")); + + 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, ictx->features, 0); + expect_get_parent(mock_image_ctx, -EOPNOTSUPP); + expect_get_parent_legacy(mock_image_ctx, 0); + MockGetMetadataRequest mock_get_metadata_request; + expect_get_metadata(mock_image_ctx, mock_get_metadata_request, + mock_image_ctx.header_oid, {}, 0); + expect_get_metadata(mock_image_ctx, mock_get_metadata_request, RBD_INFO, {}, + 0); + expect_apply_metadata(mock_image_ctx, 0); + expect_get_group(mock_image_ctx, 0); + expect_get_snapshots(mock_image_ctx, true, -EOPNOTSUPP); + expect_get_snapshots_legacy(mock_image_ctx, true, -EOPNOTSUPP); + expect_get_snapshots_legacy(mock_image_ctx, false, 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); + } + expect_add_snap(mock_image_ctx, "snap", ictx->snap_ids.begin()->second); + + C_SaferCond ctx; + MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, false, false, &ctx); + req->send(); + + ASSERT_EQ(0, ctx.wait()); +} + + TEST_F(TestMockImageRefreshRequest, SuccessSetSnapshotV2) { REQUIRE_FORMAT_V2(); -- 2.39.5