mon/OSDMonitor: batch noup/noin osds support

This is a follow-up change of https://github.com/ceph/ceph/pull/15381.
This patch also simplifies the original code logic a bit.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>

qa/workunits/cephtool/test.sh

@@ -1421,28 +1421,46 @@ function test_mon_osd()
   ceph osd dump | grep 'osd.0.*in'
   ceph osd find 0
 
+  ceph osd add-nodown 0 1
+  ceph health detail | grep 'nodown osd(s).*0.*1'
+  ceph osd rm-nodown 0 1
+  ! ceph health detail | grep 'nodown osd(s).*0.*1'
+
+  ceph osd out 0 # so we can mark it as noin later
+  ceph osd add-noin 0
+  ceph health detail | grep 'noin osd(s).*0'
+  ceph osd rm-noin 0
+  ! ceph health detail | grep 'noin osd(s).*0'
+  ceph osd in 0
+
   ceph osd add-noout 0
   ceph health detail | grep 'noout osd(s).*0'
   ceph osd rm-noout 0
   ! ceph health detail | grep 'noout osds(s).*0'
 
-  ceph osd add-nodown 0 1
+  # test osd id parse
-  ceph health detail | grep 'nodown osd(s).*0.*1'
+  expect_false ceph osd add-noup 797er
-  ceph osd rm-nodown all
+  expect_false ceph osd add-nodown u9uwer
-  ! ceph health detail | grep 'nodown osd(s).*0.*1'
+  expect_false ceph osd add-noin 78~15
+  expect_false ceph osd add-noout 0 all 1
+
+  expect_false ceph osd rm-noup 1234567
+  expect_false ceph osd rm-nodown fsadf7
+  expect_false ceph osd rm-noin 0 1 any
+  expect_false ceph osd rm-noout 790-fd
 
   ids=`ceph osd ls-tree default`
   for osd in $ids
   do
-    ceph osd add-noout $osd
     ceph osd add-nodown $osd
+    ceph osd add-noout $osd
   done
-  ceph -s | grep 'noout osd(s)'
   ceph -s | grep 'nodown osd(s)'
-  ceph osd rm-noout any
+  ceph -s | grep 'noout osd(s)'
   ceph osd rm-nodown any
-  ! ceph -s | grep 'noout osd(s)'
+  ceph osd rm-noout all
   ! ceph -s | grep 'nodown osd(s)'
+  ! ceph -s | grep 'noout osd(s)'
 
   # make sure mark out preserves weight
   ceph osd reweight osd.0 .5

src/include/rados.h

