librbd: track reason why ImageCtx is read-only

This will be utilized by the RefreshRequest state machine to flag the image
as read-only if the new RBD_FEATURE_NON_PRIMARY feature is enabled. Also
allow that flag to be masked out by rbd-mirror daemon to permit IO and
operations against a non-primary image.

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

diff --git a/src/librbd/ImageCtx.cc b/src/librbd/ImageCtx.cc
index 65669db5bb8296c770ae02b3f6395d5cab078d43..4026fb934595af9651644dc871979d6ee0ec50dd 100644 (file)
--- a/src/librbd/ImageCtx.cc
+++ b/src/librbd/ImageCtx.cc
@@ -100,6 +100,7 @@ public:
       snap_id(CEPH_NOSNAP),
       snap_exists(true),
       read_only(ro),
+      read_only_flags(ro ? IMAGE_READ_ONLY_FLAG_USER : 0U),
       exclusive_locked(false),
       name(image_name),
       image_watcher(NULL),

diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h
index ecc4cdb400c56d49a826e9fb8528c83579acd17b..11f33b1d40c669ac7b0753da4ccb8284fef0099e 100644 (file)
--- a/src/librbd/ImageCtx.h
+++ b/src/librbd/ImageCtx.h
@@ -97,6 +97,8 @@ namespace librbd {
     bool snap_exists; // false if our snap_id was deleted
     // whether the image was opened read-only. cannot be changed after opening
     bool read_only;
+    uint32_t read_only_flags = 0U;
+    uint32_t read_only_mask = ~0U;
 
     std::map<rados::cls::lock::locker_id_t,
             rados::cls::lock::locker_info_t> lockers;

diff --git a/src/librbd/Types.h b/src/librbd/Types.h
index 3f1104478ebcea505d3debe7e30cc212f0e8a193..2e014b8b476439ffc72aa047fc801e94092f8ab0 100644 (file)
--- a/src/librbd/Types.h
+++ b/src/librbd/Types.h
@@ -94,6 +94,11 @@ enum {
   OPEN_FLAG_IGNORE_MIGRATING = 1 << 2
 };
 
+enum ImageReadOnlyFlag {
+  IMAGE_READ_ONLY_FLAG_USER        = 1 << 0,
+  IMAGE_READ_ONLY_FLAG_NON_PRIMARY = 1 << 1,
+};
+
 struct MigrationInfo {
   int64_t pool_id = -1;
   std::string pool_namespace;

diff --git a/src/librbd/image/DetachChildRequest.cc b/src/librbd/image/DetachChildRequest.cc
index fdaeb331a7582113afb632bf5993b67736c7589d..d84a0edff115fc7fae4675b6774e73b7bbe3b4c5 100644 (file)
--- a/src/librbd/image/DetachChildRequest.cc
+++ b/src/librbd/image/DetachChildRequest.cc
@@ -164,6 +164,9 @@ void DetachChildRequest<I>::clone_v2_open_parent() {
   m_parent_image_ctx = I::create("", m_parent_spec.image_id, nullptr,
                                  m_parent_io_ctx, false);
 
+  // ensure non-primary images can be modified
+  m_parent_image_ctx->read_only_mask &= ~IMAGE_READ_ONLY_FLAG_NON_PRIMARY;
+
   auto ctx = create_context_callback<
     DetachChildRequest<I>,
     &DetachChildRequest<I>::handle_clone_v2_open_parent>(this);

diff --git a/src/librbd/image/OpenRequest.cc b/src/librbd/image/OpenRequest.cc
index 0188230c9f4ac4796bb487a3862f50007d3ef564..88d5d62e0e5ab27a981a4f7f5a6c4d3355067119 100644 (file)
--- a/src/librbd/image/OpenRequest.cc
+++ b/src/librbd/image/OpenRequest.cc
@@ -597,7 +597,7 @@ Context *OpenRequest<I>::send_init_cache(int *result) {
 
 template <typename I>
 Context *OpenRequest<I>::send_register_watch(int *result) {
-  if (m_image_ctx->read_only) {
+  if ((m_image_ctx->read_only_flags & IMAGE_READ_ONLY_FLAG_USER) != 0U) {
     return send_set_snap(result);
   }
 

diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc
index 602221bfdd6425e2e38ce31bdaa62572b56cdb03..8679e179af68d39ee14f39e393796307db0d6336 100644 (file)
--- a/src/librbd/image/RefreshRequest.cc
+++ b/src/librbd/image/RefreshRequest.cc
@@ -113,7 +113,7 @@ Context *RefreshRequest<I>::handle_get_migration_header(int *result) {
 
   switch(m_migration_spec.header_type) {
   case cls::rbd::MIGRATION_HEADER_TYPE_SRC:
-    if (!m_image_ctx.read_only) {
+    if (!m_read_only) {
       lderr(cct) << "image being migrated" << dendl;
       *result = -EROFS;
       return m_on_finish;
@@ -193,6 +193,12 @@ Context *RefreshRequest<I>::handle_v1_read_header(int *result) {
     }
   }
 
+  {
+    std::shared_lock image_locker{m_image_ctx.image_lock};
+    m_read_only = m_image_ctx.read_only;
+    m_read_only_flags = m_image_ctx.read_only_flags;
+  }
+
   memcpy(&v1_header, m_out_bl.c_str(), sizeof(v1_header));
   m_order = v1_header.options.order;
   m_size = v1_header.image_size;
@@ -332,9 +338,14 @@ void RefreshRequest<I>::send_v2_get_mutable_metadata() {
   {
     std::shared_lock image_locker{m_image_ctx.image_lock};
     snap_id = m_image_ctx.snap_id;
+    m_read_only = m_image_ctx.read_only;
+    m_read_only_flags = m_image_ctx.read_only_flags;
   }
 
-  bool read_only = m_image_ctx.read_only || snap_id != CEPH_NOSNAP;
+  // mask out the non-primary read-only flag since its state can change
+  bool read_only = (
+    ((m_read_only_flags & ~IMAGE_READ_ONLY_FLAG_NON_PRIMARY) != 0) ||
+    (snap_id != CEPH_NOSNAP));
   librados::ObjectReadOperation op;
   cls_client::get_size_start(&op, CEPH_NOSNAP);
   cls_client::get_features_start(&op, read_only);
@@ -411,6 +422,21 @@ Context *RefreshRequest<I>::handle_v2_get_mutable_metadata(int *result) {
     m_incomplete_update = true;
   }
 
+  if (((m_incompatible_features & RBD_FEATURE_NON_PRIMARY) != 0U) &&
+      ((m_read_only_flags & IMAGE_READ_ONLY_FLAG_NON_PRIMARY) == 0U) &&
+      ((m_image_ctx.read_only_mask & IMAGE_READ_ONLY_FLAG_NON_PRIMARY) != 0U)) {
+    // implies we opened a non-primary image in R/W mode
+    ldout(cct, 5) << "adding non-primary read-only image flag" << dendl;
+    m_read_only_flags |= IMAGE_READ_ONLY_FLAG_NON_PRIMARY;
+  } else if ((((m_incompatible_features & RBD_FEATURE_NON_PRIMARY) == 0U) ||
+              ((m_image_ctx.read_only_mask &
+                  IMAGE_READ_ONLY_FLAG_NON_PRIMARY) == 0U)) &&
+             ((m_read_only_flags & IMAGE_READ_ONLY_FLAG_NON_PRIMARY) != 0U)) {
+    ldout(cct, 5) << "removing non-primary read-only image flag" << dendl;
+    m_read_only_flags &= ~IMAGE_READ_ONLY_FLAG_NON_PRIMARY;
+  }
+  m_read_only = (m_read_only_flags != 0U);
+
   send_v2_get_parent();
   return nullptr;
 }
@@ -804,7 +830,7 @@ Context *RefreshRequest<I>::handle_v2_refresh_parent(int *result) {
 template <typename I>
 void RefreshRequest<I>::send_v2_init_exclusive_lock() {
   if ((m_features & RBD_FEATURE_EXCLUSIVE_LOCK) == 0 ||
-      m_image_ctx.read_only || !m_image_ctx.snap_name.empty() ||
+      m_read_only || !m_image_ctx.snap_name.empty() ||
       m_image_ctx.exclusive_lock != nullptr) {
     send_v2_open_object_map();
     return;
@@ -846,7 +872,7 @@ template <typename I>
 void RefreshRequest<I>::send_v2_open_journal() {
   bool journal_disabled = (
     (m_features & RBD_FEATURE_JOURNALING) == 0 ||
-     m_image_ctx.read_only ||
+     m_read_only ||
      !m_image_ctx.snap_name.empty() ||
      m_image_ctx.journal != nullptr ||
      m_image_ctx.exclusive_lock == nullptr ||
@@ -948,7 +974,7 @@ void RefreshRequest<I>::send_v2_open_object_map() {
   if ((m_features & RBD_FEATURE_OBJECT_MAP) == 0 ||
       m_image_ctx.object_map != nullptr ||
       (m_image_ctx.snap_name.empty() &&
-       (m_image_ctx.read_only ||
+       (m_read_only ||
         m_image_ctx.exclusive_lock == nullptr ||
         !m_image_ctx.exclusive_lock->is_lock_owner()))) {
     send_v2_open_journal();
@@ -1234,6 +1260,8 @@ void RefreshRequest<I>::apply() {
 
   std::scoped_lock locker{m_image_ctx.owner_lock, m_image_ctx.image_lock};
 
+  m_image_ctx.read_only_flags = m_read_only_flags;
+  m_image_ctx.read_only = m_read_only;
   m_image_ctx.size = m_size;
   m_image_ctx.lockers = m_lockers;
   m_image_ctx.lock_tag = m_lock_tag;

diff --git a/src/librbd/image/RefreshRequest.h b/src/librbd/image/RefreshRequest.h
index bd3fda40fc56c847ffe1f51c018961ececaf2afc..74e80cf98b159733ce208f3ecb70d8b234c85e84 100644 (file)
--- a/src/librbd/image/RefreshRequest.h
+++ b/src/librbd/image/RefreshRequest.h
@@ -144,6 +144,8 @@ private:
   uint64_t m_incompatible_features = 0;
   uint64_t m_flags = 0;
   uint64_t m_op_features = 0;
+  uint32_t m_read_only_flags = 0U;
+  bool m_read_only = false;
 
   librados::IoCtx m_pool_metadata_io_ctx;
   std::map<std::string, bufferlist> m_metadata;

diff --git a/src/test/librbd/image/test_mock_DetachChildRequest.cc b/src/test/librbd/image/test_mock_DetachChildRequest.cc
index 5fecd6a23e3f713c3ec5d2cdfe5883107e3b5eae..cd62099c00ff9f87e748942fa1b8fa7b9a0bf382 100644 (file)
--- a/src/test/librbd/image/test_mock_DetachChildRequest.cc
+++ b/src/test/librbd/image/test_mock_DetachChildRequest.cc
@@ -158,7 +158,9 @@ public:
 
   void expect_open(MockImageCtx &mock_image_ctx, int r) {
     EXPECT_CALL(*mock_image_ctx.state, open(true, _))
-      .WillOnce(WithArg<1>(Invoke([this, r](Context* ctx) {
+      .WillOnce(WithArg<1>(Invoke([this, &mock_image_ctx, r](Context* ctx) {
+                             EXPECT_EQ(0U, mock_image_ctx.read_only_mask &
+                                             IMAGE_READ_ONLY_FLAG_NON_PRIMARY);
                              image_ctx->op_work_queue->queue(ctx, r);
                            })));
     if (r == 0) {

diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc
index 0b35e6c0529b8572eb7936b3084c63af1fa10ca8..b871561e9887f2f0bf181925c683f7cea0d73734 100644 (file)
--- a/src/test/librbd/image/test_mock_RefreshRequest.cc
+++ b/src/test/librbd/image/test_mock_RefreshRequest.cc
@@ -1453,5 +1453,66 @@ TEST_F(TestMockImageRefreshRequest, ApplyMetadataError) {
   ASSERT_EQ(0, ctx.wait());
 }
 
+TEST_F(TestMockImageRefreshRequest, NonPrimaryFeature) {
+  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);
+
+  InSequence seq;
+
+  // ensure the image is put into read-only mode
+  expect_get_mutable_metadata(mock_image_ctx,
+                              ictx->features | RBD_FEATURE_NON_PRIMARY, 0);
+  expect_get_parent(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_refresh_parent_is_required(mock_refresh_parent_request, false);
+
+  C_SaferCond ctx1;
+  auto req = new MockRefreshRequest(mock_image_ctx, false, false, &ctx1);
+  req->send();
+
+  ASSERT_EQ(0, ctx1.wait());
+  ASSERT_TRUE(mock_image_ctx.read_only);
+  ASSERT_EQ(IMAGE_READ_ONLY_FLAG_NON_PRIMARY, mock_image_ctx.read_only_flags);
+
+  // try again but permit R/W against non-primary image
+  mock_image_ctx.read_only_mask = ~IMAGE_READ_ONLY_FLAG_NON_PRIMARY;
+
+  expect_get_mutable_metadata(mock_image_ctx,
+                              ictx->features | RBD_FEATURE_NON_PRIMARY, 0);
+  expect_get_parent(mock_image_ctx, 0);
+  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_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 ctx2;
+  req = new MockRefreshRequest(mock_image_ctx, false, false, &ctx2);
+  req->send();
+
+  ASSERT_EQ(0, ctx2.wait());
+  ASSERT_FALSE(mock_image_ctx.read_only);
+  ASSERT_EQ(0U, mock_image_ctx.read_only_flags);
+}
+
 } // namespace image
 } // namespace librbd

diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h
index 32097393ea3340a89e75db10aa4e51c8bf40f989..dcd620abd76c7d0b15ca113b57446910d9f226b1 100644 (file)
--- a/src/test/librbd/mock/MockImageCtx.h
+++ b/src/test/librbd/mock/MockImageCtx.h
@@ -56,6 +56,8 @@ struct MockImageCtx {
       snap_ids(image_ctx.snap_ids),
       old_format(image_ctx.old_format),
       read_only(image_ctx.read_only),
+      read_only_flags(image_ctx.read_only_flags),
+      read_only_mask(image_ctx.read_only_mask),
       clone_copy_on_read(image_ctx.clone_copy_on_read),
       lockers(image_ctx.lockers),
       exclusive_locked(image_ctx.exclusive_locked),
@@ -235,6 +237,8 @@ struct MockImageCtx {
 
   bool old_format;
   bool read_only;
+  uint32_t read_only_flags;
+  uint32_t read_only_mask;
 
   bool clone_copy_on_read;