rbd-mirror: fix crashes in unittest_rbd_mirror

Signed-off-by: Vinay Bhaskar Varada <vvarada@redhat.com>

diff --git a/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc b/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc
index c4de9c50214b5eeffd4f023e5a4be044ce344971..49b1399642107e072931f00798ae29cad197958c 100644 (file)
--- a/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc
+++ b/src/test/rbd_mirror/image_replayer/snapshot/test_mock_Replayer.cc
@@ -158,15 +158,14 @@ struct CreateNonPrimaryRequest<MockTestImageCtx> {
   static CreateNonPrimaryRequest* s_instance;
   static CreateNonPrimaryRequest* create(MockTestImageCtx *image_ctx,
                                          bool demoted,
+                                         const std::string group_id,
+                                         const std::string group_snap_id,
                                          const std::string &primary_mirror_uuid,
                                          uint64_t primary_snap_id,
                                          const SnapSeqs& snap_seqs,
-                                         uint64_t local_group_pool_id,
-                                         const std::string &local_group_id,
-                                         const std::string &local_group_snap_id,
                                          const ImageState &image_state,
                                          uint64_t *snap_id,
-                                         Context *on_finish) {
+                                         Context *on_finish){
     ceph_assert(s_instance != nullptr);
     s_instance->demoted = demoted;
     s_instance->primary_mirror_uuid = primary_mirror_uuid;

diff --git a/src/test/rbd_mirror/test_mock_ImageReplayer.cc b/src/test/rbd_mirror/test_mock_ImageReplayer.cc
index 0a0f2245477c312e07178aabf628ccfdbc4ff294..7b8c17c6c7b11ec3fdc10794a17375270f8c4e46 100644 (file)
--- a/src/test/rbd_mirror/test_mock_ImageReplayer.cc
+++ b/src/test/rbd_mirror/test_mock_ImageReplayer.cc
@@ -177,6 +177,9 @@ struct MockReplayer : public Replayer {
   MOCK_CONST_METHOD0(is_resync_requested, bool());
   MOCK_CONST_METHOD0(get_error_code, int());
   MOCK_CONST_METHOD0(get_error_description, std::string());
+  MOCK_METHOD1(prune_snapshot, void(uint64_t snapshot_id));
+  MOCK_METHOD1(set_remote_snap_id_end_limit, void(uint64_t snapshot_id));
+  MOCK_METHOD0(get_remote_snap_id_end_limit, uint64_t());
 };
 
 template <>

diff --git a/src/test/rbd_mirror/test_mock_InstanceReplayer.cc b/src/test/rbd_mirror/test_mock_InstanceReplayer.cc
index d5fae11501b396eb60ca830adef0357b5fc39d75..57138c56942fc034586e8ab35e083265736c1e17 100644 (file)
--- a/src/test/rbd_mirror/test_mock_InstanceReplayer.cc
+++ b/src/test/rbd_mirror/test_mock_InstanceReplayer.cc
@@ -89,7 +89,7 @@ struct GroupReplayer<librbd::MockTestImageCtx> {
   }
 
   MOCK_METHOD0(destroy, void());
-  MOCK_METHOD4(start, void(Context *, bool, bool, bool));
+  MOCK_METHOD3(start, void(Context *, bool, bool));
   MOCK_METHOD2(stop, void(Context *, bool));
   MOCK_METHOD2(restart, void(Context*, bool));
   MOCK_METHOD0(flush, void());
@@ -197,7 +197,8 @@ public:
 
   void expect_work_queue(MockThreads &mock_threads) {
     EXPECT_CALL(*mock_threads.work_queue, queue(_, _))
-      .WillOnce(Invoke([this](Context *ctx, int r) {
+      .Times(testing::AtLeast(1))
+      .WillRepeatedly(Invoke([this](Context *ctx, int r) {
           m_threads->work_queue->queue(ctx, r);
         }));
   }
@@ -224,7 +225,8 @@ public:
 
   void expect_cancel_event(MockThreads &mock_threads, bool canceled) {
     EXPECT_CALL(*mock_threads.timer, cancel_event(_))
-      .WillOnce(Return(canceled));
+      .Times(testing::AtLeast(1))
+      .WillRepeatedly(Return(canceled));
   }
 };
 
@@ -434,6 +436,7 @@ TEST_F(TestMockInstanceReplayer, Reacquire) {
 
   expect_work_queue(mock_threads);
   expect_cancel_event(mock_threads, true);
+  expect_work_queue(mock_threads);
   EXPECT_CALL(mock_image_replayer, is_stopped()).WillOnce(Return(true));
   expect_work_queue(mock_threads);
   expect_work_queue(mock_threads);

diff --git a/src/tools/rbd_mirror/ImageMap.cc b/src/tools/rbd_mirror/ImageMap.cc
index 932b9db6e8ba83c39f9606357d4ecbc48d8e0935..9ec991a6d9f67fadb8beb59c707f0b3514fa23b1 100644 (file)
--- a/src/tools/rbd_mirror/ImageMap.cc
+++ b/src/tools/rbd_mirror/ImageMap.cc
@@ -211,6 +211,14 @@ void ImageMap<I>::schedule_update_task() {
   schedule_update_task(m_threads->timer_lock, after);
 }
 
+template <typename I>
+void ImageMap<I>::schedule_update_task(const ceph::mutex &timer_lock) {
+  ceph_assert(ceph_mutex_is_locked(m_threads->timer_lock));
+  CephContext *cct = reinterpret_cast<CephContext *>(m_ioctx.cct());
+  double after = cct->_conf.get_val<double>("rbd_mirror_image_policy_update_throttle_interval");
+  schedule_update_task(m_threads->timer_lock, after);
+}
+
 template <typename I>
 void ImageMap<I>::schedule_update_task(const ceph::mutex &timer_lock,
                                        double after) {
@@ -259,7 +267,7 @@ void ImageMap<I>::rebalance() {
     }
   }
 
-  schedule_update_task();
+  schedule_update_task(m_threads->timer_lock);
 }
 
 template <typename I>
@@ -422,7 +430,7 @@ void ImageMap<I>::update_images_added(
   for (auto &entity : entities) {
     auto global_id = GlobalId(entity.type, entity.global_id);
     auto result = m_peer_map[global_id].insert(mirror_uuid);
-    if ((result.second || entity.type == MIRROR_ENTITY_TYPE_GROUP)) {
+    if ((result.second && m_peer_map[global_id].size() == 1) || entity.type == MIRROR_ENTITY_TYPE_GROUP) {
       if (m_policy->add_entity(global_id, entity.count)) {
         schedule_action(global_id);
       }

diff --git a/src/tools/rbd_mirror/ImageMap.h b/src/tools/rbd_mirror/ImageMap.h
index aac7299ef3b37198a6553ebf14fa9f2dd2de9c20..1cd99cc49de2af40bcfa6914623ed28583f38c84 100644 (file)
--- a/src/tools/rbd_mirror/ImageMap.h
+++ b/src/tools/rbd_mirror/ImageMap.h
@@ -148,6 +148,7 @@ private:
   void schedule_action(const image_map::GlobalId &global_id);
 
   void schedule_update_task();
+  void schedule_update_task(const ceph::mutex &timer_lock);
   void schedule_update_task(const ceph::mutex &timer_lock, double after);
   void process_updates();
   void update_image_mapping(Updates&& map_updates,

diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc
index 9ec215145bcd103ebfcdb8ad6df896626deaeb20..3c8317ac9a1fa36d95d2280702eda9d05dccfa02 100644 (file)
--- a/src/tools/rbd_mirror/ImageReplayer.cc
+++ b/src/tools/rbd_mirror/ImageReplayer.cc
@@ -1025,14 +1025,6 @@ void ImageReplayer<I>::handle_shut_down(int r) {
     return;
   }
 
-  if (!m_status_removed) {
-    auto ctx = new LambdaContext([this, r](int) {
-      m_status_removed = true;
-      handle_shut_down(r);
-    });
-    remove_image_status(m_delete_in_progress, ctx);
-    return;
-  }
 
   if (m_local_group_ctx != nullptr) {
     if (m_local_status_updater->mirror_group_image_exists(
@@ -1062,25 +1054,12 @@ void ImageReplayer<I>::handle_shut_down(int r) {
       return;
     }
   } else {
-    if (m_local_status_updater->mirror_image_exists(m_global_image_id)) {
-      dout(15) << "removing local mirror image status" << dendl;
+    if (!m_status_removed) {
       auto ctx = new LambdaContext([this, r](int) {
-          handle_shut_down(r);
-        });
-      m_local_status_updater->remove_mirror_image_status(m_global_image_id,
-                                                         true, ctx);
-      return;
-    }
-
-    if (m_remote_image_peer.mirror_status_updater != nullptr &&
-        m_remote_image_peer.mirror_status_updater->mirror_image_exists(
-            m_global_image_id)) {
-      dout(15) << "removing remote mirror image status" << dendl;
-      auto ctx = new LambdaContext([this, r](int) {
-          handle_shut_down(r);
-        });
-      m_remote_image_peer.mirror_status_updater->remove_mirror_image_status(
-          m_global_image_id, true, ctx);
+        m_status_removed = true;
+        handle_shut_down(r);
+      });
+      remove_image_status(m_delete_in_progress, ctx);
       return;
     }
   }

diff --git a/src/tools/rbd_mirror/InstanceReplayer.cc b/src/tools/rbd_mirror/InstanceReplayer.cc
index ad49a35a57f6eb5610a32f5f3f711fd98ed90ba1..01e61a09e43a2ab80278050ac808f36cb474b489 100644 (file)
--- a/src/tools/rbd_mirror/InstanceReplayer.cc
+++ b/src/tools/rbd_mirror/InstanceReplayer.cc
@@ -607,9 +607,9 @@ void InstanceReplayer<I>::handle_stop_image_replayers(int r) {
     }
     m_image_replayers.clear();
 
-    std::swap(on_finish, m_on_shut_down);
   }
-  if (on_finish) {
+  if (--m_shutdown_counter == 0 ) {
+    std::swap(on_finish, m_on_shut_down);
     on_finish->complete(r);
   }
 }
@@ -803,9 +803,9 @@ void InstanceReplayer<I>::handle_stop_group_replayers(int r) {
     }
     m_group_replayers.clear();
 
-    std::swap(on_finish, m_on_shut_down);
   }
-  if (on_finish) {
+  if (--m_shutdown_counter == 0) {
+    std::swap(on_finish, m_on_shut_down);
     on_finish->complete(r);
   }
 }

diff --git a/src/tools/rbd_mirror/InstanceReplayer.h b/src/tools/rbd_mirror/InstanceReplayer.h
index f83a2ba688f8a78cfd5df4c1eda41782d9fd9b55..93907e0a1034fe8e06d65c87607acc02697658d3 100644 (file)
--- a/src/tools/rbd_mirror/InstanceReplayer.h
+++ b/src/tools/rbd_mirror/InstanceReplayer.h
@@ -125,6 +125,7 @@ private:
   Context *m_image_state_check_task = nullptr;
   Context *m_group_state_check_task = nullptr;
   Context *m_on_shut_down = nullptr;
+  std::atomic<int> m_shutdown_counter{2};
   bool m_manual_stop = false;
   bool m_blocklisted = false;