@@ -118,8 +118,10 @@ struct ceph_eversion {
 #define CEPH_OSD_NEARFULL     (1<<5)  /* osd is at or above nearfull threshold */
 #define CEPH_OSD_BACKFILLFULL (1<<6)  /* osd is at or above backfillfull threshold */
 #define CEPH_OSD_DESTROYED    (1<<7)  /* osd has been destroyed */
-#define CEPH_OSD_NODOWN       (1<<8) /* osd can not be marked down */
+#define CEPH_OSD_NOUP         (1<<8)  /* osd can not be marked up */
-#define CEPH_OSD_NOOUT        (1<<9) /* osd can not be marked out */
+#define CEPH_OSD_NODOWN       (1<<9)  /* osd can not be marked down */
+#define CEPH_OSD_NOIN         (1<<10) /* osd can not be marked in */
+#define CEPH_OSD_NOOUT        (1<<11) /* osd can not be marked out */
 
 extern const char *ceph_osd_state_name(int s);
 

src/mon/MonCommands.h

@@ -687,21 +687,31 @@ COMMAND("osd rm " \
 	"remove osd(s) <id> [<id>...], "
         "or use <any|all|*> to remove all osds", \
         "osd", "rw", "cli,rest")
-COMMAND("osd add-noout " \
+COMMAND("osd add-noup " \
         "name=ids,type=CephString,n=N", \
-        "mark osd(s) <id> [<id>...] as noout, " \
+        "mark osd(s) <id> [<id>...] as noup, " \
-        "or use <all|any|*> to mark all osds as noout", \
+        "or use <all|any|*> to mark all osds as noup", \
         "osd", "rw", "cli,rest")
 COMMAND("osd add-nodown " \
         "name=ids,type=CephString,n=N", \
         "mark osd(s) <id> [<id>...] as nodown, " \
         "or use <all|any|*> to mark all osds as nodown", \
         "osd", "rw", "cli,rest")
-COMMAND("osd rm-noout " \
+COMMAND("osd add-noin " \
         "name=ids,type=CephString,n=N", \
-        "allow osd(s) <id> [<id>...] to be marked out " \
+        "mark osd(s) <id> [<id>...] as noin, " \
-        "(if they are currently marked as noout), " \
+        "or use <all|any|*> to mark all osds as noin", \
-        "can use <all|any|*> to automatically filter out all noout osds", \
+        "osd", "rw", "cli,rest")
+COMMAND("osd add-noout " \
+        "name=ids,type=CephString,n=N", \
+        "mark osd(s) <id> [<id>...] as noout, " \
+        "or use <all|any|*> to mark all osds as noout", \
+        "osd", "rw", "cli,rest")
+COMMAND("osd rm-noup " \
+        "name=ids,type=CephString,n=N", \
+        "allow osd(s) <id> [<id>...] to be marked up " \
+        "(if they are currently marked as noup), " \
+        "can use <all|any|*> to automatically filter out all noup osds", \
         "osd", "rw", "cli,rest")
 COMMAND("osd rm-nodown " \
         "name=ids,type=CephString,n=N", \
@@ -709,6 +719,18 @@ COMMAND("osd rm-nodown " \
         "(if they are currently marked as nodown), " \
         "can use <all|any|*> to automatically filter out all nodown osds", \
         "osd", "rw", "cli,rest")
+COMMAND("osd rm-noin " \
+        "name=ids,type=CephString,n=N", \
+        "allow osd(s) <id> [<id>...] to be marked in " \
+        "(if they are currently marked as noin), " \
+        "can use <all|any|*> to automatically filter out all noin osds", \
+        "osd", "rw", "cli,rest")
+COMMAND("osd rm-noout " \
+        "name=ids,type=CephString,n=N", \
+        "allow osd(s) <id> [<id>...] to be marked out " \
+        "(if they are currently marked as noout), " \
+        "can use <all|any|*> to automatically filter out all noout osds", \
+        "osd", "rw", "cli,rest")
 COMMAND("osd reweight " \
 	"name=id,type=CephOsdName " \
 	"type=CephFloat,name=weight,range=0.0|1.0", \

src/mon/OSDMonitor.cc

@@ -1623,9 +1623,17 @@ bool OSDMonitor::can_mark_down(int i)
 bool OSDMonitor::can_mark_up(int i)
 {
   if (osdmap.test_flag(CEPH_OSDMAP_NOUP)) {
-    dout(5) << "can_mark_up NOUP flag set, will not mark osd." << i << " up" << dendl;
+    dout(5) << __func__ << " NOUP flag set, will not mark osd." << i
+            << " up" << dendl;
     return false;
   }
+
+  if (osdmap.is_noup(i)) {
+    dout(5) << __func__ << " osd." << i << " is marked as noup, "
+            << "will not mark it up" << dendl;
+    return false;
+  }
+
   return true;
 }
 
@@ -1671,9 +1679,17 @@ bool OSDMonitor::can_mark_out(int i)
 bool OSDMonitor::can_mark_in(int i)
 {
   if (osdmap.test_flag(CEPH_OSDMAP_NOIN)) {
-    dout(5) << "can_mark_in NOIN flag set, will not mark osd." << i << " in" << dendl;
+    dout(5) << __func__ << " NOIN flag set, will not mark osd." << i
+            << " in" << dendl;
     return false;
   }
+
+  if (osdmap.is_noin(i)) {
+    dout(5) << __func__ << " osd." << i << " is marked as noin, "
+            << "will not mark it in" << dendl;
+    return false;
+  }
+
   return true;
 }
 
@@ -3508,6 +3524,19 @@ void OSDMonitor::get_health(list<pair<health_status_t,string> >& summary,
         }
       }
 
+      // warn if there is any noup osds.
+      vector<int> noup_osds;
+      osdmap.get_noup_osds(&noup_osds);
+      if (noup_osds.size()) {
+        ostringstream ss;
+        ss << noup_osds.size() << " noup osd(s)";
+        summary.push_back(make_pair(HEALTH_WARN, ss.str()));
+        if (detail) {
+          ss << ": " << noup_osds;
+          detail->push_back(make_pair(HEALTH_WARN, ss.str()));
+        }
+      }
+
       // warn if there is any nodown osds.
       vector<int> nodown_osds;
       osdmap.get_nodown_osds(&nodown_osds);
@@ -3521,6 +3550,19 @@ void OSDMonitor::get_health(list<pair<health_status_t,string> >& summary,
         }
       }
 
+      // warn if there is any noin osds.
+      vector<int> noin_osds;
+      osdmap.get_noin_osds(&noin_osds);
+      if (noin_osds.size()) {
+        ostringstream ss;
+        ss << noin_osds.size() << " noin osd(s)";
+        summary.push_back(make_pair(HEALTH_WARN, ss.str()));
+        if (detail) {
+          ss << ": " << noin_osds;
+          detail->push_back(make_pair(HEALTH_WARN, ss.str()));
+        }
+      }
+
       // warn if there is any noout osds.
       vector<int> noout_osds;
       osdmap.get_noout_osds(&noout_osds);
@@ -8278,10 +8320,28 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op,
 						get_last_committed() + 1));
       return true;
     }
