From: Kefu Chai <kchai@redhat.com>
Date: Fri, 11 Sep 2020 08:35:29 +0000 (+0800)
Subject: ciommon/admin_socket: extract find_matched_hook() out
X-Git-Tag: v15.2.9~122^2~64^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c350b133b55bf4a0ecd32f77a17cfd5ea5357fdc;p=ceph.git

ciommon/admin_socket: extract find_matched_hook() out

this function is complicated and isolated enough to be a separated
method.

Signed-off-by: Kefu Chai <kchai@redhat.com>
(cherry picked from commit 0fc4ed1d9109abc489a65340b92908107337fafd)

Conflicts:
	src/common/admin_socket.cc
- octopus has a "while" block (instead of "if") under the comment
  // make sure one of the registered commands with this prefix validates
  but this is being removed
---

diff --git a/src/common/admin_socket.cc b/src/common/admin_socket.cc
index 1f57ca0832f1..253298de4b9c 100644
--- a/src/common/admin_socket.cc
+++ b/src/common/admin_socket.cc
@@ -492,32 +492,20 @@ void AdminSocket::execute_command(
 
   Formatter *f = Formatter::create(format, "json-pretty", "json-pretty");
 
-  std::unique_lock l(lock);
-  decltype(hooks)::iterator p;
-  p = hooks.find(prefix);
-  if (p == hooks.cend()) {
+  auto [retval, hook] = find_matched_hook(prefix, cmdmap);
+  switch (retval) {
+  case ENOENT:
     lderr(m_cct) << "AdminSocket: request '" << cmdvec
 		 << "' not defined" << dendl;
     delete f;
     return on_finish(-EINVAL, "unknown command prefix "s + prefix, empty);
+  case EINVAL:
+    delete f;
+    return on_finish(-EINVAL, "invalid command json", empty);
+  default:
+    assert(retval == 0);
   }
 
-  // make sure one of the registered commands with this prefix validates.
-  while (!validate_cmd(m_cct, p->second.desc, cmdmap, errss)) {
-    ++p;
-    if (p->first != prefix) {
-      delete f;
-      return on_finish(-EINVAL, "invalid command json", empty);
-    }
-  }
-
-  // Drop lock to avoid cycles in cases where the hook takes
-  // the same lock that was held during calls to register/unregister,
-  // and set in_hook to allow unregister to wait for us before
-  // removing this hook.
-  in_hook = true;
-  auto hook = p->second.hook;
-  l.unlock();
   hook->call_async(
     prefix, cmdmap, f, inbl,
     [f, on_finish](int r, const std::string& err, bufferlist& out) {
@@ -529,11 +517,35 @@ void AdminSocket::execute_command(
       on_finish(r, err, out);
     });
 
-  l.lock();
+  std::unique_lock l(lock);
   in_hook = false;
   in_hook_cond.notify_all();
 }
 
+std::pair<int, AdminSocketHook*>
+AdminSocket::find_matched_hook(std::string& prefix,
+			       const cmdmap_t& cmdmap)
+{
+  std::unique_lock l(lock);
+  // Drop lock after done with the lookup to avoid cycles in cases where the
+  // hook takes the same lock that was held during calls to
+  // register/unregister, and set in_hook to allow unregister to wait for us
+  // before removing this hook.
+  auto p = hooks.find(prefix);
+  if (p == hooks.cend()) {
+    return {ENOENT, nullptr};
+  }
+  // make sure one of the registered commands with this prefix validates.
+  stringstream errss;
+  if (!validate_cmd(m_cct, p->second.desc, cmdmap, errss)) {
+    if (p->first != prefix) {
+      return {EINVAL, nullptr};
+    }
+  }
+  in_hook = true;
+  return {0, p->second.hook};
+}
+
 void AdminSocket::queue_tell_command(cref_t<MCommand> m)
 {
   ldout(m_cct,10) << __func__ << " " << *m << dendl;
diff --git a/src/common/admin_socket.h b/src/common/admin_socket.h
index 2105097bbeeb..2e4e153f4de8 100644
--- a/src/common/admin_socket.h
+++ b/src/common/admin_socket.h
@@ -207,6 +207,11 @@ private:
       : hook(hook), desc(desc), help(help) {}
   };
 
+  /// find the first hook which matches the given prefix and cmdmap
+  std::pair<int, AdminSocketHook*> find_matched_hook(
+    std::string& prefix,
+    const cmdmap_t& cmdmap);
+
   std::multimap<std::string, hook_info, std::less<>> hooks;
 
   friend class AdminSocketTest;