librbd: retrieve the op features on image refresh

Signed-off-by: Jason Dillaman <dillaman@redhat.com>

diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h
index a7ec2989d720d04009c24aa2f8abe5281b17bf63..d6978c23dba4d6ca8543c60b174afd887ecae277 100644 (file)
--- 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 3dff35fe6d3803baa29a2026d91788b786e1f606..fc5cb47521441c16aae541c84c6b80a374cc02fe 100644 (file)
--- a/src/librbd/image/RefreshRequest.cc
+++ b/src/librbd/image/RefreshRequest.cc
@@ -403,6 +403,49 @@ Context *RefreshRequest<I>::handle_v2_get_flags(int *result) {
     return m_on_finish;
   }
 
+  send_v2_get_op_features();
+  return nullptr;
+}
+
+template <typename I>
+void RefreshRequest<I>::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<I>, &RefreshRequest<I>::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 <typename I>
+Context *RefreshRequest<I>::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<I>::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 33f4d2568af551236b9bc41c18442398208bd32a..430b351c708ea485b0be6aa6884b30fb4c8b5181 100644 (file)
--- 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<std::string, bufferlist> 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 7747956e34b1e2e35a956b527ff77ab6fc00b034..97ffa7e215739d5065085b4896462a52c52741ab 100644 (file)
--- 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 1d074fc8893deefed878c574963642f6de1528d3..9da47beb3e73dd0b7d59e2643710dc947500ff01 100644 (file)
--- 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;