common/admin_socket: drop unregister_command(); use per-hook variant

There's never a need to unregister individual commands; doing it by hook
is sufficient for all users.  Simpler and faster.

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

diff --git a/src/common/admin_socket.cc b/src/common/admin_socket.cc
index 293afd005505919e17a4143ebc92cd81567c4b80..898c8534809a152a58ab092ea830a94264d28ebf 100644 (file)
--- a/src/common/admin_socket.cc
+++ b/src/common/admin_socket.cc
@@ -487,30 +487,6 @@ int AdminSocket::register_command(std::string_view cmddesc,
   return ret;
 }
 
-int AdminSocket::unregister_command(std::string_view command)
-{
-  int ret;
-  std::unique_lock l(lock);
-  auto i = hooks.find(command);
-  if (i != hooks.cend()) {
-    ldout(m_cct, 5) << "unregister_command " << command << dendl;
-
-    // If we are currently processing a command, wait for it to
-    // complete in case it referenced the hook that we are
-    // unregistering.
-    in_hook_cond.wait(l, [this]() { return !in_hook; });
-
-    hooks.erase(i);
-
-
-    ret = 0;
-  } else {
-    ldout(m_cct, 5) << "unregister_command " << command << " ENOENT" << dendl;
-    ret = -ENOENT;
-  }
-  return ret;
-}
-
 void AdminSocket::unregister_commands(const AdminSocketHook *hook)
 {
   std::unique_lock l(lock);
@@ -674,10 +650,10 @@ void AdminSocket::shutdown()
   unregister_commands(version_hook.get());
   version_hook.reset();
 
-  unregister_command("help");
+  unregister_commands(help_hook.get());
   help_hook.reset();
 
-  unregister_command("get_command_descriptions");
+  unregister_commands(getdescs_hook.get());
   getdescs_hook.reset();
 
   remove_cleanup_file(m_path);

diff --git a/src/common/admin_socket.h b/src/common/admin_socket.h
index e8f91547dab1ec4ef5fae6caf04e0a7140f4504d..2249ef852e555bfa3e8e2ae14a46f87c194c2ace 100644 (file)
--- a/src/common/admin_socket.h
+++ b/src/common/admin_socket.h
@@ -74,18 +74,6 @@ public:
                       AdminSocketHook *hook,
                       std::string_view help);
 
-  /**
-   * unregister an admin socket command.
-   *
-   * If a command is currently in progress, this will block until it
-   * is done.  For that reason, you must not hold any locks required
-   * by your hook while you call this.
-   *
-   * @param command command string
-   * @return 0 on succest, -ENOENT if command dne.
-   */
-  int unregister_command(std::string_view command);
-
   /*
    * unregister all commands belong to hook.
    */

diff --git a/src/common/ceph_context.cc b/src/common/ceph_context.cc
index f679dfc86377d0e1ef6d938f825bbc01c9b53129..ee5b44b81cb7b66f41d903e87cef4f74eb296ab3 100644 (file)
--- a/src/common/ceph_context.cc
+++ b/src/common/ceph_context.cc
@@ -145,7 +145,7 @@ public:
   }
   ~MempoolObs() override {
     cct->_conf.remove_observer(this);
-    cct->get_admin_socket()->unregister_command("dump_mempools");
+    cct->get_admin_socket()->unregister_commands(this);
   }
 
   // md_config_obs_t

diff --git a/src/librbd/LibrbdAdminSocketHook.cc b/src/librbd/LibrbdAdminSocketHook.cc
index 31e0dd3245a5809068d350dfc2042a140f2da606..7f713a67c96202943f70707ac012a9a3c721bbc9 100644 (file)
--- a/src/librbd/LibrbdAdminSocketHook.cc
+++ b/src/librbd/LibrbdAdminSocketHook.cc
@@ -81,9 +81,9 @@ LibrbdAdminSocketHook::LibrbdAdminSocketHook(ImageCtx *ictx) :
 }
 
 LibrbdAdminSocketHook::~LibrbdAdminSocketHook() {
+  (void)admin_socket->unregister_commands(this);
   for (Commands::const_iterator i = commands.begin(); i != commands.end();
        ++i) {
-    (void)admin_socket->unregister_command(i->first);
     delete i->second;
   }
 }

diff --git a/src/os/bluestore/Allocator.cc b/src/os/bluestore/Allocator.cc
index c8d339f8d5be4af43da8392f1cf88ac5f2bc1161..8e5b1b6f4f34613a2cde6dd3f9138be09fd4b20b 100644 (file)
--- a/src/os/bluestore/Allocator.cc
+++ b/src/os/bluestore/Allocator.cc
@@ -46,12 +46,7 @@ public:
   {
     AdminSocket *admin_socket = g_ceph_context->get_admin_socket();
     if (admin_socket && alloc) {
-      int r = admin_socket->unregister_command(("bluestore allocator dump " + name).c_str());
-      ceph_assert(r == 0);
-      r = admin_socket->unregister_command(("bluestore allocator score " + name).c_str());
-      ceph_assert(r == 0);
-      r = admin_socket->unregister_command(("bluestore allocator fragmentation " + name).c_str());
-      ceph_assert(r == 0);
+      admin_socket->unregister_commands(this);
     }
   }
 

diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc
index 4ea05895f07f21a830438cbdd3092fd93afad95f..06a574232c570dab4b71a1aa45d172252016c56a 100644 (file)
--- a/src/os/bluestore/BlueFS.cc
+++ b/src/os/bluestore/BlueFS.cc
@@ -68,8 +68,7 @@ public:
 
   ~SocketHook() {
     AdminSocket* admin_socket = bluefs->cct->get_admin_socket();
-    int r = admin_socket->unregister_command("bluestore bluefs available");
-    ceph_assert(r == 0);
+    admin_socket->unregister_commands(this);
   }
 private:
   SocketHook(BlueFS* bluefs) :

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index b9ba9a17108b8db163fadaac051c9d8160293142..3799ece8572a5a6e0d1624093635e47739cc6d5f 100644 (file)
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -531,7 +531,7 @@ void Objecter::shutdown()
   // shutdown() with the ::initialized check at start.
   if (m_request_state_hook) {
     AdminSocket* admin_socket = cct->get_admin_socket();
-    admin_socket->unregister_command("objecter_requests");
+    admin_socket->unregister_commands(m_request_state_hook);
     delete m_request_state_hook;
     m_request_state_hook = NULL;
   }

diff --git a/src/rgw/rgw_coroutine.cc b/src/rgw/rgw_coroutine.cc
index c888ccaa0bf17c79e45c856413de97dc85c2e4a1..fbaa6ecf796d6928f04176468c3fe79c3edac8de 100644 (file)
--- a/src/rgw/rgw_coroutine.cc
+++ b/src/rgw/rgw_coroutine.cc
@@ -832,7 +832,7 @@ RGWCoroutinesManagerRegistry::~RGWCoroutinesManagerRegistry()
 {
   AdminSocket *admin_socket = cct->get_admin_socket();
   if (!admin_command.empty()) {
-    admin_socket->unregister_command(admin_command);
+    admin_socket->unregister_commands(this);
   }
 }
 
@@ -840,7 +840,7 @@ int RGWCoroutinesManagerRegistry::hook_to_admin_command(const string& command)
 {
   AdminSocket *admin_socket = cct->get_admin_socket();
   if (!admin_command.empty()) {
-    admin_socket->unregister_command(admin_command);
+    admin_socket->unregister_commands(this);
   }
   admin_command = command;
   int r = admin_socket->register_command(admin_command, this,

diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc
index 0170e4fb57e97c6ceabd36d5188b32a5f7172afb..854769ebfe4760f6a37e3b92b6b7dfccf267f916 100644 (file)
--- a/src/tools/rbd_mirror/ImageDeleter.cc
+++ b/src/tools/rbd_mirror/ImageDeleter.cc
@@ -98,9 +98,9 @@ public:
   }
 
   ~ImageDeleterAdminSocketHook() override {
+    (void)admin_socket->unregister_commands(this);
     for (Commands::const_iterator i = commands.begin(); i != commands.end();
         ++i) {
-      (void)admin_socket->unregister_command(i->first);
       delete i->second;
     }
   }

diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc
index a5b1ccf27e06959b21438f723f51e3ec856bd320..e1a2551f42807abb7200dcb1ce365d5c8ddcefa8 100644 (file)
--- a/src/tools/rbd_mirror/ImageReplayer.cc
+++ b/src/tools/rbd_mirror/ImageReplayer.cc
@@ -188,10 +188,8 @@ public:
   }
 
   ~ImageReplayerAdminSocketHook() override {
+    admin_socket->unregister_commands(this);
     for (auto &it : commands) {
-      if (it.second->registered) {
-        admin_socket->unregister_command(it.first);
-      }
       delete it.second;
     }
     commands.clear();

diff --git a/src/tools/rbd_mirror/Mirror.cc b/src/tools/rbd_mirror/Mirror.cc
index a12aa5473f57c4299bfaf1665ee6bedc17af31be..2b580ffd157ce5c3f16e7bcca452cb0d039a34c4 100644 (file)
--- a/src/tools/rbd_mirror/Mirror.cc
+++ b/src/tools/rbd_mirror/Mirror.cc
@@ -314,9 +314,9 @@ public:
   }
 
   ~MirrorAdminSocketHook() override {
+    (void)admin_socket->unregister_commands(this);
     for (Commands::const_iterator i = commands.begin(); i != commands.end();
         ++i) {
-      (void)admin_socket->unregister_command(i->first);
       delete i->second;
     }
   }

diff --git a/src/tools/rbd_mirror/PoolReplayer.cc b/src/tools/rbd_mirror/PoolReplayer.cc
index 8f2452d2fe9d9f05d7ca33b3d9a86f4dfea1b731..6f6c80814d31286addc85a2e8653d4961c455e1c 100644 (file)
--- a/src/tools/rbd_mirror/PoolReplayer.cc
+++ b/src/tools/rbd_mirror/PoolReplayer.cc
@@ -179,8 +179,8 @@ public:
   }
 
   ~PoolReplayerAdminSocketHook() override {
+    (void)admin_socket->unregister_commands(this);
     for (auto i = commands.begin(); i != commands.end(); ++i) {
-      (void)admin_socket->unregister_command(i->first);
       delete i->second;
     }
   }