common/admin_socket: simplify command routing

Back in e30e937c8962249af283a7571eb106ef444b79e3 we made it possible to
route a command via any prefix. This worked when we wanted to pass
arguments but were just dealing with a vector<string>.  These days we have
an actual prefix followed by named arguments, so we don't need this
ad hoc routing.

Derive the prefix from the cmddesc at registration time, and match that
explicitly against the prefix at execution time.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/common/admin_socket.cc b/src/common/admin_socket.cc
index ef22b3c2615a38d072892347944b6c7cb966b697..2c3167035a030a9c6a9b631b44b3b05d7188a0b4 100644 (file)
--- a/src/common/admin_socket.cc
+++ b/src/common/admin_socket.cc
@@ -397,10 +397,10 @@ int AdminSocket::execute_command(const std::vector<std::string>& cmdvec,
     ldout(m_cct, 0) << "AdminSocket: " << errss.str() << dendl;
     return false;
   }
-  string match;
+  string prefix;
   try {
     cmd_getval(m_cct, cmdmap, "format", format);
-    cmd_getval(m_cct, cmdmap, "prefix", match);
+    cmd_getval(m_cct, cmdmap, "prefix", prefix);
   } catch (const bad_cmd_get& e) {
     return false;
   }
@@ -410,21 +410,7 @@ int AdminSocket::execute_command(const std::vector<std::string>& cmdvec,
 
   std::unique_lock l(lock);
   decltype(hooks)::iterator p;
-  while (match.size()) {
-    p = hooks.find(match);
-    if (p != hooks.cend())
-      break;
-
-    // drop right-most word
-    size_t pos = match.rfind(' ');
-    if (pos == std::string::npos) {
-      match.clear();  // we fail
-      break;
-    } else {
-      match.resize(pos);
-    }
-  }
-
+  p = hooks.find(prefix);
   if (p == hooks.cend()) {
     lderr(m_cct) << "AdminSocket: request '" << cmdvec
                 << "' not defined" << dendl;
@@ -436,20 +422,20 @@ int AdminSocket::execute_command(const std::vector<std::string>& cmdvec,
   // and set in_hook to allow unregister to wait for us before
   // removing this hook.
   in_hook = true;
-  auto match_hook = p->second.hook;
+  auto hook = p->second.hook;
   l.unlock();
-  bool success = (validate(match, cmdmap, out) &&
-      match_hook->call(match, cmdmap, format, out));
+  bool success = (validate(prefix, cmdmap, out) &&
+      hook->call(prefix, cmdmap, format, out));
   l.lock();
   in_hook = false;
   in_hook_cond.notify_all();
   if (!success) {
-    ldout(m_cct, 0) << "AdminSocket: request '" << match
-                   << "' to " << match_hook << " failed" << dendl;
+    ldout(m_cct, 0) << "AdminSocket: request '" << prefix
+                   << "' to " << hook << " failed" << dendl;
     out.append("failed");
   } else {
-    ldout(m_cct, 5) << "AdminSocket: request '" << match
-        << "' to " << match_hook
+    ldout(m_cct, 5) << "AdminSocket: request '" << prefix
+        << "' to " << hook
         << " returned " << out.length() << " bytes" << dendl;
   }
   return true;
@@ -464,12 +450,12 @@ void AdminSocket::queue_tell_command(ref_t<MCommand> m)
 }
 
 
-bool AdminSocket::validate(const std::string& command,
+bool AdminSocket::validate(const std::string& prefix,
                           const cmdmap_t& cmdmap,
                           bufferlist& out) const
 {
   stringstream os;
-  if (validate_cmd(m_cct, hooks.at(command).desc, cmdmap, os)) {
+  if (validate_cmd(m_cct, hooks.at(prefix).desc, cmdmap, os)) {
     return true;
   } else {
     out.append(os);
@@ -484,17 +470,18 @@ int AdminSocket::register_command(std::string_view command,
 {
   int ret;
   std::unique_lock l(lock);
-  auto i = hooks.find(command);
+  string prefix = cmddesc_get_prefix(cmddesc);
+  auto i = hooks.find(prefix);
   if (i != hooks.cend()) {
-    ldout(m_cct, 5) << "register_command " << command << " hook " << hook
+    ldout(m_cct, 5) << "register_command " << prefix << " hook " << hook
                    << " EEXIST" << dendl;
     ret = -EEXIST;
   } else {
-    ldout(m_cct, 5) << "register_command " << command << " hook " << hook
+    ldout(m_cct, 5) << "register_command " << prefix << " hook " << hook
                    << dendl;
     hooks.emplace_hint(i,
                       std::piecewise_construct,
-                      std::forward_as_tuple(command),
+                      std::forward_as_tuple(prefix),
                       std::forward_as_tuple(hook, cmddesc, help));
     ret = 0;
   }

diff --git a/src/common/cmdparse.cc b/src/common/cmdparse.cc
index 5945c0baf766da37759e708ed639172d7b40c2df..4a89eaaa699084f3837e0f5f95cd0b2fe9929785 100644 (file)
--- a/src/common/cmdparse.cc
+++ b/src/common/cmdparse.cc
@@ -22,9 +22,10 @@
  * Given a cmddesc like "foo baz name=bar,type=CephString",
  * return the prefix "foo baz".
  */
-std::string cmddesc_get_prefix(const std::string &cmddesc)
+std::string cmddesc_get_prefix(const std::string_view &cmddesc)
 {
-  stringstream ss(cmddesc);
+  string tmp(cmddesc); // FIXME: stringstream ctor can't take string_view :(
+  stringstream ss(tmp);
   std::string word;
   std::ostringstream result;
   bool first = true;

diff --git a/src/common/cmdparse.h b/src/common/cmdparse.h
index 2c59567d588d9738a494347c958fc7813c17252d..af94352a0ebebaf1542e4c15c71d13339e3b415b 100644 (file)
--- a/src/common/cmdparse.h
+++ b/src/common/cmdparse.h
@@ -22,7 +22,7 @@ typedef boost::variant<std::string,
                       std::vector<double>>  cmd_vartype;
 typedef std::map<std::string, cmd_vartype, std::less<>> cmdmap_t;
 
-std::string cmddesc_get_prefix(const std::string &cmddesc);
+std::string cmddesc_get_prefix(const std::string_view &cmddesc);
 std::string cmddesc_get_prenautilus_compat(const std::string &cmddesc);
 void dump_cmd_to_json(ceph::Formatter *f, uint64_t features,
                       const std::string& cmd);