From: Jason Dillaman <dillaman@redhat.com>
Date: Tue, 8 May 2018 20:25:42 +0000 (-0400)
Subject: rbd-mirror: policy map shouldn't default to first instance if dead
X-Git-Tag: v13.1.1~1^2~20
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=1f398ad147d479406ba816489ea212d396be85ee;p=ceph.git

rbd-mirror: policy map shouldn't default to first instance if dead

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
(cherry picked from commit 524f08cef697c94b331d52b2077c09a0b3ff9d2e)
---

diff --git a/src/test/rbd_mirror/image_map/test_Policy.cc b/src/test/rbd_mirror/image_map/test_Policy.cc
index 5d015537113c..f2907ecf363c 100644
--- a/src/test/rbd_mirror/image_map/test_Policy.cc
+++ b/src/test/rbd_mirror/image_map/test_Policy.cc
@@ -144,6 +144,9 @@ TEST_F(TestImageMapPolicy, ShuffleImageRemoveInstance) {
     "global id 1", "global id 2", "global id 3", "global id 4", "global id 5"
   };
 
+  std::set<std::string> shuffle_global_image_ids;
+  m_policy->add_instances({stringify(m_local_io_ctx.get_instance_id())},
+                          &shuffle_global_image_ids);
   for (auto const &global_image_id : global_image_ids) {
     // map image
     map_image(global_image_id);
@@ -152,7 +155,6 @@ TEST_F(TestImageMapPolicy, ShuffleImageRemoveInstance) {
     ASSERT_TRUE(info.instance_id != UNMAPPED_INSTANCE_ID);
   }
 
-  std::set<std::string> shuffle_global_image_ids;
   m_policy->add_instances({"9876"}, &shuffle_global_image_ids);
 
   for (auto const &global_image_id : shuffle_global_image_ids) {
@@ -247,6 +249,9 @@ TEST_F(TestImageMapPolicy, ReshuffleWithMapFailure) {
     "global id 6"
   };
 
+  std::set<std::string> shuffle_global_image_ids;
+  m_policy->add_instances({stringify(m_local_io_ctx.get_instance_id())},
+                          &shuffle_global_image_ids);
   for (auto const &global_image_id : global_image_ids) {
     // map image
     map_image(global_image_id);
@@ -255,7 +260,6 @@ TEST_F(TestImageMapPolicy, ReshuffleWithMapFailure) {
     ASSERT_TRUE(info.instance_id != UNMAPPED_INSTANCE_ID);
   }
 
-  std::set<std::string> shuffle_global_image_ids;
   m_policy->add_instances({"9876"}, &shuffle_global_image_ids);
   ASSERT_FALSE(shuffle_global_image_ids.empty());
 
@@ -292,6 +296,9 @@ TEST_F(TestImageMapPolicy, ShuffleFailureAndRemove) {
     "global id 6"
   };
 
+  std::set<std::string> shuffle_global_image_ids;
+  m_policy->add_instances({stringify(m_local_io_ctx.get_instance_id())},
+                          &shuffle_global_image_ids);
   for (auto const &global_image_id : global_image_ids) {
     // map image
     map_image(global_image_id);
@@ -300,7 +307,6 @@ TEST_F(TestImageMapPolicy, ShuffleFailureAndRemove) {
     ASSERT_TRUE(info.instance_id != UNMAPPED_INSTANCE_ID);
   }
 
-  std::set<std::string> shuffle_global_image_ids;
   m_policy->add_instances({"9876"}, &shuffle_global_image_ids);
   ASSERT_FALSE(shuffle_global_image_ids.empty());
 
diff --git a/src/tools/rbd_mirror/image_map/SimplePolicy.cc b/src/tools/rbd_mirror/image_map/SimplePolicy.cc
index 3c817e2eaf22..a606487adf1a 100644
--- a/src/tools/rbd_mirror/image_map/SimplePolicy.cc
+++ b/src/tools/rbd_mirror/image_map/SimplePolicy.cc
@@ -67,15 +67,19 @@ void SimplePolicy::do_shuffle_add_instances(
 
 std::string SimplePolicy::do_map(const InstanceToImageMap& map,
                                  const std::string &global_image_id) {
-  auto min_it = map.begin();
-  for (auto it = min_it; it != map.end(); ++it) {
+  auto min_it = map.end();
+  for (auto it = map.begin(); it != map.end(); ++it) {
     assert(it->second.find(global_image_id) == it->second.end());
-    if (it->second.size() < min_it->second.size() &&
-        !Policy::is_dead_instance(it->first)) {
+    if (Policy::is_dead_instance(it->first)) {
+      continue;
+    } else if (min_it == map.end()) {
+      min_it = it;
+    } else if (it->second.size() < min_it->second.size()) {
       min_it = it;
     }
   }
 
+  assert(min_it != map.end());
   dout(20) << "global_image_id=" << global_image_id << " maps to instance_id="
            << min_it->first << dendl;
   return min_it->first;