-  } else if (prefix == "osd add-noout" ||
+  } else if (prefix == "osd add-noup" ||
-             prefix == "osd add-nodown") {
+             prefix == "osd add-nodown" ||
+             prefix == "osd add-noin" ||
+             prefix == "osd add-noout") {
+
+    enum {
+      OP_NOUP,
+      OP_NODOWN,
+      OP_NOIN,
+      OP_NOOUT,
+    } option;
+
+    if (prefix == "osd add-noup") {
+      option = OP_NOUP;
+    } else if (prefix == "osd add-nodown") {
+      option = OP_NODOWN;
+    } else if (prefix == "osd add-noin") {
+      option = OP_NOIN;
+    } else {
+      option = OP_NOOUT;
+    }
 
-    bool noout = prefix == "osd add-noout";
     bool any = false;
     bool stop = false;
 
@@ -8317,46 +8377,73 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op,
           continue;
         }
 
-        if (noout) {
+        switch (option) {
-          if (osdmap.is_out(osd)) {
+        case OP_NOUP:
-            ss << "osd." << osd << " is already out. ";
+          if (osdmap.is_up(osd)) {
+            ss << "osd." << osd << " is already up. ";
             continue;
           }
 
-          if (osdmap.is_noout(osd)) { // already noout?
+          if (osdmap.is_noup(osd)) {
-            // continue to check if there is any pending "rm-noout" request
+            if (pending_inc.pending_osd_state_clear(osd, CEPH_OSD_NOUP))
-            if (pending_inc.pending_osd_has_state(osd, CEPH_OSD_NOOUT)) {
-              // cancel it
-              pending_inc.pending_osd_state_clear(osd, CEPH_OSD_NOOUT);
-              any = true;
-            }
-
-            continue;
-          }
-
-          pending_inc.pending_osd_state_set(osd, CEPH_OSD_NOOUT);
               any = true;
           } else {
-          // nodown
+            pending_inc.pending_osd_state_set(osd, CEPH_OSD_NOUP);
+            any = true;
+          }
 
+          break;
+
+        case OP_NODOWN:
           if (osdmap.is_down(osd)) {
             ss << "osd." << osd << " is already down. ";
             continue;
           }
 
-          if (osdmap.is_nodown(osd)) { // already nodown?
+          if (osdmap.is_nodown(osd)) {
-            // continue to check if there is any pending "rm-nodown" request
+            if (pending_inc.pending_osd_state_clear(osd, CEPH_OSD_NODOWN))
-            if (pending_inc.pending_osd_has_state(osd, CEPH_OSD_NODOWN)) {
+              any = true;
-              // cancel it
+          } else {
-              pending_inc.pending_osd_state_clear(osd, CEPH_OSD_NODOWN);
+            pending_inc.pending_osd_state_set(osd, CEPH_OSD_NODOWN);
             any = true;
           }
 
+          break;
+
+        case OP_NOIN:
+          if (osdmap.is_in(osd)) {
+            ss << "osd." << osd << " is already in. ";
             continue;
           }
 
-          pending_inc.pending_osd_state_set(osd, CEPH_OSD_NODOWN);
+          if (osdmap.is_noin(osd)) {
+            if (pending_inc.pending_osd_state_clear(osd, CEPH_OSD_NOIN))
               any = true;
+          } else {
+            pending_inc.pending_osd_state_set(osd, CEPH_OSD_NOIN);
+            any = true;
+          }
+
+          break;
+
+        case OP_NOOUT:
+          if (osdmap.is_out(osd)) {
+            ss << "osd." << osd << " is already out. ";
+            continue;
+          }
+
+          if (osdmap.is_noout(osd)) {
+            if (pending_inc.pending_osd_state_clear(osd, CEPH_OSD_NOOUT))
+              any = true;
+          } else {
+            pending_inc.pending_osd_state_set(osd, CEPH_OSD_NOOUT);
+            any = true;
+          }
+
+          break;
+
+        default:
+	  assert(0 == "invalid option");
         }
       }
     }
@@ -8367,47 +8454,96 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op,
                                  get_last_committed() + 1));
       return true;
     }
