From adf1486e46cb1f7820c77adb7b6436c47c743242 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Thu, 19 Sep 2019 11:11:55 -0500 Subject: [PATCH] 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 --- src/client/Client.cc | 12 ++-- src/client/Client.h | 2 +- src/common/admin_socket.cc | 56 +++++++-------- src/common/admin_socket.h | 7 +- src/common/ceph_context.cc | 21 ++---- src/common/ceph_context.h | 4 +- src/librbd/LibrbdAdminSocketHook.cc | 27 ++----- src/librbd/LibrbdAdminSocketHook.h | 2 +- src/mds/MDSDaemon.cc | 11 +-- src/mds/MDSDaemon.h | 2 +- src/mgr/ClusterState.cc | 14 ++-- src/mgr/ClusterState.h | 6 +- src/mon/Monitor.cc | 70 ++++++++----------- src/mon/Monitor.h | 4 +- src/os/bluestore/Allocator.cc | 15 +--- src/os/bluestore/BlueFS.cc | 5 +- src/osd/OSD.cc | 53 +++++++------- src/osd/OSD.h | 2 +- src/osdc/Objecter.cc | 7 +- src/rbd_replay/Replayer.cc | 13 ++-- src/rgw/rgw_coroutine.cc | 6 +- src/rgw/rgw_coroutine.h | 2 +- src/rgw/rgw_sync_trace.cc | 37 +++++----- src/rgw/rgw_sync_trace.h | 2 +- src/rgw/services/svc_sys_obj_cache.cc | 36 +++------- src/test/admin_socket.cc | 6 +- src/test/common/test_context.cc | 28 ++++---- .../rbd_mirror/test_mock_InstanceReplayer.cc | 2 +- src/test/rbd_mirror/test_mock_PoolReplayer.cc | 5 +- src/tools/rbd_mirror/ImageDeleter.cc | 62 ++++++---------- src/tools/rbd_mirror/ImageDeleter.h | 4 +- src/tools/rbd_mirror/ImageReplayer.cc | 48 +++++-------- src/tools/rbd_mirror/ImageReplayer.h | 2 +- src/tools/rbd_mirror/InstanceReplayer.cc | 8 +-- src/tools/rbd_mirror/InstanceReplayer.h | 2 +- src/tools/rbd_mirror/Mirror.cc | 54 ++++++-------- src/tools/rbd_mirror/Mirror.h | 2 +- src/tools/rbd_mirror/NamespaceReplayer.cc | 6 +- src/tools/rbd_mirror/NamespaceReplayer.h | 2 +- src/tools/rbd_mirror/PoolReplayer.cc | 52 ++++++-------- src/tools/rbd_mirror/PoolReplayer.h | 2 +- src/tools/rbd_mirror/Throttler.cc | 16 ++--- src/tools/rbd_mirror/Throttler.h | 2 +- 43 files changed, 294 insertions(+), 425 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index b911003f0eb..583f67a638a 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 30c208155fc..a88abb5d86e 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 7f26a9b31be..4dfe7bdec46 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 0e6db2fdd97..d9e100e006e 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 9ffa2c78330..ca469f4c2cb 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 0928966fb8a..684e89cdf53 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 5322c5104b4..6b7a2b53c98 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 4797c4ed3c3..d07a9280e58 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 6c64d2c6d23..75d80d54e6b 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 7448eed3d95..3be4795857c 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 f9e8d8eb844..a34fb1de826 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 66045194dd4..3b06808de86 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 125e68d9daa..796dcfba1b4 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 fed94354839..355341b72ca 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 48e4204a1b1..ea86b5645db 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 3ab9e354696..59cdcc3b910 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 584f8ba6c79..ab63822f072 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 c9df1db143f..3736ebd13f9 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 1b1d6b10f29..07c637a303a 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 335c0c50b0a..b159d8a51ff 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 79a8b2f7cae..36ff88fe463 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 8a8334f2718..a38421f4fc5 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 dee57b224d9..ddcdea24952 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 935f844d04c..8e0a31cdfcb 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 d5b7f8b4aad..8df76c85b59 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 eba1a30f1bd..36e6f3e7cf9 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 f0f70dbdb31..6b00f61c17b 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 9779758c9d3..191f0e9437b 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 b7a8ff557ab..3b952eda738 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 1b16ac77e08..e3c808c0b53 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 e5db2f05c69..65cb3a83296 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 117f4f30369..b6f5de73be1 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 1c1a442bdbb..33bd8e811ee 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 689eee740c6..3a5ad0efa08 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 a8f8e8ea42f..a1e1c4816ea 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 6df32fb6455..9b2596bbf73 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 6af03af9f6d..9b1496285ad 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 fcc6bbd3d05..99c06212365 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 7e2b01a368d..cf8e8211028 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 8844faa0527..e518f2d4f95 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 73bdb32d54c..9534d755c5f 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 ed377f1b061..b2029896390 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 f92c032729b..ae689de4838 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; -- 2.39.5