ceph, mon/OSDMonitor: fix up osd crush commands for <osd.N> or <N>

The new parsing code had been trying to allow flexibility for the
'old form' commands (where id could be different from N in osd.N),
but also accept 'new form' commands.  The new rule is that where
there's an OSD specified in the osd crush command, it is of type
CephOsdName, which can be an id *or* 'osd.<id>', but not both.

Pass CephOsdName as int64_t 'id' for convenience in mon code

Signed-off-by: Dan Mick <dan.mick@inktank.com>
Reviewed-by: Sage Weil <sage@inktank.com>

diff --git a/src/ceph.in b/src/ceph.in
index 857197185f436bf96c154c742dc67ed3bbaa254e..c34c9cecd3e7223d98ce465f633b09f61d7c8608 100755 (executable)
--- a/src/ceph.in
+++ b/src/ceph.in
@@ -345,8 +345,9 @@ class CephPgid(CephArgtype):
 
 class CephName(CephArgtype):
     """
-    Name, or type.id, where type is osd|mon|client|mds, and id is a base10 int,
-    or just id.
+    Name (type.id) where:
+    type is osd|mon|client|mds
+    id is a base10 int, if type == osd, or a string otherwise
 
     Also accept '*'
     """
@@ -357,7 +358,7 @@ class CephName(CephArgtype):
             self.nameid = None
             return True
         if s.find('.') == -1:
-            i = s
+            raise ArgumentFormat('CephName: no . in {0}'.format(s))
         else:
             t, i = s.split('.')
             if not t in ('osd', 'mon', 'client', 'mds'):
@@ -375,6 +376,36 @@ class CephName(CephArgtype):
     def __str__(self):
         return '<name (type.id)>'
 
