From: Joao Eduardo Luis <joao@redhat.com>
Date: Fri, 17 Oct 2014 16:43:33 +0000 (+0100)
Subject: mon: MDSMonitor: check all conditions are met *before* osdmon proposal
X-Git-Tag: v0.88~56^2~3
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=1d1ae413e9f0aafa314af903bcf6b815f6558082;p=ceph.git

mon: MDSMonitor: check all conditions are met *before* osdmon proposal

We should not allow ourselves to request the osdmon to propose before we
know for sure that we meet the required conditions to go through with
our own state change.  Even if we still can't guarantee that our
proposal is going to be committed, we shouldn't just change the osdmon's
state just because we can.  This way, at least, we make sure that our
checks hold up before doing anything with side-effects.

Signed-off-by: Joao Eduardo Luis <joao@redhat.com>
---

diff --git a/src/mon/MDSMonitor.cc b/src/mon/MDSMonitor.cc
index dd3a6698e5ce1..aa20d933a0fdd 100644
--- a/src/mon/MDSMonitor.cc
+++ b/src/mon/MDSMonitor.cc
@@ -1117,14 +1117,11 @@ bool MDSMonitor::management_command(
     pg_pool_t const *metadata_pool = mon->osdmon()->osdmap.get_pg_pool(metadata);
     assert(metadata_pool != NULL);  // Checked it existed above
 
-    // Automatically set crash_replay_interval on data pool if it
-    // isn't already set.
-    if (data_pool->get_crash_replay_interval() == 0) {
-      r = mon->osdmon()->set_crash_replay_interval(data, g_conf->osd_default_data_pool_replay_window);
-      assert(r == 0);  // We just did get_pg_pool so it must exist and be settable
-      request_proposal(mon->osdmon());
-    }
-
+    // we must make these checks before we even allow ourselves to *think*
+    // about requesting a proposal to the osdmonitor and bail out now if
+    // we believe we must.  bailing out *after* we request the proposal is
+    // bad business as we could have changed the osdmon's state and ending up
+    // returning an error to the user.
     r = _check_pool(data, &ss);
     if (r) {
       return true;
@@ -1135,6 +1132,14 @@ bool MDSMonitor::management_command(
       return true;
     }
 
+    // Automatically set crash_replay_interval on data pool if it
+    // isn't already set.
+    if (data_pool->get_crash_replay_interval() == 0) {
+      r = mon->osdmon()->set_crash_replay_interval(data, g_conf->osd_default_data_pool_replay_window);
+      assert(r == 0);  // We just did get_pg_pool so it must exist and be settable
+      request_proposal(mon->osdmon());
+    }
+
     // All checks passed, go ahead and create.
     MDSMap newmap;
     newmap.inc = pending_mdsmap.inc;