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 <dillaman@redhat.com>
(cherry picked from commit c644121820b83c97e68f9896393a45cd34787672)

Conflicts:
	src/test/librbd/image/test_mock_RefreshRequest.cc: tweaked to support pool configs

diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc
index 8a93ee109b158ca2a61e0d662516881e60905c71..560b22ee6106b0bde65e54fb4ce9dfa4c4c65541 100644 (file)
--- a/src/librbd/image/RefreshRequest.cc
+++ b/src/librbd/image/RefreshRequest.cc
@@ -670,11 +670,13 @@ void RefreshRequest<I>::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<I>::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<I>::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<I>::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 74e80cf98b159733ce208f3ecb70d8b234c85e84..4e314c7e6980c635ef632c7172d5d7f72fa1c427 100644 (file)
--- 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 9e4fea44b3c9e5459581accf2fc1495104016b80..f9a076e3229c3004a19b05e6b4dd7134c071e9ea 100644 (file)
--- 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();