-  } else if (prefix == "osd rm-noout" ||
+  } else if (prefix == "osd rm-noup" ||
-             prefix == "osd rm-nodown") {
+             prefix == "osd rm-nodown" ||
+             prefix == "osd rm-noin" ||
+             prefix == "osd rm-noout") {
+
+    enum {
+      OP_NOUP,
+      OP_NODOWN,
+      OP_NOIN,
+      OP_NOOUT,
+    } option;
+
+    if (prefix == "osd rm-noup") {
+      option = OP_NOUP;
+    } else if (prefix == "osd rm-nodown") {
+      option = OP_NODOWN;
+    } else if (prefix == "osd rm-noin") {
+      option = OP_NOIN;
+    } else {
+      option = OP_NOOUT;
+    }
 
     bool any = false;
     bool stop = false;
-    bool noout = prefix == "osd rm-noout";
 
     vector<string> idvec;
     cmd_getval(g_ceph_context, cmdmap, "ids", idvec);
 
     for (unsigned j = 0; j < idvec.size() && !stop; j++) {
+
       vector<int> osds;
 
       // wildcard?
       if (j == 0 &&
           (idvec[0] == "any" || idvec[0] == "all" || idvec[0] == "*")) {
 
-        // touch previous noout/nodown osds only
+        // touch previous noup/nodown/noin/noout osds only
-        if (noout) {
+        switch (option) {
-          osdmap.get_noout_osds(&osds);
+        case OP_NOUP:
-        } else {
+          osdmap.get_noup_osds(&osds);
+          break;
+        case OP_NODOWN:
           osdmap.get_nodown_osds(&osds);
+          break;
+        case OP_NOIN:
+          osdmap.get_noin_osds(&osds);
+          break;
+        case OP_NOOUT:
+          osdmap.get_noout_osds(&osds);
+          break;
+        default:
+          assert(0 == "invalid option");
         }
 
-        // cancel pending noout/nodown requests too,
+        // cancel any pending noup/nodown/noin/noout requests too
-        // if there is any
         vector<int> pending_state_osds;
         (void) pending_inc.get_pending_state_osds(&pending_state_osds);
         for (auto &p : pending_state_osds) {
-          if (noout) {
+
-            if (!osdmap.is_noout(p) &&
+          switch (option) {
-                pending_inc.pending_osd_has_state(p, CEPH_OSD_NOOUT)) {
+          case OP_NOUP:
-              pending_inc.pending_osd_state_clear(p, CEPH_OSD_NOOUT);
+            if (!osdmap.is_noup(p) &&
+                pending_inc.pending_osd_state_clear(p, CEPH_OSD_NOUP)) {
               any = true;
             }
-          } else {
+            break;
+
+          case OP_NODOWN:
             if (!osdmap.is_nodown(p) &&
-                pending_inc.pending_osd_has_state(p, CEPH_OSD_NODOWN)) {
+                pending_inc.pending_osd_state_clear(p, CEPH_OSD_NODOWN)) {
-              pending_inc.pending_osd_state_clear(p, CEPH_OSD_NODOWN);
               any = true;
             }
+            break;
+
+          case OP_NOIN:
+            if (!osdmap.is_noin(p) &&
+                pending_inc.pending_osd_state_clear(p, CEPH_OSD_NOIN)) {
+              any = true;
+            }
+            break;
+
+          case OP_NOOUT:
+            if (!osdmap.is_noout(p) &&
+                pending_inc.pending_osd_state_clear(p, CEPH_OSD_NOOUT)) {
+              any = true;
+            }
+            break;
+
+          default:
+            assert(0 == "invalid option");
           }
         }
 
@@ -8433,33 +8569,49 @@ bool OSDMonitor::prepare_command_impl(MonOpRequestRef op,
           continue;
         }
 
-        if (noout) {
+        switch (option) {
-          if (osdmap.is_noout(osd)) {
+          case OP_NOUP:
-            pending_inc.pending_osd_state_set(osd, CEPH_OSD_NOOUT);
+            if (osdmap.is_noup(osd)) {
+              pending_inc.pending_osd_state_set(osd, CEPH_OSD_NOUP);
               any = true;
-          } else {
+            } else if (pending_inc.pending_osd_state_clear(
-            // noout flag is not set or has already been successfully cancelled
+              osd, CEPH_OSD_NOUP)) {
-            // continue to check pending_inc
-            if (pending_inc.pending_osd_has_state(osd, CEPH_OSD_NOOUT)) {
-              // cancel pending noout flag
-              pending_inc.pending_osd_state_clear(osd, CEPH_OSD_NOOUT);
               any = true;
             }
-          }
+            break;
-        } else {
+
-          // nodown
+          case OP_NODOWN:
             if (osdmap.is_nodown(osd)) {
               pending_inc.pending_osd_state_set(osd, CEPH_OSD_NODOWN);
               any = true;
-          } else {
+            } else if (pending_inc.pending_osd_state_clear(
-            // nodown flag is not set or has already been successfully cancelled
+              osd, CEPH_OSD_NODOWN)) {
-            // continue to check pending_inc
-            if (pending_inc.pending_osd_has_state(osd, CEPH_OSD_NODOWN)) {
-              // cancel pending nodown flag
-              pending_inc.pending_osd_state_clear(osd, CEPH_OSD_NODOWN);
               any = true;
             }
+            break;
+
+          case OP_NOIN:
+            if (osdmap.is_noin(osd)) {
+              pending_inc.pending_osd_state_set(osd, CEPH_OSD_NOIN);
+              any = true;
+            } else if (pending_inc.pending_osd_state_clear(
+              osd, CEPH_OSD_NOIN)) {
+              any = true;
             }
+            break;
+
+          case OP_NOOUT:
+            if (osdmap.is_noout(osd)) {
+              pending_inc.pending_osd_state_set(osd, CEPH_OSD_NOOUT);
+              any = true;
+            } else if (pending_inc.pending_osd_state_clear(
+              osd, CEPH_OSD_NOOUT)) {
+              any = true;
+            }
+            break;
+
+          default:
+            assert(0 == "invalid option");
         }
       }
     }

src/osd/OSD.cc

@@ -5609,7 +5609,7 @@ void OSD::_preboot(epoch_t oldest, epoch_t newest)
   // if our map within recent history, try to add ourselves to the osdmap.
   if (osdmap->get_epoch() == 0) {
     derr << "waiting for initial osdmap" << dendl;
-  } else if (osdmap->test_flag(CEPH_OSDMAP_NOUP)) {
+  } else if (osdmap->test_flag(CEPH_OSDMAP_NOUP) || osdmap->is_noup(whoami)) {
     derr << "osdmap NOUP flag is set, waiting for it to clear" << dendl;
   } else if (!osdmap->test_flag(CEPH_OSDMAP_SORTBITWISE)) {
     derr << "osdmap SORTBITWISE OSDMap flag is NOT set; please set it"
@@ -7604,8 +7604,9 @@ void OSD::_committed_osd_maps(epoch_t first, epoch_t last, MOSDMap *m)
       }
     }
 
-    if (osdmap->test_flag(CEPH_OSDMAP_NOUP) !=
+    if ((osdmap->test_flag(CEPH_OSDMAP_NOUP) !=
-	newmap->test_flag(CEPH_OSDMAP_NOUP)) {
+        newmap->test_flag(CEPH_OSDMAP_NOUP)) ||
+        (osdmap->is_noup(whoami) != newmap->is_noup(whoami))) {
       dout(10) << __func__ << " NOUP flag changed in " << newmap->get_epoch()
 	       << dendl;
       if (is_booting()) {

src/osd/OSDMap.h

@@ -478,8 +478,16 @@ public:
       new_state[osd] |= state;
     }
 
-    void pending_osd_state_clear(int osd, unsigned state) {
+    // cancel the specified pending osd state if there is any
+    // return ture on success, false otherwise.
+    bool pending_osd_state_clear(int osd, unsigned state) {
+      if (!pending_osd_has_state(osd, state)) {
+        // never has been set or already has been cancelled.
+        return false;
+      }
+
       new_state[osd] &= ~state;
+      return true;
     }
 
   };
@@ -771,14 +779,33 @@ public:
     return !is_out(osd);
   }
 
+  bool is_noup(int osd) const {
+    return exists(osd) && (osd_state[osd] & CEPH_OSD_NOUP);
+  }
+
   bool is_nodown(int osd) const {
     return exists(osd) && (osd_state[osd] & CEPH_OSD_NODOWN);
   }
 
+  bool is_noin(int osd) const {
+    return exists(osd) && (osd_state[osd] & CEPH_OSD_NOIN);
+  }
+
   bool is_noout(int osd) const {
     return exists(osd) && (osd_state[osd] & CEPH_OSD_NOOUT);
   }
 
+  void get_noup_osds(vector<int> *osds) const {
+    assert(osds);
+    osds->clear();
+
+    for (int i = 0; i < max_osd; i++) {
+      if (is_noup(i)) {
+        osds->push_back(i);
+      }
+    }
+  }
+
   void get_nodown_osds(vector<int> *osds) const {
     assert(osds);
     osds->clear();
@@ -790,6 +817,17 @@ public:
     }
   }
 
+  void get_noin_osds(vector<int> *osds) const {
+    assert(osds);
+    osds->clear();
+
+    for (int i = 0; i < max_osd; i++) {
+      if (is_noin(i)) {
+        osds->push_back(i);
+      }
+    }
+  }
+
   void get_noout_osds(vector<int> *osds) const {
     assert(osds);
     osds->clear();