librbd: acquire exclusive-lock during copy on read

Fixes: http://tracker.ceph.com/issues/18888
Signed-off-by: Venky Shankar <vshankar@redhat.com>

diff --git a/src/librbd/image/RefreshRequest.cc b/src/librbd/image/RefreshRequest.cc
index bf64f9590c99d5a993792a9f306badce6601ce6d..01db2dde0e7f3a62c8e491021b01a4efa58f7446 100644 (file)
--- a/src/librbd/image/RefreshRequest.cc
+++ b/src/librbd/image/RefreshRequest.cc
@@ -1075,6 +1075,7 @@ void RefreshRequest<I>::apply() {
       // object map and journaling
       assert(m_exclusive_lock == nullptr);
       m_exclusive_lock = m_image_ctx.exclusive_lock;
+      m_image_ctx.io_work_queue->clear_require_lock_on_read();
     } else {
       if (m_exclusive_lock != nullptr) {
         assert(m_image_ctx.exclusive_lock == nullptr);
@@ -1094,6 +1095,10 @@ void RefreshRequest<I>::apply() {
           m_object_map != nullptr) {
         std::swap(m_object_map, m_image_ctx.object_map);
       }
+      if (m_image_ctx.clone_copy_on_read &&
+          m_image_ctx.io_work_queue->is_lock_required()) {
+        m_image_ctx.io_work_queue->set_require_lock_on_read();
+      }
     }
   }
 }

diff --git a/src/librbd/io/ImageRequestWQ.h b/src/librbd/io/ImageRequestWQ.h
index 8bf10be4825d1566e7e050becc583aaf4d2ea55a..a7b979e63f273392c8c8911a0619afddc5f5eb22 100644 (file)
--- a/src/librbd/io/ImageRequestWQ.h
+++ b/src/librbd/io/ImageRequestWQ.h
@@ -43,6 +43,7 @@ public:
 
   void shut_down(Context *on_shutdown);
 
