From c14c98d3f094d34907cc1fb97e7ef816f8dc0ba9 Mon Sep 17 00:00:00 2001
From: Joao Eduardo Luis <joao.luis@inktank.com>
Date: Fri, 25 Oct 2013 03:33:53 +0100
Subject: [PATCH] mon: OSDMonitor: Make 'osd pool rename' idempotent

'ceph osd pool rename' takes two arguments: source pool and dest pool.
If by chance 'source pool' does not exist and 'destination pool' does,
then, in order to assure it's idempotent, we want to assume that if
'source pool' no longer exists is because it was already renamed.

However, while we will return success in such case, we want to make sure
to let the user know that we made such assumption.  Mostly to warn the
user of such a thing in case of a mistake on the user's part (say, the
user didn't notice that the source pool didn't exist, while the dest did),
but also to make sure that the user is not surprised by the command
returning success if the user expected an ENOENT or EEXIST.

Fixes: #6635

Signed-off-by: Joao Eduardo Luis <joao.luis@inktank.com>
---
 src/mon/OSDMonitor.cc | 50 +++++++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 16 deletions(-)

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index 83e85847045..53f2e9491c7 100644
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -3784,25 +3784,43 @@ done:
     string srcpoolstr, destpoolstr;
     cmd_getval(g_ceph_context, cmdmap, "srcpool", srcpoolstr);
     cmd_getval(g_ceph_context, cmdmap, "destpool", destpoolstr);
-    int64_t pool = osdmap.lookup_pg_pool_name(srcpoolstr.c_str());
-    if (pool < 0) {
-      ss << "unrecognized pool '" << srcpoolstr << "'";
-      err = -ENOENT;
-    } else if (osdmap.lookup_pg_pool_name(destpoolstr.c_str()) >= 0) {
+    int64_t pool_src = osdmap.lookup_pg_pool_name(srcpoolstr.c_str());
+    int64_t pool_dst = osdmap.lookup_pg_pool_name(destpoolstr.c_str());
+
+    if (pool_src < 0) {
+      if (pool_dst >= 0) {
+        // src pool doesn't exist, dst pool does exist: to ensure idempotency
+        // of operations, assume this rename succeeded, as it is not changing
+        // the current state.  Make sure we output something understandable
+        // for whoever is issuing the command, if they are paying attention,
+        // in case it was not intentional; or to avoid a "wtf?" and a bug
+        // report in case it was intentional, while expecting a failure.
+        ss << "pool '" << srcpoolstr << "' does not exist; pool '"
+          << destpoolstr << "' does -- assuming successful rename";
+        err = 0;
+      } else {
+        ss << "unrecognized pool '" << srcpoolstr << "'";
+        err = -ENOENT;
+      }
+      goto reply;
+    } else if (pool_dst >= 0) {
+      // source pool exists and so does the destination pool
       ss << "pool '" << destpoolstr << "' already exists";
       err = -EEXIST;
-    } else {
-      int ret = _prepare_rename_pool(pool, destpoolstr);
-      if (ret == 0) {
-	ss << "pool '" << srcpoolstr << "' renamed to '" << destpoolstr << "'";
-      } else {
-	ss << "failed to rename pool '" << srcpoolstr << "' to '" << destpoolstr << "': "
-	   << cpp_strerror(ret);
-      }
-      getline(ss, rs);
-      wait_for_finished_proposal(new Monitor::C_Command(mon, m, ret, rs, get_last_committed()));
-      return true;
+      goto reply;
     }
+
+    int ret = _prepare_rename_pool(pool_src, destpoolstr);
+    if (ret == 0) {
+      ss << "pool '" << srcpoolstr << "' renamed to '" << destpoolstr << "'";
+    } else {
+      ss << "failed to rename pool '" << srcpoolstr << "' to '" << destpoolstr << "': "
+        << cpp_strerror(ret);
+    }
+    getline(ss, rs);
+    wait_for_finished_proposal(new Monitor::C_Command(mon, m, ret, rs, get_last_committed()));
+    return true;
+
   } else if (prefix == "osd pool set") {
     err = prepare_command_pool_set(cmdmap, ss);
     if (err < 0)