From: Sage Weil Date: Thu, 19 Sep 2019 16:11:55 +0000 (-0500) Subject: common/admin_socket: pass Formatter from generic infrastructure X-Git-Tag: v15.1.0~1323^2~11 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=adf1486e46cb1f7820c77adb7b6436c47c743242;p=ceph.git common/admin_socket: pass Formatter from generic infrastructure The implementation can choose to either use the provided Formatter, or put something directly into outbl. The implementation may choose to flush the formatter to the output buffer|stream, or let the caller do it for them (usually the latter). Lots of fiddling/cleanup in the implementations to make this build, including dropping the (seeminlyg unused?) ostream& output mode for the librbd asok implementations. Signed-off-by: Sage Weil --- diff --git a/src/client/Client.cc b/src/client/Client.cc index b911003f0ebc..583f67a638a7 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -141,29 +141,27 @@ Client::CommandHook::CommandHook(Client *client) : int Client::CommandHook::call( std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) { - std::unique_ptr f(Formatter::create(format)); f->open_object_section("result"); { std::lock_guard l{m_client->client_lock}; if (command == "mds_requests") - m_client->dump_mds_requests(f.get()); + m_client->dump_mds_requests(f); else if (command == "mds_sessions") - m_client->dump_mds_sessions(f.get()); + m_client->dump_mds_sessions(f); else if (command == "dump_cache") - m_client->dump_cache(f.get()); + m_client->dump_cache(f); else if (command == "kick_stale_sessions") m_client->_kick_stale_sessions(); else if (command == "status") - m_client->dump_status(f.get()); + m_client->dump_status(f); else ceph_abort_msg("bad command registered"); } f->close_section(); - f->flush(out); return 0; } diff --git a/src/client/Client.h b/src/client/Client.h index 30c208155fc6..a88abb5d86e8 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -266,7 +266,7 @@ public: public: explicit CommandHook(Client *client); int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) override; private: diff --git a/src/common/admin_socket.cc b/src/common/admin_socket.cc index 7f26a9b31bec..4dfe7bdec46f 100644 --- a/src/common/admin_socket.cc +++ b/src/common/admin_socket.cc @@ -448,9 +448,8 @@ void AdminSocket::execute_command( return on_finish(-EINVAL, "invalid json, missing format and/or prefix", empty); } - if (format != "json" && format != "json-pretty" && - format != "xml" && format != "xml-pretty") - format = "json-pretty"; + + Formatter *f = Formatter::create(format, "json-pretty", "json-pretty"); std::unique_lock l(lock); decltype(hooks)::iterator p; @@ -458,6 +457,7 @@ void AdminSocket::execute_command( if (p == hooks.cend()) { lderr(m_cct) << "AdminSocket: request '" << cmdvec << "' not defined" << dendl; + delete f; return on_finish(-EINVAL, "unknown command prefix "s + prefix, empty); } @@ -465,6 +465,7 @@ void AdminSocket::execute_command( 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); } } @@ -476,7 +477,16 @@ void AdminSocket::execute_command( in_hook = true; auto hook = p->second.hook; l.unlock(); - hook->call_async(prefix, cmdmap, format, inbl, on_finish); + hook->call_async( + prefix, cmdmap, f, inbl, + [f, on_finish](int r, const std::string& err, bufferlist& out) { + // handle either existing output in bufferlist *or* via formatter + if (r >= 0 && out.length() == 0) { + f->flush(out); + } + delete f; + on_finish(r, err, out); + }); l.lock(); in_hook = false; @@ -539,26 +549,22 @@ void AdminSocket::unregister_commands(const AdminSocketHook *hook) class VersionHook : public AdminSocketHook { public: int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) override { if (command == "0"sv) { out.append(CEPH_ADMIN_SOCK_VERSION); } else { - JSONFormatter jf; - jf.open_object_section("version"); + f->open_object_section("version"); if (command == "version") { - jf.dump_string("version", ceph_version_to_str()); - jf.dump_string("release", ceph_release_to_str()); - jf.dump_string("release_type", ceph_release_type()); + f->dump_string("version", ceph_version_to_str()); + f->dump_string("release", ceph_release_to_str()); + f->dump_string("release_type", ceph_release_type()); } else if (command == "git_version") { - jf.dump_string("git_version", git_version_to_str()); + f->dump_string("git_version", git_version_to_str()); } ostringstream ss; - jf.close_section(); - jf.enable_line_break(); - jf.flush(ss); - out.append(ss.str()); + f->close_section(); } return 0; } @@ -569,20 +575,15 @@ class HelpHook : public AdminSocketHook { public: explicit HelpHook(AdminSocket *as) : m_as(as) {} int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) override { - std::unique_ptr f(Formatter::create(format, "json-pretty"sv, - "json-pretty"sv)); f->open_object_section("help"); for (const auto& [command, info] : m_as->hooks) { if (info.help.length()) f->dump_string(command.c_str(), info.help); } f->close_section(); - ostringstream ss; - f->flush(ss); - out.append(ss.str()); return 0; } }; @@ -592,30 +593,25 @@ class GetdescsHook : public AdminSocketHook { public: explicit GetdescsHook(AdminSocket *as) : m_as(as) {} int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) override { int cmdnum = 0; - JSONFormatter jf; - jf.open_object_section("command_descriptions"); + f->open_object_section("command_descriptions"); for (const auto& [command, info] : m_as->hooks) { // GCC 8 actually has [[maybe_unused]] on a structured binding // do what you'd expect. GCC 7 does not. (void)command; ostringstream secname; secname << "cmd" << setfill('0') << std::setw(3) << cmdnum; - dump_cmd_and_help_to_json(&jf, + dump_cmd_and_help_to_json(f, CEPH_FEATURES_ALL, secname.str().c_str(), info.desc, info.help); cmdnum++; } - jf.close_section(); // command_descriptions - jf.enable_line_break(); - ostringstream ss; - jf.flush(ss); - out.append(ss.str()); + f->close_section(); // command_descriptions return 0; } }; diff --git a/src/common/admin_socket.h b/src/common/admin_socket.h index 0e6db2fdd979..d9e100e006e4 100644 --- a/src/common/admin_socket.h +++ b/src/common/admin_socket.h @@ -40,19 +40,20 @@ public: virtual int call( std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, ceph::buffer::list& out) = 0; + virtual void call_async( std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, const bufferlist& inbl, std::function on_finish) { // by default, call the synchronous handler and then finish bufferlist out; std::ostringstream errss; - int r = call(command, cmdmap, format, errss, out); + int r = call(command, cmdmap, f, errss, out); on_finish(r, errss.str(), out); } virtual ~AdminSocketHook() {} diff --git a/src/common/ceph_context.cc b/src/common/ceph_context.cc index 9ffa2c78330e..ca469f4c2cbe 100644 --- a/src/common/ceph_context.cc +++ b/src/common/ceph_context.cc @@ -168,15 +168,13 @@ public: // AdminSocketHook int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) override { if (command == "dump_mempools") { - std::unique_ptr f(Formatter::create(format)); f->open_object_section("mempools"); - mempool::dump(f.get()); + mempool::dump(f); f->close_section(); - f->flush(out); return 0; } return -ENOSYS; @@ -441,11 +439,11 @@ public: explicit CephContextHook(CephContext *cct) : m_cct(cct) {} int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) override { try { - return m_cct->do_command(command, cmdmap, format, errss, &out); + return m_cct->do_command(command, cmdmap, f, errss, &out); } catch (const bad_cmd_get& e) { return -EINVAL; } @@ -453,12 +451,12 @@ public: }; int CephContext::do_command(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist *out) { try { - return _do_command(command, cmdmap, format, ss, out); + return _do_command(command, cmdmap, f, ss, out); } catch (const bad_cmd_get& e) { ss << e.what(); return -EINVAL; @@ -467,11 +465,10 @@ int CephContext::do_command(std::string_view command, const cmdmap_t& cmdmap, int CephContext::_do_command( std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist *out) { - Formatter *f = Formatter::create(format, "json-pretty", "json-pretty"); int r = 0; lgeneric_dout(this, 1) << "do_command '" << command << "' '" << cmdmap << "'" << dendl; @@ -631,10 +628,6 @@ int CephContext::_do_command( } f->close_section(); } - if (r >= 0) { - f->flush(*out); - } - delete f; lgeneric_dout(this, 1) << "do_command '" << command << "' '" << cmdmap << "' result is " << out->length() << " bytes" << dendl; return r; diff --git a/src/common/ceph_context.h b/src/common/ceph_context.h index 0928966fb8ab..684e89cdf531 100644 --- a/src/common/ceph_context.h +++ b/src/common/ceph_context.h @@ -162,11 +162,11 @@ public: * process an admin socket command */ int do_command(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, ceph::bufferlist *out); int _do_command(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, ceph::bufferlist *out); diff --git a/src/librbd/LibrbdAdminSocketHook.cc b/src/librbd/LibrbdAdminSocketHook.cc index 5322c5104b49..6b7a2b53c98d 100644 --- a/src/librbd/LibrbdAdminSocketHook.cc +++ b/src/librbd/LibrbdAdminSocketHook.cc @@ -17,20 +17,15 @@ namespace librbd { class LibrbdAdminSocketCommand { public: virtual ~LibrbdAdminSocketCommand() {} - virtual bool call(stringstream *ss) = 0; + virtual int call(Formatter *f) = 0; }; class FlushCacheCommand : public LibrbdAdminSocketCommand { public: explicit FlushCacheCommand(ImageCtx *ictx) : ictx(ictx) {} - bool call(stringstream *ss) override { - int r = ictx->io_work_queue->flush(); - if (r < 0) { - *ss << "flush: " << cpp_strerror(r); - return false; - } - return true; + int call(Formatter *f) override { + return ictx->io_work_queue->flush(); } private: @@ -41,13 +36,8 @@ struct InvalidateCacheCommand : public LibrbdAdminSocketCommand { public: explicit InvalidateCacheCommand(ImageCtx *ictx) : ictx(ictx) {} - bool call(stringstream *ss) override { - int r = invalidate_cache(ictx); - if (r < 0) { - *ss << "invalidate_cache: " << cpp_strerror(r); - return false; - } - return true; + int call(Formatter *f) override { + return invalidate_cache(ictx); } private: @@ -90,15 +80,12 @@ LibrbdAdminSocketHook::~LibrbdAdminSocketHook() { int LibrbdAdminSocketHook::call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) { Commands::const_iterator i = commands.find(command); ceph_assert(i != commands.end()); - stringstream outss; - int r = i->second->call(&outss); - out.append(outss); - return r; + return i->second->call(f); } } // namespace librbd diff --git a/src/librbd/LibrbdAdminSocketHook.h b/src/librbd/LibrbdAdminSocketHook.h index 4797c4ed3c36..d07a9280e583 100644 --- a/src/librbd/LibrbdAdminSocketHook.h +++ b/src/librbd/LibrbdAdminSocketHook.h @@ -18,7 +18,7 @@ namespace librbd { ~LibrbdAdminSocketHook() override; int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) override; diff --git a/src/mds/MDSDaemon.cc b/src/mds/MDSDaemon.cc index 6c64d2c6d234..75d80d54e6bf 100644 --- a/src/mds/MDSDaemon.cc +++ b/src/mds/MDSDaemon.cc @@ -114,22 +114,21 @@ class MDSSocketHook : public AdminSocketHook { public: explicit MDSSocketHook(MDSDaemon *m) : mds(m) {} int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist& out) override { stringstream outss; - int r = mds->asok_command(command, cmdmap, format, outss); + int r = mds->asok_command(command, cmdmap, f, outss); out.append(outss); return r; } }; int MDSDaemon::asok_command(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, std::ostream& ss) + Formatter *f, std::ostream& ss) { dout(1) << "asok_command: " << command << " (starting...)" << dendl; - Formatter *f = Formatter::create(format, "json-pretty", "json-pretty"); int r = -ENOSYS; if (command == "status") { dump_status(f); @@ -147,11 +146,7 @@ int MDSDaemon::asok_command(std::string_view command, const cmdmap_t& cmdmap, } } } - f->flush(ss); - delete f; - dout(1) << "asok_command: " << command << " (complete)" << dendl; - return r; } diff --git a/src/mds/MDSDaemon.h b/src/mds/MDSDaemon.h index 7448eed3d955..3be4795857c9 100644 --- a/src/mds/MDSDaemon.h +++ b/src/mds/MDSDaemon.h @@ -116,7 +116,7 @@ class MDSDaemon : public Dispatcher { void clean_up_admin_socket(); void check_ops_in_flight(); // send off any slow ops to monitor int asok_command(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, ostream& ss); + Formatter *f, ostream& ss); void dump_status(Formatter *f); diff --git a/src/mgr/ClusterState.cc b/src/mgr/ClusterState.cc index f9e8d8eb8444..a34fb1de8267 100644 --- a/src/mgr/ClusterState.cc +++ b/src/mgr/ClusterState.cc @@ -182,13 +182,13 @@ class ClusterSocketHook : public AdminSocketHook { public: explicit ClusterSocketHook(ClusterState *o) : cluster_state(o) {} int call(std::string_view admin_command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) override { stringstream outss; int r = 0; try { - r = cluster_state->asok_command(admin_command, cmdmap, format, outss); + r = cluster_state->asok_command(admin_command, cmdmap, f, outss); out.append(outss); } catch (const bad_cmd_get& e) { errss << e.what(); @@ -216,11 +216,13 @@ void ClusterState::shutdown() asok_hook = NULL; } -bool ClusterState::asok_command(std::string_view admin_command, const cmdmap_t& cmdmap, - std::string_view format, ostream& ss) +bool ClusterState::asok_command( + std::string_view admin_command, + const cmdmap_t& cmdmap, + Formatter *f, + ostream& ss) { std::lock_guard l(lock); - Formatter *f = Formatter::create(format, "json-pretty", "json-pretty"); if (admin_command == "dump_osd_network") { int64_t value = 0; // Default to health warning level if nothing specified @@ -367,7 +369,5 @@ bool ClusterState::asok_command(std::string_view admin_command, const cmdmap_t& } else { ceph_abort_msg("broken asok registration"); } - f->flush(ss); - delete f; return true; } diff --git a/src/mgr/ClusterState.h b/src/mgr/ClusterState.h index 66045194dd45..3b06808de86b 100644 --- a/src/mgr/ClusterState.h +++ b/src/mgr/ClusterState.h @@ -141,8 +141,10 @@ public: } void final_init(); void shutdown(); - bool asok_command(std::string_view admin_command, const cmdmap_t& cmdmap, - std::string_view format, ostream& ss); + bool asok_command(std::string_view admin_command, + const cmdmap_t& cmdmap, + Formatter *f, + ostream& ss); }; #endif diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc index 125e68d9daae..796dcfba1b45 100644 --- a/src/mon/Monitor.cc +++ b/src/mon/Monitor.cc @@ -267,23 +267,25 @@ class AdminHook : public AdminSocketHook { public: explicit AdminHook(Monitor *m) : mon(m) {} int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) override { stringstream outss; - mon->do_admin_command(command, cmdmap, format, outss); + mon->do_admin_command(command, cmdmap, f, errss, outss); out.append(outss); return 0; } }; -void Monitor::do_admin_command(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, std::ostream& ss) +void Monitor::do_admin_command( + std::string_view command, + const cmdmap_t& cmdmap, + Formatter *f, + std::ostream& err, + std::ostream& out) { std::lock_guard l(lock); - boost::scoped_ptr f(Formatter::create(format)); - string args; for (auto p = cmdmap.begin(); p != cmdmap.end(); ++p) { @@ -306,68 +308,52 @@ void Monitor::do_admin_command(std::string_view command, const cmdmap_t& cmdmap, << "cmd='" << command << "' args=" << args << ": dispatch"; if (command == "mon_status") { - get_mon_status(f.get(), ss); - if (f) - f->flush(ss); + get_mon_status(f, out); } else if (command == "quorum_status") { - _quorum_status(f.get(), ss); + _quorum_status(f, out); } else if (command == "sync_force") { string validate; if ((!cmd_getval(g_ceph_context, cmdmap, "validate", validate)) || (validate != "--yes-i-really-mean-it")) { - ss << "are you SURE? this will mean the monitor store will be erased " - "the next time the monitor is restarted. pass " - "'--yes-i-really-mean-it' if you really do."; + err << "are you SURE? this will mean the monitor store will be erased " + "the next time the monitor is restarted. pass " + "'--yes-i-really-mean-it' if you really do."; goto abort; } - sync_force(f.get(), ss); + sync_force(f, out); } else if (command.compare(0, 23, "add_bootstrap_peer_hint") == 0 || command.compare(0, 24, "add_bootstrap_peer_hintv") == 0) { - if (!_add_bootstrap_peer_hint(command, cmdmap, ss)) + if (!_add_bootstrap_peer_hint(command, cmdmap, out)) goto abort; } else if (command == "quorum enter") { elector.start_participating(); start_election(); - ss << "started responding to quorum, initiated new election"; + out << "started responding to quorum, initiated new election"; } else if (command == "quorum exit") { start_election(); elector.stop_participating(); - ss << "stopped responding to quorum, initiated new election"; + out << "stopped responding to quorum, initiated new election"; } else if (command == "ops") { - (void)op_tracker.dump_ops_in_flight(f.get()); - if (f) { - f->flush(ss); - } + (void)op_tracker.dump_ops_in_flight(f); } else if (command == "sessions") { - - if (f) { - f->open_array_section("sessions"); - for (auto p : session_map.sessions) { - f->dump_stream("session") << *p; - } - f->close_section(); - f->flush(ss); + f->open_array_section("sessions"); + for (auto p : session_map.sessions) { + f->dump_stream("session") << *p; } - + f->close_section(); } else if (command == "dump_historic_ops") { - if (op_tracker.dump_historic_ops(f.get())) { - f->flush(ss); - } else { - ss << "op_tracker tracking is not enabled now, so no ops are tracked currently, even those get stuck. \ + if (!op_tracker.dump_historic_ops(f)) { + err << "op_tracker tracking is not enabled now, so no ops are tracked currently, even those get stuck. \ please enable \"mon_enable_op_tracker\", and the tracker will start to track new ops received afterwards."; } } else if (command == "dump_historic_ops_by_duration" ) { - if (op_tracker.dump_historic_ops(f.get(), true)) { - f->flush(ss); - } else { - ss << "op_tracker tracking is not enabled now, so no ops are tracked currently, even those get stuck. \ + if (op_tracker.dump_historic_ops(f, true)) { + err << "op_tracker tracking is not enabled now, so no ops are tracked currently, even those get stuck. \ please enable \"mon_enable_op_tracker\", and the tracker will start to track new ops received afterwards."; } } else if (command == "dump_historic_slow_ops") { - if (op_tracker.dump_historic_slow_ops(f.get(), {})) { - f->flush(ss); - } else { - ss << "op_tracker tracking is not enabled now, so no ops are tracked currently, even those get stuck. \ + if (op_tracker.dump_historic_slow_ops(f, {})) { + err << "op_tracker tracking is not enabled now, so no ops are tracked currently, even those get stuck. \ please enable \"mon_enable_op_tracker\", and the tracker will start to track new ops received afterwards."; } } else { diff --git a/src/mon/Monitor.h b/src/mon/Monitor.h index fed943548391..355341b72ca6 100644 --- a/src/mon/Monitor.h +++ b/src/mon/Monitor.h @@ -989,7 +989,9 @@ private: int write_fsid(MonitorDBStore::TransactionRef t); void do_admin_command(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, std::ostream& ss); + Formatter *f, + std::ostream& err, + std::ostream& out); private: // don't allow copying diff --git a/src/os/bluestore/Allocator.cc b/src/os/bluestore/Allocator.cc index 48e4204a1b12..ea86b5645dbc 100644 --- a/src/os/bluestore/Allocator.cc +++ b/src/os/bluestore/Allocator.cc @@ -50,13 +50,13 @@ public: } } - int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + int call(std::string_view command, + const cmdmap_t& cmdmap, + Formatter *f, std::ostream& ss, bufferlist& out) override { int r = 0; if (command == "bluestore allocator dump " + name) { - Formatter *f = Formatter::create(format, "json-pretty", "json-pretty"); f->open_array_section("free_regions"); auto iterated_allocation = [&](size_t off, size_t len) { ceph_assert(len > 0); @@ -70,24 +70,15 @@ public: f->close_section(); }; alloc->dump(iterated_allocation); - - f->close_section(); - f->flush(out); } else if (command == "bluestore allocator score " + name) { - Formatter *f = Formatter::create(format, "json-pretty", "json-pretty"); f->open_object_section("fragmentation_score"); f->dump_float("fragmentation_rating", alloc->get_fragmentation_score()); f->close_section(); - f->flush(out); - delete f; } else if (command == "bluestore allocator fragmentation " + name) { - Formatter* f = Formatter::create(format, "json-pretty", "json-pretty"); f->open_object_section("fragmentation"); f->dump_float("fragmentation_rating", alloc->get_fragmentation()); f->close_section(); - f->flush(out); - delete f; } else { ss << "Invalid command" << std::endl; r = -ENOSYS; diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index 3ab9e3546961..59cdcc3b9100 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -74,7 +74,7 @@ private: SocketHook(BlueFS* bluefs) : bluefs(bluefs) {} int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist& out) override { if (command == "bluestore bluefs available") { @@ -86,7 +86,6 @@ private: } if (alloc_size == 0) alloc_size = bluefs->cct->_conf->bluefs_alloc_size; - Formatter *f = Formatter::create(format, "json-pretty", "json-pretty"); f->open_object_section("bluefs_available_space"); for (unsigned dev = BDEV_WAL; dev <= BDEV_SLOW; dev++) { if (bluefs->bdev[dev]) { @@ -103,8 +102,6 @@ private: } f->dump_int("available_from_bluestore", extra_space); f->close_section(); - f->flush(out); - delete f; } else { ss << "Invalid command" << std::endl; return -ENOSYS; diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index 584f8ba6c799..ab63822f0729 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -2324,7 +2324,7 @@ class OSDSocketHook : public AdminSocketHook { public: explicit OSDSocketHook(OSD *o) : osd(o) {} int call(std::string_view prefix, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist& out) override { ceph_abort("should use async hook"); @@ -2332,11 +2332,11 @@ public: void call_async( std::string_view prefix, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, const bufferlist& inbl, std::function on_finish) override { try { - osd->asok_command(prefix, cmdmap, format, inbl, on_finish); + osd->asok_command(prefix, cmdmap, f, inbl, on_finish); } catch (const bad_cmd_get& e) { bufferlist empty; on_finish(-EINVAL, e.what(), empty); @@ -2357,12 +2357,10 @@ std::set OSD::get_mapped_pools() void OSD::asok_command( std::string_view prefix, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, const bufferlist& inbl, std::function on_finish) { - std::unique_ptr f(Formatter::create( - format, "json-pretty", "json-pretty")); int ret = 0; stringstream ss; // stderr error message stream bufferlist outbl; // if empty at end, we'll dump formatter as output @@ -2452,35 +2450,35 @@ will start to track new ops received afterwards."; if (prefix == "dump_ops_in_flight" || prefix == "ops") { - if (!op_tracker.dump_ops_in_flight(f.get(), false, filters)) { + if (!op_tracker.dump_ops_in_flight(f, false, filters)) { ss << error_str; ret = -EINVAL; goto out; } } if (prefix == "dump_blocked_ops") { - if (!op_tracker.dump_ops_in_flight(f.get(), true, filters)) { + if (!op_tracker.dump_ops_in_flight(f, true, filters)) { ss << error_str; ret = -EINVAL; goto out; } } if (prefix == "dump_historic_ops") { - if (!op_tracker.dump_historic_ops(f.get(), false, filters)) { + if (!op_tracker.dump_historic_ops(f, false, filters)) { ss << error_str; ret = -EINVAL; goto out; } } if (prefix == "dump_historic_ops_by_duration") { - if (!op_tracker.dump_historic_ops(f.get(), true, filters)) { + if (!op_tracker.dump_historic_ops(f, true, filters)) { ss << error_str; ret = -EINVAL; goto out; } } if (prefix == "dump_historic_slow_ops") { - if (!op_tracker.dump_historic_slow_ops(f.get(), filters)) { + if (!op_tracker.dump_historic_slow_ops(f, filters)) { ss << error_str; ret = -EINVAL; goto out; @@ -2488,7 +2486,7 @@ will start to track new ops received afterwards."; } } else if (prefix == "dump_op_pq_state") { f->open_object_section("pq"); - op_shardedwq.dump(f.get()); + op_shardedwq.dump(f); f->close_section(); } else if (prefix == "dump_blacklist") { list > bl; @@ -2500,7 +2498,7 @@ will start to track new ops received afterwards."; it != bl.end(); ++it) { f->open_object_section("entry"); f->open_object_section("entity_addr_t"); - it->first.dump(f.get()); + it->first.dump(f); f->close_section(); //entity_addr_t it->second.localtime(f->dump_stream("expire_time")); f->close_section(); //entry @@ -2527,14 +2525,14 @@ will start to track new ops received afterwards."; f->dump_string("object", it->obj.oid.name); f->open_object_section("entity_name"); - it->wi.name.dump(f.get()); + it->wi.name.dump(f); f->close_section(); //entity_name_t f->dump_unsigned("cookie", it->wi.cookie); f->dump_unsigned("timeout", it->wi.timeout_seconds); f->open_object_section("entity_addr_t"); - it->wi.addr.dump(f.get()); + it->wi.addr.dump(f); f->close_section(); //entity_addr_t f->close_section(); //watch @@ -2544,10 +2542,10 @@ will start to track new ops received afterwards."; } else if (prefix == "dump_recovery_reservations") { f->open_object_section("reservations"); f->open_object_section("local_reservations"); - service.local_reserver.dump(f.get()); + service.local_reserver.dump(f); f->close_section(); f->open_object_section("remote_reservations"); - service.remote_reserver.dump(f.get()); + service.remote_reserver.dump(f); f->close_section(); f->close_section(); } else if (prefix == "dump_scrub_reservations") { @@ -2600,11 +2598,11 @@ will start to track new ops received afterwards."; f->dump_int("value", value); f->close_section(); } else if (prefix == "dump_objectstore_kv_stats") { - store->get_db_statistics(f.get()); + store->get_db_statistics(f); } else if (prefix == "dump_scrubs") { - service.dumps_scrub(f.get()); + service.dumps_scrub(f); } else if (prefix == "calc_objectstore_db_histogram") { - store->generate_db_histogram(f.get()); + store->generate_db_histogram(f); } else if (prefix == "flush_store_cache") { store->flush_cache(&ss); } else if (prefix == "dump_pgstate_history") { @@ -2616,7 +2614,7 @@ will start to track new ops received afterwards."; f->open_object_section("pg"); f->dump_stream("pg") << pg->pg_id; f->dump_string("currently", pg->get_current_state()); - pg->dump_pgstate_history(f.get()); + pg->dump_pgstate_history(f); f->close_section(); } f->close_section(); @@ -2857,7 +2855,7 @@ will start to track new ops received afterwards."; string s = stringify(pg->pg_id); f->open_array_section(s.c_str()); pg->lock(); - pg->dump_missing(f.get()); + pg->dump_missing(f); pg->unlock(); f->close_section(); } @@ -2893,7 +2891,7 @@ will start to track new ops received afterwards."; else if (prefix == "dump_pg_recovery_stats") { lock_guard l(osd_lock); - pg_recovery_stats.dump_formatted(f.get()); + pg_recovery_stats.dump_formatted(f); } else if (prefix == "reset_pg_recovery_stats") { @@ -2907,7 +2905,7 @@ will start to track new ops received afterwards."; cmd_getval(cct, cmdmap, "logger", logger); cmd_getval(cct, cmdmap, "counter", counter); cct->get_perfcounters_collection()->dump_formatted_histograms( - f.get(), false, logger, counter); + f, false, logger, counter); } else if (prefix == "cache drop") { @@ -2938,7 +2936,7 @@ will start to track new ops received afterwards."; } f->open_object_section("cache_status"); f->dump_int("object_ctx", obj_ctx_count); - store->dump_cache_stats(f.get()); + store->dump_cache_stats(f); f->close_section(); } @@ -3080,9 +3078,6 @@ will start to track new ops received afterwards."; } out: - if (ret >= 0 && outbl.length() == 0) { - f->flush(outbl); - } on_finish(ret, ss.str(), outbl); } @@ -3092,7 +3087,7 @@ class TestOpsSocketHook : public AdminSocketHook { public: TestOpsSocketHook(OSDService *s, ObjectStore *st) : service(s), store(st) {} int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) override { int r = 0; diff --git a/src/osd/OSD.h b/src/osd/OSD.h index c9df1db143f1..3736ebd13f95 100644 --- a/src/osd/OSD.h +++ b/src/osd/OSD.h @@ -1179,7 +1179,7 @@ protected: void asok_command( std::string_view prefix, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, const bufferlist& inbl, std::function on_finish); diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc index 1b1d6b10f298..07c637a303af 100644 --- a/src/osdc/Objecter.cc +++ b/src/osdc/Objecter.cc @@ -177,7 +177,7 @@ class Objecter::RequestStateHook : public AdminSocketHook { public: explicit RequestStateHook(Objecter *objecter); int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, ceph::buffer::list& out) override; }; @@ -4711,15 +4711,12 @@ Objecter::RequestStateHook::RequestStateHook(Objecter *objecter) : int Objecter::RequestStateHook::call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, ceph::buffer::list& out) { - Formatter *f = Formatter::create(format, "json-pretty", "json-pretty"); shared_lock rl(m_objecter->rwlock); m_objecter->dump_requests(f); - f->flush(out); - delete f; return 0; } diff --git a/src/rbd_replay/Replayer.cc b/src/rbd_replay/Replayer.cc index 335c0c50b0ab..b159d8a51ff3 100644 --- a/src/rbd_replay/Replayer.cc +++ b/src/rbd_replay/Replayer.cc @@ -300,11 +300,11 @@ void Replayer::erase_image(imagectx_id_t imagectx_id) { if (m_dump_perf_counters) { string command = "perf dump"; cmdmap_t cmdmap; - string format = "json-pretty"; + JSONFormatter jf(true); bufferlist out; stringstream ss; - g_ceph_context->do_command(command, cmdmap, format, ss, &out); - out.write_stream(cout); + g_ceph_context->do_command(command, cmdmap, &jf, ss, &out); + jf.flush(cout); cout << std::endl; cout.flush(); } @@ -370,12 +370,13 @@ void Replayer::clear_images() { if (m_dump_perf_counters && !m_images.empty()) { string command = "perf dump"; cmdmap_t cmdmap; - string format = "json-pretty"; + JSONFormatter jf(true); bufferlist out; stringstream ss; - g_ceph_context->do_command(command, cmdmap, format, ss, &out); - out.write_stream(cout); + g_ceph_context->do_command(command, cmdmap, &jf, ss, &out); + jf.flush(cout); cout << std::endl; + cout.flush(); } for (auto& p : m_images) { delete p.second; diff --git a/src/rgw/rgw_coroutine.cc b/src/rgw/rgw_coroutine.cc index 79a8b2f7caee..36ff88fe4630 100644 --- a/src/rgw/rgw_coroutine.cc +++ b/src/rgw/rgw_coroutine.cc @@ -854,13 +854,11 @@ int RGWCoroutinesManagerRegistry::hook_to_admin_command(const string& command) int RGWCoroutinesManagerRegistry::call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist& out) { std::shared_lock rl{lock}; - JSONFormatter f; - ::encode_json("cr_managers", *this, &f); - f.flush(out); + ::encode_json("cr_managers", *this, f); return 0; } diff --git a/src/rgw/rgw_coroutine.h b/src/rgw/rgw_coroutine.h index 8a8334f27183..a38421f4fc58 100644 --- a/src/rgw/rgw_coroutine.h +++ b/src/rgw/rgw_coroutine.h @@ -560,7 +560,7 @@ public: int hook_to_admin_command(const string& command); int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist& out) override; diff --git a/src/rgw/rgw_sync_trace.cc b/src/rgw/rgw_sync_trace.cc index dee57b224d9a..ddcdea249520 100644 --- a/src/rgw/rgw_sync_trace.cc +++ b/src/rgw/rgw_sync_trace.cc @@ -155,18 +155,18 @@ int RGWSyncTraceManager::hook_to_admin_command() return 0; } -static void dump_node(RGWSyncTraceNode *entry, bool show_history, JSONFormatter& f) +static void dump_node(RGWSyncTraceNode *entry, bool show_history, Formatter *f) { - f.open_object_section("entry"); - ::encode_json("status", entry->to_str(), &f); + f->open_object_section("entry"); + ::encode_json("status", entry->to_str(), f); if (show_history) { - f.open_array_section("history"); + f->open_array_section("history"); for (auto h : entry->get_history()) { - ::encode_json("entry", h, &f); + ::encode_json("entry", h, f); } - f.close_section(); + f->close_section(); } - f.close_section(); + f->close_section(); } string RGWSyncTraceManager::get_active_names() @@ -196,7 +196,7 @@ string RGWSyncTraceManager::get_active_names() } int RGWSyncTraceManager::call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist& out) { @@ -213,10 +213,8 @@ int RGWSyncTraceManager::call(std::string_view command, const cmdmap_t& cmdmap, shunique_lock rl(lock, ceph::acquire_shared); - JSONFormatter f(true); - - f.open_object_section("result"); - f.open_array_section("running"); + f->open_object_section("result"); + f->open_array_section("running"); for (auto n : nodes) { auto& entry = n.second; @@ -229,16 +227,16 @@ int RGWSyncTraceManager::call(std::string_view command, const cmdmap_t& cmdmap, if (show_short) { const string& name = entry->get_resource_name(); if (!name.empty()) { - ::encode_json("entry", name, &f); + ::encode_json("entry", name, f); } } else { dump_node(entry.get(), show_history, f); } - f.flush(out); + f->flush(out); } - f.close_section(); + f->close_section(); - f.open_array_section("complete"); + f->open_array_section("complete"); for (auto& entry : complete_nodes) { if (!search.empty() && !entry->match(search, show_history)) { continue; @@ -247,12 +245,11 @@ int RGWSyncTraceManager::call(std::string_view command, const cmdmap_t& cmdmap, continue; } dump_node(entry.get(), show_history, f); - f.flush(out); + f->flush(out); } - f.close_section(); + f->close_section(); - f.close_section(); - f.flush(out); + f->close_section(); return 0; } diff --git a/src/rgw/rgw_sync_trace.h b/src/rgw/rgw_sync_trace.h index 935f844d04c6..8e0a31cdfcb8 100644 --- a/src/rgw/rgw_sync_trace.h +++ b/src/rgw/rgw_sync_trace.h @@ -134,7 +134,7 @@ public: int hook_to_admin_command(); int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist& out) override; string get_active_names(); diff --git a/src/rgw/services/svc_sys_obj_cache.cc b/src/rgw/services/svc_sys_obj_cache.cc index d5b7f8b4aad4..8df76c85b592 100644 --- a/src/rgw/services/svc_sys_obj_cache.cc +++ b/src/rgw/services/svc_sys_obj_cache.cc @@ -518,7 +518,7 @@ public: void shutdown(); int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist& out) override; }; @@ -545,7 +545,7 @@ void RGWSI_SysObj_Cache_ASocketHook::shutdown() int RGWSI_SysObj_Cache_ASocketHook::call( std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist& out) { @@ -554,31 +554,17 @@ int RGWSI_SysObj_Cache_ASocketHook::call( if (auto i = cmdmap.find("filter"); i != cmdmap.cend()) { filter = boost::get(i->second); } - std::unique_ptr f(ceph::Formatter::create(format, "table")); - if (f) { - f->open_array_section("cache_entries"); - svc->asocket.call_list(filter, f.get()); - f->close_section(); - f->flush(out); - return 0; - } else { - ss << "Unable to create Formatter.\n"; - return -EINVAL; - } + f->open_array_section("cache_entries"); + svc->asocket.call_list(filter, f); + f->close_section(); + return 0; } else if (command == "cache inspect"sv) { - std::unique_ptr f(ceph::Formatter::create(format, "json-pretty")); - if (f) { - const auto& target = boost::get(cmdmap.at("target")); - if (svc->asocket.call_inspect(target, f.get())) { - f->flush(out); - return 0; - } else { - ss << "Unable to find entry "s + target + ".\n"; - return -ENOENT; - } + const auto& target = boost::get(cmdmap.at("target")); + if (svc->asocket.call_inspect(target, f)) { + return 0; } else { - ss << "Unable to create Formatter.\n"; - return -EINVAL; + ss << "Unable to find entry "s + target + ".\n"; + return -ENOENT; } } else if (command == "cache erase"sv) { const auto& target = boost::get(cmdmap.at("target")); diff --git a/src/test/admin_socket.cc b/src/test/admin_socket.cc index eba1a30f1bd8..36e6f3e7cf90 100644 --- a/src/test/admin_socket.cc +++ b/src/test/admin_socket.cc @@ -116,7 +116,7 @@ TEST(AdminSocket, SendTooLongRequest) { class MyTest : public AdminSocketHook { int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist& result) override { std::vector args; @@ -151,7 +151,7 @@ TEST(AdminSocket, RegisterCommand) { class MyTest2 : public AdminSocketHook { int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist& result) override { std::vector args; @@ -208,7 +208,7 @@ public: BlockingHook() = default; int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist& result) override { std::unique_lock l{_lock}; diff --git a/src/test/common/test_context.cc b/src/test/common/test_context.cc index f0f70dbdb31a..6b00f61c17bc 100644 --- a/src/test/common/test_context.cc +++ b/src/test/common/test_context.cc @@ -41,7 +41,9 @@ TEST(CephContext, do_command) { stringstream ss; bufferlist out; - cct->do_command("config get", cmdmap, "xml", ss, &out); + std::unique_ptr f(Formatter::create("xml", "xml")); + cct->do_command("config get", cmdmap, f.get(), ss, &out); + f->flush(out); string s(out.c_str(), out.length()); EXPECT_EQ("" + value + "", s); } @@ -50,7 +52,11 @@ TEST(CephContext, do_command) stringstream ss; bufferlist out; cmdmap_t bad_cmdmap; // no 'var' field - int r = cct->do_command("config get", bad_cmdmap, "xml", ss, &out); + std::unique_ptr f(Formatter::create("xml", "xml")); + int r = cct->do_command("config get", bad_cmdmap, f.get(), ss, &out); + if (r >= 0) { + f->flush(out); + } string s(out.c_str(), out.length()); EXPECT_EQ(-EINVAL, r); EXPECT_EQ("", s); @@ -61,7 +67,11 @@ TEST(CephContext, do_command) bufferlist out; cmdmap_t bad_cmdmap; bad_cmdmap["var"] = string("doesnotexist123"); - int r = cct->do_command("config help", bad_cmdmap, "xml", ss, &out); + std::unique_ptr f(Formatter::create("xml", "xml")); + int r = cct->do_command("config help", bad_cmdmap, f.get(), ss, &out); + if (r >= 0) { + f->flush(out); + } string s(out.c_str(), out.length()); EXPECT_EQ(-ENOENT, r); EXPECT_EQ("", s); @@ -71,15 +81,9 @@ TEST(CephContext, do_command) { stringstream ss; bufferlist out; - cct->do_command("config get", cmdmap, "UNSUPPORTED", ss, &out); - string s(out.c_str(), out.length()); - EXPECT_EQ("{\n \"key\": \"value\"\n}\n", s); - } - - { - stringstream ss; - bufferlist out; - cct->do_command("config diff get", cmdmap, "xml", ss, &out); + std::unique_ptr f(Formatter::create("xml", "xml")); + cct->do_command("config diff get", cmdmap, f.get(), ss, &out); + f->flush(out); string s(out.c_str(), out.length()); EXPECT_EQ("" + value + "value6161", s); } diff --git a/src/test/rbd_mirror/test_mock_InstanceReplayer.cc b/src/test/rbd_mirror/test_mock_InstanceReplayer.cc index 9779758c9d36..191f0e9437b6 100644 --- a/src/test/rbd_mirror/test_mock_InstanceReplayer.cc +++ b/src/test/rbd_mirror/test_mock_InstanceReplayer.cc @@ -90,7 +90,7 @@ struct ImageReplayer { MOCK_METHOD2(stop, void(Context *, bool)); MOCK_METHOD0(restart, void()); MOCK_METHOD0(flush, void()); - MOCK_METHOD2(print_status, void(Formatter *, stringstream *)); + MOCK_METHOD1(print_status, void(Formatter *)); MOCK_METHOD2(add_peer, void(const std::string &, librados::IoCtx &)); MOCK_METHOD0(get_global_image_id, const std::string &()); MOCK_METHOD0(get_local_image_id, const std::string &()); diff --git a/src/test/rbd_mirror/test_mock_PoolReplayer.cc b/src/test/rbd_mirror/test_mock_PoolReplayer.cc index b7a8ff557ab7..3b952eda738a 100644 --- a/src/test/rbd_mirror/test_mock_PoolReplayer.cc +++ b/src/test/rbd_mirror/test_mock_PoolReplayer.cc @@ -16,6 +16,7 @@ #include "tools/rbd_mirror/PoolReplayer.h" #include "tools/rbd_mirror/ServiceDaemon.h" #include "tools/rbd_mirror/Threads.h" +#include "common/Formatter.h" namespace librbd { @@ -116,7 +117,7 @@ struct Throttler { s_instance = nullptr; } - MOCK_METHOD2(print_status, void(Formatter*, std::stringstream*)); + MOCK_METHOD1(print_status, void(Formatter*)); }; Throttler* Throttler::s_instance = nullptr; @@ -154,7 +155,7 @@ struct NamespaceReplayer { MOCK_METHOD1(handle_instances_added, void(const std::vector &)); MOCK_METHOD1(handle_instances_removed, void(const std::vector &)); - MOCK_METHOD2(print_status, void(Formatter*, std::stringstream*)); + MOCK_METHOD1(print_status, void(Formatter*)); MOCK_METHOD0(start, void()); MOCK_METHOD0(stop, void()); MOCK_METHOD0(restart, void()); diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc index 1b16ac77e08e..e3c808c0b535 100644 --- a/src/tools/rbd_mirror/ImageDeleter.cc +++ b/src/tools/rbd_mirror/ImageDeleter.cc @@ -59,7 +59,7 @@ namespace { class ImageDeleterAdminSocketCommand { public: virtual ~ImageDeleterAdminSocketCommand() {} - virtual bool call(Formatter *f, stringstream *ss) = 0; + virtual int call(Formatter *f) = 0; }; template @@ -67,9 +67,9 @@ class StatusCommand : public ImageDeleterAdminSocketCommand { public: explicit StatusCommand(ImageDeleter *image_del) : image_del(image_del) {} - bool call(Formatter *f, stringstream *ss) override { - image_del->print_status(f, ss); - return true; + int call(Formatter *f) override { + image_del->print_status(f); + return 0; } private: @@ -106,17 +106,12 @@ public: } int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) override { Commands::const_iterator i = commands.find(command); ceph_assert(i != commands.end()); - Formatter *f = Formatter::create(format); - stringstream dss; - int r = i->second->call(f, &dss); - delete f; - out.append(dss); - return r; + return i->second->call(f); } private: @@ -308,33 +303,25 @@ ImageDeleter::find_delete_info(const std::string &image_id) { } template -void ImageDeleter::print_status(Formatter *f, stringstream *ss) { +void ImageDeleter::print_status(Formatter *f) { dout(20) << dendl; - if (f) { - f->open_object_section("image_deleter_status"); - f->open_array_section("delete_images_queue"); - } + f->open_object_section("image_deleter_status"); + f->open_array_section("delete_images_queue"); std::lock_guard l{m_lock}; for (const auto& image : m_delete_queue) { - image->print_status(f, ss); - } - - if (f) { - f->close_section(); - f->open_array_section("failed_deletes_queue"); + image->print_status(f); } + f->close_section(); + f->open_array_section("failed_deletes_queue"); for (const auto& image : m_retry_delete_queue) { - image->print_status(f, ss, true); + image->print_status(f, true); } - if (f) { - f->close_section(); - f->close_section(); - f->flush(*ss); - } + f->close_section(); + f->close_section(); } template @@ -544,20 +531,15 @@ void ImageDeleter::notify_on_delete(const std::string& image_id, } template -void ImageDeleter::DeleteInfo::print_status(Formatter *f, stringstream *ss, +void ImageDeleter::DeleteInfo::print_status(Formatter *f, bool print_failure_info) { - if (f) { - f->open_object_section("delete_info"); - f->dump_string("image_id", image_id); - if (print_failure_info) { - f->dump_string("error_code", cpp_strerror(error_code)); - f->dump_int("retries", retries); - } - f->close_section(); - f->flush(*ss); - } else { - *ss << *this; + f->open_object_section("delete_info"); + f->dump_string("image_id", image_id); + if (print_failure_info) { + f->dump_string("error_code", cpp_strerror(error_code)); + f->dump_int("retries", retries); } + f->close_section(); } } // namespace mirror diff --git a/src/tools/rbd_mirror/ImageDeleter.h b/src/tools/rbd_mirror/ImageDeleter.h index e5db2f05c693..65cb3a83296b 100644 --- a/src/tools/rbd_mirror/ImageDeleter.h +++ b/src/tools/rbd_mirror/ImageDeleter.h @@ -71,7 +71,7 @@ public: void init(Context* on_finish); void shut_down(Context* on_finish); - void print_status(Formatter *f, std::stringstream *ss); + void print_status(Formatter *f); // for testing purposes void wait_for_deletion(const std::string &image_id, @@ -119,7 +119,7 @@ private: return os; } - void print_status(Formatter *f, std::stringstream *ss, + void print_status(Formatter *f, bool print_failure_info=false); }; typedef std::shared_ptr DeleteInfoRef; diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc index 117f4f303694..b6f5de73be1e 100644 --- a/src/tools/rbd_mirror/ImageReplayer.cc +++ b/src/tools/rbd_mirror/ImageReplayer.cc @@ -85,7 +85,7 @@ public: : desc(desc), replayer(replayer) { } virtual ~ImageReplayerAdminSocketCommand() {} - virtual bool call(Formatter *f, stringstream *ss) = 0; + virtual int call(Formatter *f) = 0; std::string desc; ImageReplayer *replayer; @@ -99,9 +99,9 @@ public: : ImageReplayerAdminSocketCommand(desc, replayer) { } - bool call(Formatter *f, stringstream *ss) override { - this->replayer->print_status(f, ss); - return true; + int call(Formatter *f) override { + this->replayer->print_status(f); + return 0; } }; @@ -112,9 +112,9 @@ public: : ImageReplayerAdminSocketCommand(desc, replayer) { } - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { this->replayer->start(nullptr, true); - return true; + return 0; } }; @@ -125,9 +125,9 @@ public: : ImageReplayerAdminSocketCommand(desc, replayer) { } - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { this->replayer->stop(nullptr, true); - return true; + return 0; } }; @@ -138,9 +138,9 @@ public: : ImageReplayerAdminSocketCommand(desc, replayer) { } - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { this->replayer->restart(); - return true; + return 0; } }; @@ -151,9 +151,9 @@ public: : ImageReplayerAdminSocketCommand(desc, replayer) { } - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { this->replayer->flush(); - return true; + return 0; } }; @@ -196,17 +196,12 @@ public: } int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) override { auto i = commands.find(command); ceph_assert(i != commands.end()); - Formatter *f = Formatter::create(format); - stringstream dss; - int r = i->second->call(f, &dss); - delete f; - out.append(dss); - return r; + return i->second->call(f); } private: @@ -920,21 +915,16 @@ bool ImageReplayer::on_replay_interrupted() } template -void ImageReplayer::print_status(Formatter *f, stringstream *ss) +void ImageReplayer::print_status(Formatter *f) { dout(10) << dendl; std::lock_guard l{m_lock}; - if (f) { - f->open_object_section("image_replayer"); - f->dump_string("name", m_name); - f->dump_string("state", to_string(m_state)); - f->close_section(); - f->flush(*ss); - } else { - *ss << m_name << ": state: " << to_string(m_state); - } + f->open_object_section("image_replayer"); + f->dump_string("name", m_name); + f->dump_string("state", to_string(m_state)); + f->close_section(); } template diff --git a/src/tools/rbd_mirror/ImageReplayer.h b/src/tools/rbd_mirror/ImageReplayer.h index 1c1a442bdbb0..33bd8e811ee9 100644 --- a/src/tools/rbd_mirror/ImageReplayer.h +++ b/src/tools/rbd_mirror/ImageReplayer.h @@ -126,7 +126,7 @@ public: void resync_image(Context *on_finish=nullptr); - void print_status(Formatter *f, stringstream *ss); + void print_status(Formatter *f); virtual void handle_replay_ready(); virtual void handle_replay_complete(int r, const std::string &error_desc); diff --git a/src/tools/rbd_mirror/InstanceReplayer.cc b/src/tools/rbd_mirror/InstanceReplayer.cc index 689eee740c69..3a5ad0efa086 100644 --- a/src/tools/rbd_mirror/InstanceReplayer.cc +++ b/src/tools/rbd_mirror/InstanceReplayer.cc @@ -219,19 +219,15 @@ void InstanceReplayer::remove_peer_image(const std::string &global_image_id, } template -void InstanceReplayer::print_status(Formatter *f, stringstream *ss) { +void InstanceReplayer::print_status(Formatter *f) { dout(10) << dendl; - if (!f) { - return; - } - std::lock_guard locker{m_lock}; f->open_array_section("image_replayers"); for (auto &kv : m_image_replayers) { auto &image_replayer = kv.second; - image_replayer->print_status(f, ss); + image_replayer->print_status(f); } f->close_section(); } diff --git a/src/tools/rbd_mirror/InstanceReplayer.h b/src/tools/rbd_mirror/InstanceReplayer.h index a8f8e8ea42f9..a1e1c4816ea3 100644 --- a/src/tools/rbd_mirror/InstanceReplayer.h +++ b/src/tools/rbd_mirror/InstanceReplayer.h @@ -62,7 +62,7 @@ public: void release_all(Context *on_finish); - void print_status(Formatter *f, stringstream *ss); + void print_status(Formatter *f); void start(); void stop(); void restart(); diff --git a/src/tools/rbd_mirror/Mirror.cc b/src/tools/rbd_mirror/Mirror.cc index 6df32fb6455c..9b2596bbf732 100644 --- a/src/tools/rbd_mirror/Mirror.cc +++ b/src/tools/rbd_mirror/Mirror.cc @@ -37,16 +37,16 @@ namespace { class MirrorAdminSocketCommand { public: virtual ~MirrorAdminSocketCommand() {} - virtual bool call(Formatter *f, stringstream *ss) = 0; + virtual int call(Formatter *f) = 0; }; class StatusCommand : public MirrorAdminSocketCommand { public: explicit StatusCommand(Mirror *mirror) : mirror(mirror) {} - bool call(Formatter *f, stringstream *ss) override { - mirror->print_status(f, ss); - return true; + int call(Formatter *f) override { + mirror->print_status(f); + return 0; } private: @@ -57,9 +57,9 @@ class StartCommand : public MirrorAdminSocketCommand { public: explicit StartCommand(Mirror *mirror) : mirror(mirror) {} - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { mirror->start(); - return true; + return 0; } private: @@ -70,9 +70,9 @@ class StopCommand : public MirrorAdminSocketCommand { public: explicit StopCommand(Mirror *mirror) : mirror(mirror) {} - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { mirror->stop(); - return true; + return 0; } private: @@ -83,9 +83,9 @@ class RestartCommand : public MirrorAdminSocketCommand { public: explicit RestartCommand(Mirror *mirror) : mirror(mirror) {} - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { mirror->restart(); - return true; + return 0; } private: @@ -96,9 +96,9 @@ class FlushCommand : public MirrorAdminSocketCommand { public: explicit FlushCommand(Mirror *mirror) : mirror(mirror) {} - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { mirror->flush(); - return true; + return 0; } private: @@ -109,9 +109,9 @@ class LeaderReleaseCommand : public MirrorAdminSocketCommand { public: explicit LeaderReleaseCommand(Mirror *mirror) : mirror(mirror) {} - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { mirror->release_leader(); - return true; + return 0; } private: @@ -322,17 +322,12 @@ public: } int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& errss, bufferlist& out) override { Commands::const_iterator i = commands.find(command); ceph_assert(i != commands.end()); - Formatter *f = Formatter::create(format); - stringstream dss; - int r = i->second->call(f, &dss); - delete f; - out.append(dss); - return r; + return i->second->call(f); } private: @@ -568,7 +563,7 @@ void Mirror::run() dout(20) << "return" << dendl; } -void Mirror::print_status(Formatter *f, stringstream *ss) +void Mirror::print_status(Formatter *f) { dout(20) << "enter" << dendl; @@ -578,18 +573,13 @@ void Mirror::print_status(Formatter *f, stringstream *ss) return; } - if (f) { - f->open_object_section("mirror_status"); - f->open_array_section("pool_replayers"); - }; - + f->open_object_section("mirror_status"); + f->open_array_section("pool_replayers"); for (auto &pool_replayer : m_pool_replayers) { - pool_replayer.second->print_status(f, ss); - } - - if (f) { - f->close_section(); + pool_replayer.second->print_status(f); } + f->close_section(); + f->close_section(); } void Mirror::start() diff --git a/src/tools/rbd_mirror/Mirror.h b/src/tools/rbd_mirror/Mirror.h index 6af03af9f6de..9b1496285adb 100644 --- a/src/tools/rbd_mirror/Mirror.h +++ b/src/tools/rbd_mirror/Mirror.h @@ -46,7 +46,7 @@ public: void run(); void handle_signal(int signum); - void print_status(Formatter *f, stringstream *ss); + void print_status(Formatter *f); void start(); void stop(); void restart(); diff --git a/src/tools/rbd_mirror/NamespaceReplayer.cc b/src/tools/rbd_mirror/NamespaceReplayer.cc index fcc6bbd3d05e..99c062123651 100644 --- a/src/tools/rbd_mirror/NamespaceReplayer.cc +++ b/src/tools/rbd_mirror/NamespaceReplayer.cc @@ -109,7 +109,7 @@ void NamespaceReplayer::shut_down(Context *on_finish) { } template -void NamespaceReplayer::print_status(Formatter *f, stringstream *ss) +void NamespaceReplayer::print_status(Formatter *f) { dout(20) << dendl; @@ -117,11 +117,11 @@ void NamespaceReplayer::print_status(Formatter *f, stringstream *ss) std::lock_guard locker{m_lock}; - m_instance_replayer->print_status(f, ss); + m_instance_replayer->print_status(f); if (m_image_deleter) { f->open_object_section("image_deleter"); - m_image_deleter->print_status(f, ss); + m_image_deleter->print_status(f); f->close_section(); } } diff --git a/src/tools/rbd_mirror/NamespaceReplayer.h b/src/tools/rbd_mirror/NamespaceReplayer.h index 7e2b01a368d6..cf8e8211028e 100644 --- a/src/tools/rbd_mirror/NamespaceReplayer.h +++ b/src/tools/rbd_mirror/NamespaceReplayer.h @@ -83,7 +83,7 @@ public: void handle_instances_added(const std::vector &instance_ids); void handle_instances_removed(const std::vector &instance_ids); - void print_status(Formatter *f, stringstream *ss); + void print_status(Formatter *f); void start(); void stop(); void restart(); diff --git a/src/tools/rbd_mirror/PoolReplayer.cc b/src/tools/rbd_mirror/PoolReplayer.cc index 8844faa05279..e518f2d4f95a 100644 --- a/src/tools/rbd_mirror/PoolReplayer.cc +++ b/src/tools/rbd_mirror/PoolReplayer.cc @@ -43,7 +43,7 @@ public: : pool_replayer(pool_replayer) { } virtual ~PoolReplayerAdminSocketCommand() {} - virtual bool call(Formatter *f, stringstream *ss) = 0; + virtual int call(Formatter *f) = 0; protected: PoolReplayer *pool_replayer; }; @@ -55,9 +55,9 @@ public: : PoolReplayerAdminSocketCommand(pool_replayer) { } - bool call(Formatter *f, stringstream *ss) override { - this->pool_replayer->print_status(f, ss); - return true; + int call(Formatter *f) override { + this->pool_replayer->print_status(f); + return 0; } }; @@ -68,9 +68,9 @@ public: : PoolReplayerAdminSocketCommand(pool_replayer) { } - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { this->pool_replayer->start(); - return true; + return 0; } }; @@ -81,9 +81,9 @@ public: : PoolReplayerAdminSocketCommand(pool_replayer) { } - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { this->pool_replayer->stop(true); - return true; + return 0; } }; @@ -94,9 +94,9 @@ public: : PoolReplayerAdminSocketCommand(pool_replayer) { } - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { this->pool_replayer->restart(); - return true; + return 0; } }; @@ -107,9 +107,9 @@ public: : PoolReplayerAdminSocketCommand(pool_replayer) { } - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { this->pool_replayer->flush(); - return true; + return 0; } }; @@ -120,9 +120,9 @@ public: : PoolReplayerAdminSocketCommand(pool_replayer) { } - bool call(Formatter *f, stringstream *ss) override { + int call(Formatter *f) override { this->pool_replayer->release_leader(); - return true; + return 0; } }; @@ -186,17 +186,12 @@ public: } int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, + Formatter *f, std::ostream& ss, bufferlist& out) override { auto i = commands.find(command); ceph_assert(i != commands.end()); - Formatter *f = Formatter::create(format); - stringstream dss; - int r = i->second->call(f, &dss); - delete f; - out.append(dss); - return r; + return i->second->call(f); } private: @@ -729,12 +724,10 @@ void PoolReplayer::namespace_replayer_acquire_leader(const std::string &name, } template -void PoolReplayer::print_status(Formatter *f, stringstream *ss) { +void PoolReplayer::print_status(Formatter *f) { dout(20) << dendl; - if (!f) { - return; - } + assert(f); std::lock_guard l{m_lock}; @@ -776,29 +769,28 @@ void PoolReplayer::print_status(Formatter *f, stringstream *ss) { if (m_image_sync_throttler) { f->open_object_section("sync_throttler"); - m_image_sync_throttler->print_status(f, ss); + m_image_sync_throttler->print_status(f); f->close_section(); // sync_throttler } if (m_image_deletion_throttler) { f->open_object_section("deletion_throttler"); - m_image_deletion_throttler->print_status(f, ss); + m_image_deletion_throttler->print_status(f); f->close_section(); // deletion_throttler } - m_default_namespace_replayer->print_status(f, ss); + m_default_namespace_replayer->print_status(f); f->open_array_section("namespaces"); for (auto &it : m_namespace_replayers) { f->open_object_section("namespace"); f->dump_string("name", it.first); - it.second->print_status(f, ss); + it.second->print_status(f); f->close_section(); // namespace } f->close_section(); // namespaces f->close_section(); // pool_replayer_status - f->flush(*ss); } template diff --git a/src/tools/rbd_mirror/PoolReplayer.h b/src/tools/rbd_mirror/PoolReplayer.h index 73bdb32d54c9..9534d755c5f8 100644 --- a/src/tools/rbd_mirror/PoolReplayer.h +++ b/src/tools/rbd_mirror/PoolReplayer.h @@ -58,7 +58,7 @@ public: void run(); - void print_status(Formatter *f, stringstream *ss); + void print_status(Formatter *f); void start(); void stop(bool manual); void restart(); diff --git a/src/tools/rbd_mirror/Throttler.cc b/src/tools/rbd_mirror/Throttler.cc index ed377f1b061d..b2029896390c 100644 --- a/src/tools/rbd_mirror/Throttler.cc +++ b/src/tools/rbd_mirror/Throttler.cc @@ -211,22 +211,14 @@ void Throttler::set_max_concurrent_ops(uint32_t max) { } template -void Throttler::print_status(ceph::Formatter *f, std::stringstream *ss) { +void Throttler::print_status(ceph::Formatter *f) { dout(20) << dendl; std::lock_guard locker{m_lock}; - if (f) { - f->dump_int("max_parallel_requests", m_max_concurrent_ops); - f->dump_int("running_requests", m_inflight_ops.size()); - f->dump_int("waiting_requests", m_queue.size()); - f->flush(*ss); - } else { - *ss << "[ "; - *ss << "max_parallel_requests=" << m_max_concurrent_ops << ", "; - *ss << "running_requests=" << m_inflight_ops.size() << ", "; - *ss << "waiting_requests=" << m_queue.size() << " ]"; - } + f->dump_int("max_parallel_requests", m_max_concurrent_ops); + f->dump_int("running_requests", m_inflight_ops.size()); + f->dump_int("waiting_requests", m_queue.size()); } template diff --git a/src/tools/rbd_mirror/Throttler.h b/src/tools/rbd_mirror/Throttler.h index f92c032729bd..ae689de4838d 100644 --- a/src/tools/rbd_mirror/Throttler.h +++ b/src/tools/rbd_mirror/Throttler.h @@ -46,7 +46,7 @@ public: void finish_op(const std::string &ns, const std::string &id); void drain(const std::string &ns, int r); - void print_status(ceph::Formatter *f, std::stringstream *ss); + void print_status(ceph::Formatter *f); private: typedef std::pair Id;