+class CephOsdName(CephArgtype):
+    """
+    Like CephName, but specific to osds: allow <id> alone
+
+    osd.<id>, or <id>, or *, where id is a base10 int
+    """
+    def valid(self, s, partial=False):
+        if s == '*':
+            self.val = s
+            self.nametype = None
+            self.nameid = None
+            return True
+        if s.find('.') != -1:
+            t, i = s.split('.')
+        else:
+            t = 'osd'
+            i = s
+        if t != 'osd':
+            raise ArgumentValid('unknown type ' + self.t)
+        try:
+            i = int(i)
+        except:
+            raise ArgumentFormat('osd id ' + i + ' not integer')
+        self.nametype = t
+        self.nameid = i
+        self.val = i
+        return True
+
+    def __str__(self):
+        return '<osdname (id|osd.id)>'
 
 class CephChoices(CephArgtype):
     """

diff --git a/src/mon/MonCommands.h b/src/mon/MonCommands.h
index 4e64a0a70d497039c181dd34c418856a158ff918..23029e1b8a835ee553df05ff1b840eb5137dcc12 100644 (file)
--- a/src/mon/MonCommands.h
+++ b/src/mon/MonCommands.h
@@ -55,6 +55,7 @@
  * CephObjectname: Another plainold string
  * CephPgid: n.xxx where n is an int > 0, xxx is a hex number > 0
  * CephName: daemon name, '*' or '<type>.<id>' (id must be int for type osd)
+ * CephOsdName: osd name, '*' or '<id> or 'osd.<id>' (id must be int)
  * CephChoices: strings="foo|bar" means this param can be either
  * CephFilepath: openable file
  * CephFragment: cephfs 'fragID': val/bits, val in hex 0xnnn, bits in dec
@@ -324,24 +325,22 @@ COMMAND("osd crush add-bucket " \
        "name=type,type=CephString", \
        "add no-parent (probably root) crush bucket <name> of type <type>")
 COMMAND("osd crush set " \
-       "name=id,type=CephInt,range=0 " \
-       "name=name,type=CephName,req=false " \
+       "name=id,type=CephOsdName " \
        "name=weight,type=CephFloat,range=0.0 " \
        "name=args,type=CephString,n=N", \
-       "set crushmap entry for <id> to <weight> with location <args>")
+       "set crushmap entry for <name> to <weight> with location <args>")
 COMMAND("osd crush add " \
-       "name=id,type=CephInt,range=0 " \
-       "name=name,type=CephName,req=false " \
+       "name=id,type=CephOsdName " \
        "name=weight,type=CephFloat,range=0.0 " \
        "name=args,type=CephString,n=N", \
-       "add crushmap entry for <id> with <weight> and location <args>")
+       "add crushmap entry for <name> with <weight> and location <args>")
 COMMAND("osd crush create-or-move " \
-       "name=id,type=CephInt,range=0 " \
+       "name=id,type=CephOsdName " \
        "name=weight,type=CephFloat,range=0.0 " \
        "name=args,type=CephString,n=N", \
-       "create entry or move existing entry for <id> <weight> at/to location <args>")
+       "create entry or move existing entry for <name> <weight> at/to location <args>")
 COMMAND("osd crush move " \
-       "name=name,type=CephString " \
+       "name=id,type=CephOsdName " \
        "name=args,type=CephString,n=N", \
        "move existing entry for <name> to location <args>")
 COMMAND("osd crush link " \

diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc
index d6e2e1cb56065dadaba25a4eb8c1fbf063641685..d6c017f0d9c3817cc35f0a733582b14779e8247f 100644 (file)
--- a/src/mon/OSDMonitor.cc
+++ b/src/mon/OSDMonitor.cc
@@ -2482,7 +2482,13 @@ bool OSDMonitor::prepare_command(MMonCommand *m)
   cmd_getval(g_ceph_context, cmdmap, "prefix", prefix);
 
   int64_t id;
+  string name;
   bool osdid_present = cmd_getval(g_ceph_context, cmdmap, "id", id);
+  if (osdid_present) {
+    ostringstream oss;
+    oss << "osd." << id;
+    name = oss.str();
+  }
 
   if (prefix == "osd setcrushmap" ||
       (prefix == "osd crush set" && !osdid_present)) {
@@ -2544,23 +2550,19 @@ bool OSDMonitor::prepare_command(MMonCommand *m)
     ss << "added bucket " << name << " type " << typestr
        << " to crush map";
     goto update;
-  } else if (osdid_present && 
+  } else if (osdid_present &&
             (prefix == "osd crush set" || prefix == "osd crush add")) {
     do {
-      // osd crush set <osd-id> [<osd.* name>] <weight> <loc1> [<loc2> ...]
-      // osd crush add <osd-id> [<osd.* name>] <weight> <loc1> [<loc2> ...]
+      // <OsdName> is 'osd.<id>' or '<id>', passed as int64_t id
+      // osd crush set <OsdName> <weight> <loc1> [<loc2> ...]
+      // osd crush add <OsdName> <weight> <loc1> [<loc2> ...]
+
       if (!osdmap.exists(id)) {
        err = -ENOENT;
-       ss << "osd." << id << " does not exist.  create it before updating the crush map";
+       ss << name << " does not exist.  create it before updating the crush map";
        goto reply;
       }
 
-      string name;
-      if (!cmd_getval(g_ceph_context, cmdmap, "name", name)) {
-       // new usage; infer name
-       name = "osd." + stringify(id);
-      }
-
       double weight;
       cmd_getval(g_ceph_context, cmdmap, "weight", weight);
 
@@ -2604,16 +2606,13 @@ bool OSDMonitor::prepare_command(MMonCommand *m)
 
   } else if (prefix == "osd crush create-or-move") {
     do {
-      // osd crush create-or-move <id> <initial_weight> <loc1> [<loc2> ...]
-      int64_t id;
-      cmd_getval(g_ceph_context, cmdmap, "id", id);
+      // osd crush create-or-move <OsdName> <initial_weight> <loc1> [<loc2> ...]
       if (!osdmap.exists(id)) {
        err = -ENOENT;
-       ss << "osd." << id << " does not exist.  create it before updating the crush map";
+       ss << name << " does not exist.  create it before updating the crush map";
        goto reply;
       }
 
-      string name = "osd." + stringify(id);
       double weight;
       cmd_getval(g_ceph_context, cmdmap, "weight", weight);
 
@@ -2623,7 +2622,7 @@ bool OSDMonitor::prepare_command(MMonCommand *m)
       map<string,string> loc;
       parse_loc_map(argvec, &loc);
 
-      dout(0) << "create-or-move crush item id " << id << " name '" << name << "' initial_weight " << weight
+      dout(0) << "create-or-move crush item name '" << name << "' initial_weight " << weight
              << " at location " << loc << dendl;
 
       CrushWrapper newcrush;
@@ -2631,14 +2630,14 @@ bool OSDMonitor::prepare_command(MMonCommand *m)
 
       err = newcrush.create_or_move_item(g_ceph_context, id, weight, name, loc);
       if (err == 0) {
-       ss << "create-or-move updated item id " << id << " name '" << name << "' weight " << weight
+       ss << "create-or-move updated item name '" << name << "' weight " << weight
           << " at location " << loc << " to crush map";
        break;
       }
       if (err > 0) {
        pending_inc.crush.clear();
        newcrush.encode(pending_inc.crush);
-       ss << "create-or-move updating item id " << id << " name '" << name << "' weight " << weight
+       ss << "create-or-move updating item name '" << name << "' weight " << weight
           << " at location " << loc << " to crush map";
        getline(ss, rs);
        wait_for_finished_proposal(new Monitor::C_Command(mon, m, 0, rs, get_version()));
@@ -2649,8 +2648,6 @@ bool OSDMonitor::prepare_command(MMonCommand *m)
   } else if (prefix == "osd crush move") {
     do {
       // osd crush move <name> <loc1> [<loc2> ...]
-      string name;
-      cmd_getval(g_ceph_context, cmdmap, "name", name);
 
       string args;
       vector<string> argvec;

diff --git a/src/vstart.sh b/src/vstart.sh
index 02db12ee170263d35b85cfa5e2c0194a667cd43f..2e256c26aca33825101ae1aea921eeb1ffc8d237 100755 (executable)
--- a/src/vstart.sh
+++ b/src/vstart.sh
@@ -421,7 +421,7 @@ EOF
            uuid=`uuidgen`
            echo "add osd$osd $uuid"
            $SUDO $CEPH_ADM osd create $uuid
-           $SUDO $CEPH_ADM osd crush set $osd osd.$osd 1.0 host=localhost rack=localrack root=default
+           $SUDO $CEPH_ADM osd crush set osd.$osd 1.0 host=localhost rack=localrack root=default
            $SUDO $CEPH_BIN/ceph-osd -i $osd $ARGS --mkfs --mkkey --osd-uuid $uuid
 
            key_fn=dev/osd$osd/keyring