rbd-mirror: narrowed scope of exclusive lock op tracking

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

diff --git a/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc b/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc
index 370c25fb5555cb4745463105651a6ac684891089..f70df5c0582f58251047b442b06fda3c13fb9465 100644 (file)
--- a/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc
+++ b/src/test/rbd_mirror/image_sync/test_mock_ObjectCopyRequest.cc
@@ -46,6 +46,7 @@ using ::testing::DoDefault;
 using ::testing::InSequence;
 using ::testing::Invoke;
 using ::testing::Return;
+using ::testing::ReturnNew;
 using ::testing::WithArg;
 
 namespace {
@@ -88,6 +89,11 @@ public:
     ASSERT_EQ(0, open_image(m_local_io_ctx, m_image_name, &m_local_image_ctx));
   }
 
+  void expect_start_op(librbd::MockExclusiveLock &mock_exclusive_lock) {
+    EXPECT_CALL(mock_exclusive_lock, start_op()).WillOnce(
+      ReturnNew<FunctionContext>([](int) {}));
+  }
+
   void expect_list_snaps(librbd::MockTestImageCtx &mock_image_ctx,
                          librados::MockTestMemIoCtxImpl &mock_io_ctx,
                          const librados::snap_set_t &snap_set) {
@@ -346,9 +352,11 @@ TEST_F(TestMockImageSyncObjectCopyRequest, DNE) {
   librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx);
   librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
 
+  librbd::MockExclusiveLock mock_exclusive_lock;
+  mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock;
+
   librbd::MockObjectMap mock_object_map;
   mock_local_image_ctx.object_map = &mock_object_map;
-
   expect_test_features(mock_local_image_ctx);
 
   C_SaferCond ctx;
@@ -374,6 +382,9 @@ TEST_F(TestMockImageSyncObjectCopyRequest, Write) {
   librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx);
   librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
 
+  librbd::MockExclusiveLock mock_exclusive_lock;
+  mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock;
+
   librbd::MockObjectMap mock_object_map;
   mock_local_image_ctx.object_map = &mock_object_map;
 
@@ -392,7 +403,9 @@ TEST_F(TestMockImageSyncObjectCopyRequest, Write) {
   expect_list_snaps(mock_remote_image_ctx, mock_remote_io_ctx, 0);
   expect_set_snap_read(mock_remote_io_ctx, m_remote_snap_ids[0]);
   expect_sparse_read(mock_remote_io_ctx, 0, one.range_end(), 0);
+  expect_start_op(mock_exclusive_lock);
   expect_write(mock_local_io_ctx, 0, one.range_end(), {0, {}}, 0);
+  expect_start_op(mock_exclusive_lock);
   expect_update_object_map(mock_local_image_ctx, mock_object_map,
                            m_local_snap_ids[0], OBJECT_EXISTS, 0);
 
@@ -414,6 +427,9 @@ TEST_F(TestMockImageSyncObjectCopyRequest, ReadMissingStaleSnapSet) {
   librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx);
   librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
 
+  librbd::MockExclusiveLock mock_exclusive_lock;
+  mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock;
+
   librbd::MockObjectMap mock_object_map;
   mock_local_image_ctx.object_map = &mock_object_map;
 
@@ -449,12 +465,15 @@ TEST_F(TestMockImageSyncObjectCopyRequest, ReadMissingStaleSnapSet) {
   expect_list_snaps(mock_remote_image_ctx, mock_remote_io_ctx, 0);
   expect_set_snap_read(mock_remote_io_ctx, m_remote_snap_ids[3]);
   expect_sparse_read(mock_remote_io_ctx, 0, one.range_end(), 0);
+  expect_start_op(mock_exclusive_lock);
   expect_write(mock_local_io_ctx, 0, one.range_end(),
                {m_local_snap_ids[1], {m_local_snap_ids[1],
                                       m_local_snap_ids[0]}},
                 0);
+  expect_start_op(mock_exclusive_lock);
   expect_update_object_map(mock_local_image_ctx, mock_object_map,
                            m_local_snap_ids[2], OBJECT_EXISTS, 0);
+  expect_start_op(mock_exclusive_lock);
   expect_update_object_map(mock_local_image_ctx, mock_object_map,
                            m_local_snap_ids[3], OBJECT_EXISTS_CLEAN, 0);
 
@@ -472,6 +491,9 @@ TEST_F(TestMockImageSyncObjectCopyRequest, ReadMissingUpToDateSnapMap) {
   librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx);
   librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
 
+  librbd::MockExclusiveLock mock_exclusive_lock;
+  mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock;
+
   librbd::MockObjectMap mock_object_map;
   mock_local_image_ctx.object_map = &mock_object_map;
 
@@ -503,6 +525,9 @@ TEST_F(TestMockImageSyncObjectCopyRequest, ReadError) {
   librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx);
   librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
 
+  librbd::MockExclusiveLock mock_exclusive_lock;
+  mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock;
+
   librbd::MockObjectMap mock_object_map;
   mock_local_image_ctx.object_map = &mock_object_map;
 
@@ -533,6 +558,9 @@ TEST_F(TestMockImageSyncObjectCopyRequest, WriteError) {
   librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx);
   librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
 
+  librbd::MockExclusiveLock mock_exclusive_lock;
+  mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock;
+
   librbd::MockObjectMap mock_object_map;
   mock_local_image_ctx.object_map = &mock_object_map;
 
@@ -551,6 +579,7 @@ TEST_F(TestMockImageSyncObjectCopyRequest, WriteError) {
   expect_list_snaps(mock_remote_image_ctx, mock_remote_io_ctx, 0);
   expect_set_snap_read(mock_remote_io_ctx, m_remote_snap_ids[0]);
   expect_sparse_read(mock_remote_io_ctx, 0, one.range_end(), 0);
+  expect_start_op(mock_exclusive_lock);
   expect_write(mock_local_io_ctx, 0, one.range_end(), {0, {}}, -EINVAL);
 
   request->send();
@@ -577,6 +606,9 @@ TEST_F(TestMockImageSyncObjectCopyRequest, WriteSnaps) {
   librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx);
   librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
 
+  librbd::MockExclusiveLock mock_exclusive_lock;
+  mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock;
+
   librbd::MockObjectMap mock_object_map;
   mock_local_image_ctx.object_map = &mock_object_map;
 
@@ -595,15 +627,20 @@ TEST_F(TestMockImageSyncObjectCopyRequest, WriteSnaps) {
   expect_list_snaps(mock_remote_image_ctx, mock_remote_io_ctx, 0);
   expect_set_snap_read(mock_remote_io_ctx, m_remote_snap_ids[0]);
   expect_sparse_read(mock_remote_io_ctx, 0, one.range_end(), 0);
+  expect_start_op(mock_exclusive_lock);
   expect_write(mock_local_io_ctx, 0, one.range_end(), {0, {}}, 0);
   expect_set_snap_read(mock_remote_io_ctx, m_remote_snap_ids[2]);
   expect_sparse_read(mock_remote_io_ctx, two, 0);
+  expect_start_op(mock_exclusive_lock);
   expect_write(mock_local_io_ctx, two,
                {m_local_snap_ids[0], {m_local_snap_ids[0]}}, 0);
+  expect_start_op(mock_exclusive_lock);
   expect_update_object_map(mock_local_image_ctx, mock_object_map,
                            m_local_snap_ids[0], OBJECT_EXISTS, 0);
+  expect_start_op(mock_exclusive_lock);
   expect_update_object_map(mock_local_image_ctx, mock_object_map,
                            m_local_snap_ids[1], OBJECT_EXISTS, 0);
+  expect_start_op(mock_exclusive_lock);
   expect_update_object_map(mock_local_image_ctx, mock_object_map,
                            m_local_snap_ids[2], OBJECT_EXISTS_CLEAN, 0);
 
@@ -629,6 +666,9 @@ TEST_F(TestMockImageSyncObjectCopyRequest, Trim) {
   librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx);
   librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
 
+  librbd::MockExclusiveLock mock_exclusive_lock;
+  mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock;
+
   librbd::MockObjectMap mock_object_map;
   mock_local_image_ctx.object_map = &mock_object_map;
 
@@ -647,10 +687,14 @@ TEST_F(TestMockImageSyncObjectCopyRequest, Trim) {
   expect_list_snaps(mock_remote_image_ctx, mock_remote_io_ctx, 0);
   expect_set_snap_read(mock_remote_io_ctx, m_remote_snap_ids[0]);
   expect_sparse_read(mock_remote_io_ctx, 0, one.range_end(), 0);
+  expect_start_op(mock_exclusive_lock);
   expect_write(mock_local_io_ctx, 0, one.range_end(), {0, {}}, 0);
+  expect_start_op(mock_exclusive_lock);
   expect_truncate(mock_local_io_ctx, trim_offset, 0);
+  expect_start_op(mock_exclusive_lock);
   expect_update_object_map(mock_local_image_ctx, mock_object_map,
                            m_local_snap_ids[0], OBJECT_EXISTS, 0);
+  expect_start_op(mock_exclusive_lock);
   expect_update_object_map(mock_local_image_ctx, mock_object_map,
                            m_local_snap_ids[1], OBJECT_EXISTS, 0);
 
@@ -673,6 +717,9 @@ TEST_F(TestMockImageSyncObjectCopyRequest, Remove) {
   librbd::MockTestImageCtx mock_remote_image_ctx(*m_remote_image_ctx);
   librbd::MockTestImageCtx mock_local_image_ctx(*m_local_image_ctx);
 
+  librbd::MockExclusiveLock mock_exclusive_lock;
+  mock_local_image_ctx.exclusive_lock = &mock_exclusive_lock;
+
   librbd::MockObjectMap mock_object_map;
   mock_local_image_ctx.object_map = &mock_object_map;
 
@@ -691,10 +738,14 @@ TEST_F(TestMockImageSyncObjectCopyRequest, Remove) {
   expect_list_snaps(mock_remote_image_ctx, mock_remote_io_ctx, 0);
   expect_set_snap_read(mock_remote_io_ctx, m_remote_snap_ids[1]);
   expect_sparse_read(mock_remote_io_ctx, 0, one.range_end(), 0);
+  expect_start_op(mock_exclusive_lock);
   expect_write(mock_local_io_ctx, 0, one.range_end(), {0, {}}, 0);
+  expect_start_op(mock_exclusive_lock);
   expect_remove(mock_local_io_ctx, 0);
+  expect_start_op(mock_exclusive_lock);
   expect_update_object_map(mock_local_image_ctx, mock_object_map,
                            m_local_snap_ids[0], OBJECT_EXISTS, 0);
+  expect_start_op(mock_exclusive_lock);
   expect_update_object_map(mock_local_image_ctx, mock_object_map,
                            m_local_snap_ids[1], OBJECT_EXISTS_CLEAN, 0);
 

diff --git a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
index 92817b7a78850d57a98b308c6b487d1294cc74c6..cc59030878ba35846c0182e6756aaed1c88755f4 100644 (file)
--- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
+++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.cc
@@ -3,6 +3,7 @@
 
 #include "ObjectCopyRequest.h"
 #include "librados/snap_set_diff.h"
+#include "librbd/ExclusiveLock.h"
 #include "librbd/ObjectMap.h"
 #include "librbd/Utils.h"
 #include "common/errno.h"
@@ -210,6 +211,17 @@ void ObjectCopyRequest<I>::send_write_object() {
     }
   }
 
+  Context *finish_op_ctx;
+  {
+    RWLock::RLocker owner_locker(m_local_image_ctx->owner_lock);
+    finish_op_ctx = start_local_op(m_local_image_ctx->owner_lock);
+  }
+  if (finish_op_ctx == nullptr) {
+    derr << ": lost exclusive lock" << dendl;
+    finish(-EROFS);
+    return;
+  }
+
   dout(20) << ": "
            << "local_snap_seq=" << local_snap_seq << ", "
            << "local_snaps=" << local_snap_ids << dendl;
@@ -270,8 +282,11 @@ void ObjectCopyRequest<I>::send_write_object() {
     }
   }
 
-  librados::AioCompletion *comp = create_rados_callback<
-    ObjectCopyRequest<I>, &ObjectCopyRequest<I>::handle_write_object>(this);
+  auto ctx = new FunctionContext([this, finish_op_ctx](int r) {
+      handle_write_object(r);
+      finish_op_ctx->complete(0);
+    });
+  librados::AioCompletion *comp = create_rados_callback(ctx);
   int r = m_local_io_ctx.aio_operate(m_local_oid, comp, &op, local_snap_seq,
                                      local_snap_ids);
   assert(r == 0);
@@ -303,12 +318,13 @@ void ObjectCopyRequest<I>::handle_write_object(int r) {
 
 template <typename I>
 void ObjectCopyRequest<I>::send_update_object_map() {
-
+  m_local_image_ctx->owner_lock.get_read();
   m_local_image_ctx->snap_lock.get_read();
   if (!m_local_image_ctx->test_features(RBD_FEATURE_OBJECT_MAP,
                                         m_local_image_ctx->snap_lock) ||
       m_snap_object_states.empty()) {
     m_local_image_ctx->snap_lock.put_read();
+    m_local_image_ctx->owner_lock.put_read();
     finish(0);
     return;
   } else if (m_local_image_ctx->object_map == nullptr) {
@@ -316,6 +332,7 @@ void ObjectCopyRequest<I>::send_update_object_map() {
     derr << ": object map is not initialized" << dendl;
 
     m_local_image_ctx->snap_lock.put_read();
+    m_local_image_ctx->owner_lock.put_read();
     finish(-EINVAL);
     return;
   }
@@ -330,13 +347,28 @@ void ObjectCopyRequest<I>::send_update_object_map() {
            << "object_state=" << static_cast<uint32_t>(snap_object_state.second)
            << dendl;
 
+  auto finish_op_ctx = start_local_op(m_local_image_ctx->owner_lock);
+  if (finish_op_ctx == nullptr) {
+    derr << ": lost exclusive lock" << dendl;
+    m_local_image_ctx->snap_lock.put_read();
+    m_local_image_ctx->owner_lock.put_read();
+    finish(-EROFS);
+    return;
+  }
+
+  auto ctx = new FunctionContext([this, finish_op_ctx](int r) {
+      handle_update_object_map(r);
+      finish_op_ctx->complete(0);
+    });
+
   RWLock::WLocker object_map_locker(m_local_image_ctx->object_map_lock);
   bool sent = m_local_image_ctx->object_map->template aio_update<
-    ObjectCopyRequest<I>, &ObjectCopyRequest<I>::handle_update_object_map>(
+    Context, &Context::complete>(
       snap_object_state.first, m_object_number, snap_object_state.second, {},
-      {}, this);
+      {}, ctx);
   assert(sent);
   m_local_image_ctx->snap_lock.put_read();
+  m_local_image_ctx->owner_lock.put_read();
 }
 
 template <typename I>
@@ -351,6 +383,15 @@ void ObjectCopyRequest<I>::handle_update_object_map(int r) {
   finish(0);
 }
 
+template <typename I>
+Context *ObjectCopyRequest<I>::start_local_op(RWLock &owner_lock) {
+  assert(m_local_image_ctx->owner_lock.is_locked());
+  if (m_local_image_ctx->exclusive_lock == nullptr) {
+    return nullptr;
+  }
+  return m_local_image_ctx->exclusive_lock->start_op();
+}
+
 template <typename I>
 void ObjectCopyRequest<I>::compute_diffs() {
   CephContext *cct = m_local_image_ctx->cct;

diff --git a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.h b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.h
index 78068861a5c0caf1d1247a4c95b58e977e5f58cc..8430e7030c43933fb299222d7ac5d0fdea085ccd 100644 (file)
--- a/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.h
+++ b/src/tools/rbd_mirror/image_sync/ObjectCopyRequest.h
@@ -14,6 +14,7 @@
 #include <vector>
 
 class Context;
+class RWLock;
 
 namespace rbd {
 namespace mirror {
@@ -136,6 +137,8 @@ private:
   void send_update_object_map();
   void handle_update_object_map(int r);
 
+  Context *start_local_op(RWLock &owner_lock);
+
   void compute_diffs();
   void finish(int r);