+  bool is_lock_required() const;
   bool is_lock_request_needed() const;
 
   inline bool writes_blocked() const {
@@ -114,7 +115,6 @@ private:
   int start_in_flight_op(AioCompletion *c);
   void finish_in_flight_op();
 
-  bool is_lock_required() const;
   void queue(ImageRequest<ImageCtx> *req);
 
   void handle_refreshed(int r, ImageRequest<ImageCtx> *req);

diff --git a/src/test/librbd/image/test_mock_RefreshRequest.cc b/src/test/librbd/image/test_mock_RefreshRequest.cc
index 59932f2f850924762354a55a8bf19246c97d5117..ea5a5dd270599a3470a2334cf64cf08ebe72f03f 100644 (file)
--- a/src/test/librbd/image/test_mock_RefreshRequest.cc
+++ b/src/test/librbd/image/test_mock_RefreshRequest.cc
@@ -97,6 +97,10 @@ public:
   typedef RefreshRequest<MockRefreshImageCtx> MockRefreshRequest;
   typedef RefreshParentRequest<MockRefreshImageCtx> MockRefreshParentRequest;
 
+  void expect_is_lock_required(MockRefreshImageCtx &mock_image_ctx, bool require_lock) {
+    EXPECT_CALL(*mock_image_ctx.io_work_queue, is_lock_required()).WillOnce(Return(require_lock));
+  }
+
   void expect_set_require_lock_on_read(MockRefreshImageCtx &mock_image_ctx) {
     EXPECT_CALL(*mock_image_ctx.io_work_queue, set_require_lock_on_read());
   }
@@ -631,6 +635,7 @@ TEST_F(TestMockImageRefreshRequest, DisableExclusiveLock) {
   expect_get_flags(mock_image_ctx, 0);
   expect_get_group(mock_image_ctx, 0);
   expect_refresh_parent_is_required(mock_refresh_parent_request, false);
+  expect_clear_require_lock_on_read(mock_image_ctx);
   expect_shut_down_exclusive_lock(mock_image_ctx, *mock_exclusive_lock, 0);
 
   C_SaferCond ctx;
@@ -735,6 +740,58 @@ TEST_F(TestMockImageRefreshRequest, JournalDisabledByPolicy) {
   ASSERT_EQ(0, ctx.wait());
 }
 
+TEST_F(TestMockImageRefreshRequest, ExclusiveLockWithCoR) {
+  REQUIRE_FEATURE(RBD_FEATURE_EXCLUSIVE_LOCK);
+
+  std::string val;
+  ASSERT_EQ(0, _rados.conf_get("rbd_clone_copy_on_read", val));
+  if (val == "false") {
+    std::cout << "SKIPPING due to disabled rbd_copy_on_read" << std::endl;
+    return;
+  }
+
+  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;
+  mock_image_ctx.exclusive_lock = &mock_exclusive_lock;
+
+  if (ictx->test_features(RBD_FEATURE_JOURNALING)) {
+    ASSERT_EQ(0, ictx->operations->update_features(RBD_FEATURE_JOURNALING,
+                                                   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));
+  }
+
+  expect_op_work_queue(mock_image_ctx);
+  expect_test_features(mock_image_ctx);
+
+  InSequence seq;
+  expect_get_mutable_metadata(mock_image_ctx, 0);
+  expect_get_flags(mock_image_ctx, 0);
+  expect_get_group(mock_image_ctx, 0);
+  expect_refresh_parent_is_required(mock_refresh_parent_request, false);
+  expect_is_lock_required(mock_image_ctx, true);
+  expect_set_require_lock_on_read(mock_image_ctx);
+
+  C_SaferCond ctx;
+  MockRefreshRequest *req = new MockRefreshRequest(mock_image_ctx, false, false, &ctx);
+  req->send();
+
+  ASSERT_EQ(0, ctx.wait());
+}
+
 TEST_F(TestMockImageRefreshRequest, EnableJournalWithExclusiveLock) {
   REQUIRE_FEATURE(RBD_FEATURE_JOURNALING);
 

diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h
index 1b2354df1563c440948f7171a81157244dcb4894..61381114c3f4963efc8be1f1976311ac8d78a60e 100644 (file)
--- a/src/test/librbd/mock/MockImageCtx.h
+++ b/src/test/librbd/mock/MockImageCtx.h
@@ -51,6 +51,7 @@ struct MockImageCtx {
       object_set(image_ctx.object_set),
       old_format(image_ctx.old_format),
       read_only(image_ctx.read_only),
+      clone_copy_on_read(image_ctx.clone_copy_on_read),
       lockers(image_ctx.lockers),
       exclusive_locked(image_ctx.exclusive_locked),
       lock_tag(image_ctx.lock_tag),
@@ -208,6 +209,8 @@ struct MockImageCtx {
   bool old_format;
   bool read_only;
 
+  bool clone_copy_on_read;
+
   std::map<rados::cls::lock::locker_id_t,
            rados::cls::lock::locker_info_t> lockers;
   bool exclusive_locked;

diff --git a/src/test/librbd/mock/io/MockImageRequestWQ.h b/src/test/librbd/mock/io/MockImageRequestWQ.h
index 972b75f7421574d141941fb45a92cd2e70bc4d30..049d1bc233e0f4313529ce4f9cf2298a9cd9918a 100644 (file)
--- a/src/test/librbd/mock/io/MockImageRequestWQ.h
+++ b/src/test/librbd/mock/io/MockImageRequestWQ.h
@@ -18,6 +18,7 @@ struct MockImageRequestWQ {
   MOCK_METHOD0(set_require_lock_on_read, void());
   MOCK_METHOD0(clear_require_lock_on_read, void());
 
+  MOCK_CONST_METHOD0(is_lock_required, bool());
   MOCK_CONST_METHOD0(is_lock_request_needed, bool());
 };