From e616bece0845290eb05736bd0d3fc348c73183cc Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@redhat.com>
Date: Thu, 5 Sep 2019 17:11:26 -0500
Subject: [PATCH] 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>
---
 src/common/admin_socket.cc | 47 ++++++++++++++------------------------
 src/common/cmdparse.cc     |  5 ++--
 src/common/cmdparse.h      |  2 +-
 3 files changed, 21 insertions(+), 33 deletions(-)

diff --git a/src/common/admin_socket.cc b/src/common/admin_socket.cc
index ef22b3c2615..2c3167035a0 100644
--- 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 5945c0baf76..4a89eaaa699 100644
--- 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 2c59567d588..af94352a0eb 100644
--- 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);
-- 
2.39.5