From 0c5b9a2bab28364dede808076de4f52c32b5851f Mon Sep 17 00:00:00 2001
From: John Spray <john.spray@redhat.com>
Date: Tue, 12 May 2015 17:24:58 +0100
Subject: [PATCH] mds: validate the state+rank in MDS map

Especially:
 * once I have been assigned a rank, it
can't be taken away without restarting
the daemon.
 * once I have entered standby, I can
only go upwards through the states.

Fixes: #11481
Signed-off-by: John Spray <john.spray@redhat.com>
---
 src/mds/MDS.cc | 112 +++++++++++++++++++++++++++++++------------------
 1 file changed, 72 insertions(+), 40 deletions(-)

diff --git a/src/mds/MDS.cc b/src/mds/MDS.cc
index 61f977ea3ae..52ccf1f54ab 100644
--- a/src/mds/MDS.cc
+++ b/src/mds/MDS.cc
@@ -1567,11 +1567,53 @@ void MDS::handle_mds_map(MMDSMap *m)
   // see who i am
   addr = messenger->get_myaddr();
   whoami = mdsmap->get_rank_gid(mds_gid_t(monc->get_global_id()));
+  if (whoami == MDS_RANK_NONE && (
+      state == MDSMap::STATE_STANDBY_REPLAY || state == MDSMap::STATE_ONESHOT_REPLAY)) {
+    whoami = mdsmap->get_mds_info_gid(mds_gid_t(monc->get_global_id())).standby_for_rank;
+  }
+
   state = mdsmap->get_state_gid(mds_gid_t(monc->get_global_id()));
   incarnation = mdsmap->get_inc_gid(mds_gid_t(monc->get_global_id()));
   dout(10) << "map says i am " << addr << " mds." << whoami << "." << incarnation
 	   << " state " << ceph_mds_state_name(state) << dendl;
 
+  // Once I hold a rank it can't be taken away without
+  // restarting this daemon
+  if (whoami != oldwhoami && oldwhoami != MDS_RANK_NONE) {
+    derr << "Invalid rank transition " << oldwhoami << "->" << whoami << dendl;
+    respawn();
+  }
+
+  // Validate state transitions while I hold a rank
+  {
+    bool state_valid = true;
+    if (whoami != MDS_RANK_NONE && state != oldstate) {
+      if (oldstate == MDSMap::STATE_REPLAY) {
+        if (state != MDSMap::STATE_RESOLVE && state != MDSMap::STATE_RECONNECT) {
+          state_valid = false;
+        }
+      } else if (oldstate == MDSMap::STATE_REJOIN) {
+        if (state != MDSMap::STATE_ACTIVE
+            && state != MDSMap::STATE_CLIENTREPLAY
+            && state != MDSMap::STATE_STOPPED) {
+          state_valid = false;
+        }
+      } else if (oldstate >= MDSMap::STATE_RECONNECT && oldstate < MDSMap::STATE_ACTIVE) {
+        // Once I have entered replay, the only allowable transitions are to
+        // the next state along in the sequence.
+        if (state != oldstate + 1) {
+          state_valid = false;
+        }
+      }
+    }
+
+    if (!state_valid) {
+      derr << "Invalid state transition " << ceph_mds_state_name(oldstate)
+        << "->" << ceph_mds_state_name(state) << dendl;
+      respawn();
+    }
+  }
+
   // mark down any failed peers
   for (map<mds_gid_t,MDSMap::mds_info_t>::const_iterator p = oldmap->get_mds_info().begin();
        p != oldmap->get_mds_info().end();
@@ -1603,48 +1645,38 @@ void MDS::handle_mds_map(MMDSMap *m)
     }
   }
 
-  if (whoami < 0) {
-    if (state == MDSMap::STATE_STANDBY_REPLAY ||
-        state == MDSMap::STATE_ONESHOT_REPLAY) {
-      // fill in whoami from standby-for-rank. If we let this be changed
-      // the logic used to set it here will need to be adjusted.
-      whoami = mdsmap->get_mds_info_gid(mds_gid_t(monc->get_global_id())).standby_for_rank;
-    } else {
-      if (want_state == MDSMap::STATE_STANDBY) {
-        dout(10) << "dropped out of mdsmap, try to re-add myself" << dendl;
-        state = MDSMap::STATE_BOOT;
-        set_want_state(state);
-        goto out;
-      }
-      if (want_state == MDSMap::STATE_BOOT) {
-        dout(10) << "not in map yet" << dendl;
-      } else {
-	// did i get kicked by someone else?
-	if (g_conf->mds_enforce_unique_name) {
-	  if (mds_gid_t existing = mdsmap->find_mds_gid_by_name(name)) {
-	    MDSMap::mds_info_t& i = mdsmap->get_info_gid(existing);
-	    if (i.global_id > monc->get_global_id()) {
-	      dout(1) << "handle_mds_map i (" << addr
-		      << ") dne in the mdsmap, new instance has larger gid " << i.global_id
-		      << ", suicide" << dendl;
-              // Call suicide() rather than respawn() because if someone else
-              // has taken our ID, we don't want to keep restarting and
-              // fighting them for the ID.
-	      suicide();
-	      goto out;
-	    }
-	  }
-	}
-
-        dout(1) << "handle_mds_map i (" << addr
-            << ") dne in the mdsmap, respawning myself" << dendl;
-        respawn();
-      }
+  if (whoami == MDS_RANK_NONE) {
+    if (want_state == MDSMap::STATE_STANDBY) {
+      dout(10) << "dropped out of mdsmap, try to re-add myself" << dendl;
+      state = MDSMap::STATE_BOOT;
+      set_want_state(state);
       goto out;
-    }
-  }
+    } else if (want_state == MDSMap::STATE_BOOT) {
+      dout(10) << "not in map yet" << dendl;
+    } else {
+      // did i get kicked by someone else?
+      if (g_conf->mds_enforce_unique_name) {
+        if (mds_gid_t existing = mdsmap->find_mds_gid_by_name(name)) {
+          MDSMap::mds_info_t& i = mdsmap->get_info_gid(existing);
+          if (i.global_id > monc->get_global_id()) {
+            dout(1) << "handle_mds_map i (" << addr
+                    << ") dne in the mdsmap, new instance has larger gid " << i.global_id
+                    << ", suicide" << dendl;
+            // Call suicide() rather than respawn() because if someone else
+            // has taken our ID, we don't want to keep restarting and
+            // fighting them for the ID.
+            suicide();
+            goto out;
+          }
+        }
+      }
 
-  // ??
+      dout(1) << "handle_mds_map i (" << addr
+          << ") dne in the mdsmap, respawning myself" << dendl;
+      respawn();
+    }
+    goto out;
+  }
 
   if (oldwhoami != whoami || oldstate != state) {
     // update messenger.