From: Sage Weil Date: Thu, 5 Sep 2019 22:24:55 +0000 (-0500) Subject: common/admin_socket: drop unregister_command(); use per-hook variant X-Git-Tag: v15.1.0~1323^2~43 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d134a622a572ae69ae7b56209383dda98f81e2dd;p=ceph.git 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 --- diff --git a/src/common/admin_socket.cc b/src/common/admin_socket.cc index 293afd005505..898c8534809a 100644 --- 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 e8f91547dab1..2249ef852e55 100644 --- 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 f679dfc86377..ee5b44b81cb7 100644 --- 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 31e0dd3245a5..7f713a67c962 100644 --- 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 c8d339f8d5be..8e5b1b6f4f34 100644 --- 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 4ea05895f07f..06a574232c57 100644 --- 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 b9ba9a17108b..3799ece8572a 100644 --- 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 c888ccaa0bf1..fbaa6ecf796d 100644 --- 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 0170e4fb57e9..854769ebfe47 100644 --- 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 a5b1ccf27e06..e1a2551f4280 100644 --- 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 a12aa5473f57..2b580ffd157c 100644 --- 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 8f2452d2fe9d..6f6c80814d31 100644 --- 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; } }