From a4c07734bf9e1da0c4f31c83f5dcb611cc8b449b Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 13 Sep 2019 14:13:50 -0500 Subject: [PATCH] common/admin_socket: pass ostream to call() for error output This separates the error stream from the output stream for the synchronous hook. This patch includes misc cleanup in the various implementations to make use of the new stream for errors. Add a test to unittest_context to ensure we're getting the error stream. Signed-off-by: Sage Weil --- src/client/Client.cc | 4 +- src/client/Client.h | 4 +- src/common/admin_socket.cc | 9 ++- src/common/admin_socket.h | 13 +++-- src/common/ceph_context.cc | 82 ++++++++++++++++----------- src/common/ceph_context.h | 8 ++- src/librbd/LibrbdAdminSocketHook.cc | 7 ++- src/librbd/LibrbdAdminSocketHook.h | 4 +- src/mds/MDSDaemon.cc | 10 ++-- src/mgr/ClusterState.cc | 12 ++-- src/mon/Monitor.cc | 10 ++-- src/os/bluestore/Allocator.cc | 12 ++-- src/os/bluestore/BlueFS.cc | 70 +++++++++++------------ src/osd/OSD.cc | 17 +++--- src/osdc/Objecter.cc | 5 +- src/rbd_replay/Replayer.cc | 6 +- src/rgw/rgw_coroutine.cc | 11 ++-- src/rgw/rgw_coroutine.h | 4 +- src/rgw/rgw_sync_trace.cc | 12 ++-- src/rgw/rgw_sync_trace.h | 4 +- src/rgw/services/svc_sys_obj_cache.cc | 16 ++++-- src/test/admin_socket.cc | 13 ++++- src/test/common/test_context.cc | 31 +++++++++- src/tools/rbd_mirror/ImageDeleter.cc | 10 ++-- src/tools/rbd_mirror/ImageReplayer.cc | 10 ++-- src/tools/rbd_mirror/Mirror.cc | 10 ++-- src/tools/rbd_mirror/PoolReplayer.cc | 10 ++-- 27 files changed, 253 insertions(+), 151 deletions(-) diff --git a/src/client/Client.cc b/src/client/Client.cc index 6081a96eef8..b911003f0eb 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -141,7 +141,9 @@ Client::CommandHook::CommandHook(Client *client) : int Client::CommandHook::call( std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) + std::string_view format, + std::ostream& errss, + bufferlist& out) { std::unique_ptr f(Formatter::create(format)); f->open_object_section("result"); diff --git a/src/client/Client.h b/src/client/Client.h index 9b634f54b10..30c208155fc 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -266,7 +266,9 @@ public: public: explicit CommandHook(Client *client); int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) override; + std::string_view format, + std::ostream& errss, + bufferlist& out) override; private: Client *m_client; }; diff --git a/src/common/admin_socket.cc b/src/common/admin_socket.cc index c5af715090b..7f26a9b31be 100644 --- a/src/common/admin_socket.cc +++ b/src/common/admin_socket.cc @@ -539,7 +539,9 @@ 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, bufferlist& out) override { + std::string_view format, + std::ostream& errss, + bufferlist& out) override { if (command == "0"sv) { out.append(CEPH_ADMIN_SOCK_VERSION); } else { @@ -568,6 +570,7 @@ public: explicit HelpHook(AdminSocket *as) : m_as(as) {} int call(std::string_view command, const cmdmap_t& cmdmap, std::string_view format, + std::ostream& errss, bufferlist& out) override { std::unique_ptr f(Formatter::create(format, "json-pretty"sv, "json-pretty"sv)); @@ -589,7 +592,9 @@ 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, bufferlist& out) override { + std::string_view format, + std::ostream& errss, + bufferlist& out) override { int cmdnum = 0; JSONFormatter jf; jf.open_object_section("command_descriptions"); diff --git a/src/common/admin_socket.h b/src/common/admin_socket.h index d38dd7c3998..0e6db2fdd97 100644 --- a/src/common/admin_socket.h +++ b/src/common/admin_socket.h @@ -37,8 +37,12 @@ class AdminSocketHook { public: // NOTE: the sync handler doesn't take an input buffer currently because // no users need it yet. - virtual int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, ceph::buffer::list& out) = 0; + virtual int call( + std::string_view command, + const cmdmap_t& cmdmap, + std::string_view format, + std::ostream& errss, + ceph::buffer::list& out) = 0; virtual void call_async( std::string_view command, const cmdmap_t& cmdmap, @@ -47,8 +51,9 @@ public: std::function on_finish) { // by default, call the synchronous handler and then finish bufferlist out; - int r = call(command, cmdmap, format, out); - on_finish(r, "", out); + std::ostringstream errss; + int r = call(command, cmdmap, format, 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 4437f034a4b..9ffa2c78330 100644 --- a/src/common/ceph_context.cc +++ b/src/common/ceph_context.cc @@ -168,7 +168,9 @@ public: // AdminSocketHook int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) override { + std::string_view format, + std::ostream& errss, + bufferlist& out) override { if (command == "dump_mempools") { std::unique_ptr f(Formatter::create(format)); f->open_object_section("mempools"); @@ -439,9 +441,11 @@ public: explicit CephContextHook(CephContext *cct) : m_cct(cct) {} int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) override { + std::string_view format, + std::ostream& errss, + bufferlist& out) override { try { - return m_cct->do_command(command, cmdmap, format, &out); + return m_cct->do_command(command, cmdmap, format, errss, &out); } catch (const bad_cmd_get& e) { return -EINVAL; } @@ -449,21 +453,35 @@ public: }; int CephContext::do_command(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist *out) + std::string_view format, + std::ostream& ss, + bufferlist *out) +{ + try { + return _do_command(command, cmdmap, format, ss, out); + } catch (const bad_cmd_get& e) { + ss << e.what(); + return -EINVAL; + } +} + +int CephContext::_do_command( + std::string_view command, const cmdmap_t& cmdmap, + std::string_view format, + std::ostream& ss, + bufferlist *out) { Formatter *f = Formatter::create(format, "json-pretty", "json-pretty"); - stringstream ss; int r = 0; - for (auto it = cmdmap.begin(); it != cmdmap.end(); ++it) { - if (it->first != "prefix") { - ss << it->first << ":" << cmd_vartype_stringify(it->second) << " "; - } - } - lgeneric_dout(this, 1) << "do_command '" << command << "' '" - << ss.str() << dendl; + lgeneric_dout(this, 1) << "do_command '" << command << "' '" << cmdmap << "'" + << dendl; ceph_assert_always(!(command == "assert" && _conf->debug_asok_assert_abort)); - if (command == "abort" && _conf->debug_asok_assert_abort) { - ceph_abort(); + if (command == "abort") { + if (_conf->debug_asok_assert_abort) { + ceph_abort(); + } else { + return -EPERM; + } } if (command == "perfcounters_dump" || command == "1" || command == "perf dump") { @@ -512,16 +530,14 @@ int CephContext::do_command(std::string_view command, const cmdmap_t& cmdmap, else if (command == "config unset") { std::string var; if (!(cmd_getval(this, cmdmap, "var", var))) { - f->dump_string("error", "syntax error: 'config unset '"); + r = -EINVAL; } else { r = _conf.rm_val(var.c_str()); if (r < 0 && r != -ENOENT) { - f->dump_stream("error") << "error unsetting '" << var << "': " - << cpp_strerror(r); + ss << "error unsetting '" << var << "': " + << cpp_strerror(r); } else { - ostringstream ss; _conf.apply_changes(&ss); - f->dump_string("success", ss.str()); } } @@ -532,32 +548,33 @@ int CephContext::do_command(std::string_view command, const cmdmap_t& cmdmap, if (!(cmd_getval(this, cmdmap, "var", var)) || !(cmd_getval(this, cmdmap, "val", val))) { - f->dump_string("error", "syntax error: 'config set '"); + r = -EINVAL; } else { // val may be multiple words string valstr = str_join(val, " "); r = _conf.set_val(var.c_str(), valstr.c_str()); if (r < 0) { - f->dump_stream("error") << "error setting '" << var << "' to '" << valstr << "': " << cpp_strerror(r); + ss << "error setting '" << var << "' to '" << valstr << "': " + << cpp_strerror(r); } else { - ostringstream ss; + stringstream ss; _conf.apply_changes(&ss); - f->dump_string("success", ss.str()); + f->dump_string("success", ss.str()); } } } else if (command == "config get") { std::string var; if (!cmd_getval(this, cmdmap, "var", var)) { - f->dump_string("error", "syntax error: 'config get '"); + r = -EINVAL; } else { char buf[4096]; memset(buf, 0, sizeof(buf)); char *tmp = buf; r = _conf.get_val(var.c_str(), &tmp, sizeof(buf)); if (r < 0) { - f->dump_stream("error") << "error getting '" << var << "': " << cpp_strerror(r); + ss << "error getting '" << var << "': " << cpp_strerror(r); } else { - f->dump_string(var.c_str(), buf); + f->dump_string(var.c_str(), buf); } } } else if (command == "config help") { @@ -567,9 +584,8 @@ int CephContext::do_command(std::string_view command, const cmdmap_t& cmdmap, std::string key = ConfFile::normalize_key_name(var); auto schema = _conf.get_schema(key); if (!schema) { - std::ostringstream msg; - msg << "Setting not found: '" << key << "'"; - f->dump_string("error", msg.str()); + ss << "Setting not found: '" << key << "'"; + r = -ENOENT; } else { f->dump_object("option", *schema); } @@ -615,10 +631,12 @@ int CephContext::do_command(std::string_view command, const cmdmap_t& cmdmap, } f->close_section(); } - f->flush(*out); + if (r >= 0) { + f->flush(*out); + } delete f; - lgeneric_dout(this, 1) << "do_command '" << command << "' '" << ss.str() - << "result is " << out->length() << " bytes" << dendl; + 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 733efeebc9a..0928966fb8a 100644 --- a/src/common/ceph_context.h +++ b/src/common/ceph_context.h @@ -162,7 +162,13 @@ public: * process an admin socket command */ int do_command(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, ceph::bufferlist *out); + std::string_view format, + std::ostream& errss, + ceph::bufferlist *out); + int _do_command(std::string_view command, const cmdmap_t& cmdmap, + std::string_view format, + std::ostream& errss, + ceph::bufferlist *out); static constexpr std::size_t largest_singleton = 8 * 72; diff --git a/src/librbd/LibrbdAdminSocketHook.cc b/src/librbd/LibrbdAdminSocketHook.cc index 35b7e3c16ad..5322c5104b4 100644 --- a/src/librbd/LibrbdAdminSocketHook.cc +++ b/src/librbd/LibrbdAdminSocketHook.cc @@ -91,12 +91,13 @@ LibrbdAdminSocketHook::~LibrbdAdminSocketHook() { int LibrbdAdminSocketHook::call(std::string_view command, const cmdmap_t& cmdmap, std::string_view format, + std::ostream& errss, bufferlist& out) { Commands::const_iterator i = commands.find(command); ceph_assert(i != commands.end()); - stringstream ss; - int r = i->second->call(&ss); - out.append(ss); + stringstream outss; + int r = i->second->call(&outss); + out.append(outss); return r; } diff --git a/src/librbd/LibrbdAdminSocketHook.h b/src/librbd/LibrbdAdminSocketHook.h index a06bcb11918..4797c4ed3c3 100644 --- a/src/librbd/LibrbdAdminSocketHook.h +++ b/src/librbd/LibrbdAdminSocketHook.h @@ -18,7 +18,9 @@ namespace librbd { ~LibrbdAdminSocketHook() override; int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) override; + std::string_view format, + std::ostream& errss, + bufferlist& out) override; private: typedef std::mapasok_command(command, cmdmap, format, ss); - out.append(ss); + std::string_view format, + std::ostream& ss, + bufferlist& out) override { + stringstream outss; + int r = mds->asok_command(command, cmdmap, format, outss); + out.append(outss); return r; } }; diff --git a/src/mgr/ClusterState.cc b/src/mgr/ClusterState.cc index 9c95bbb1661..f9e8d8eb844 100644 --- a/src/mgr/ClusterState.cc +++ b/src/mgr/ClusterState.cc @@ -182,16 +182,18 @@ 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, bufferlist& out) override { - stringstream ss; + std::string_view format, + std::ostream& errss, + bufferlist& out) override { + stringstream outss; int r = 0; try { - r = cluster_state->asok_command(admin_command, cmdmap, format, ss); + r = cluster_state->asok_command(admin_command, cmdmap, format, outss); + out.append(outss); } catch (const bad_cmd_get& e) { - ss << e.what(); + errss << e.what(); r = -EINVAL; } - out.append(ss); return r; } }; diff --git a/src/mon/Monitor.cc b/src/mon/Monitor.cc index a355807fb66..125e68d9daa 100644 --- a/src/mon/Monitor.cc +++ b/src/mon/Monitor.cc @@ -267,10 +267,12 @@ 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, bufferlist& out) override { - stringstream ss; - mon->do_admin_command(command, cmdmap, format, ss); - out.append(ss); + std::string_view format, + std::ostream& errss, + bufferlist& out) override { + stringstream outss; + mon->do_admin_command(command, cmdmap, format, outss); + out.append(outss); return 0; } }; diff --git a/src/os/bluestore/Allocator.cc b/src/os/bluestore/Allocator.cc index 341ea021c01..48e4204a1b1 100644 --- a/src/os/bluestore/Allocator.cc +++ b/src/os/bluestore/Allocator.cc @@ -51,8 +51,9 @@ public: } int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) override { - stringstream ss; + std::string_view format, + std::ostream& ss, + bufferlist& out) override { int r = 0; if (command == "bluestore allocator dump " + name) { Formatter *f = Formatter::create(format, "json-pretty", "json-pretty"); @@ -72,26 +73,25 @@ public: f->close_section(); - f->flush(ss); + 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(ss); + 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(ss); + f->flush(out); delete f; } else { ss << "Invalid command" << std::endl; r = -ENOSYS; } - out.append(ss); return r; } diff --git a/src/os/bluestore/BlueFS.cc b/src/os/bluestore/BlueFS.cc index e1c47d61304..3ab9e354696 100644 --- a/src/os/bluestore/BlueFS.cc +++ b/src/os/bluestore/BlueFS.cc @@ -74,43 +74,43 @@ private: SocketHook(BlueFS* bluefs) : bluefs(bluefs) {} int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) override { - stringstream ss; - bool r = 0; - if (command == "bluestore bluefs available") { - int64_t alloc_size = 0; - cmd_getval(bluefs->cct, cmdmap, "alloc_size", alloc_size); - if ((alloc_size & (alloc_size - 1)) != 0) { - ss << "Invalid allocation size:'" << alloc_size << std::endl; - } - 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]) { - f->open_object_section("dev"); - f->dump_string("device", bluefs->get_device_name(dev)); - ceph_assert(bluefs->alloc[dev]); - f->dump_int("free", bluefs->alloc[dev]->get_free()); - f->close_section(); - } - } - size_t extra_space = 0; - if (bluefs->slow_dev_expander) { - extra_space = bluefs->slow_dev_expander->available_freespace(alloc_size); - } - f->dump_int("available_from_bluestore", extra_space); - f->close_section(); - f->flush(ss); - delete f; - } else { - ss << "Invalid command" << std::endl; - r = -ENOSYS; + std::string_view format, + std::ostream& ss, + bufferlist& out) override { + if (command == "bluestore bluefs available") { + int64_t alloc_size = 0; + cmd_getval(bluefs->cct, cmdmap, "alloc_size", alloc_size); + if ((alloc_size & (alloc_size - 1)) != 0) { + ss << "Invalid allocation size:'" << alloc_size << std::endl; + return -EINVAL; } - out.append(ss); - return r; + 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]) { + f->open_object_section("dev"); + f->dump_string("device", bluefs->get_device_name(dev)); + ceph_assert(bluefs->alloc[dev]); + f->dump_int("free", bluefs->alloc[dev]->get_free()); + f->close_section(); + } + } + size_t extra_space = 0; + if (bluefs->slow_dev_expander) { + extra_space = bluefs->slow_dev_expander->available_freespace(alloc_size); + } + f->dump_int("available_from_bluestore", extra_space); + f->close_section(); + f->flush(out); + delete f; + } else { + ss << "Invalid command" << std::endl; + return -ENOSYS; } + return 0; + } }; BlueFS::BlueFS(CephContext* cct) diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc index 0170d100a9a..584f8ba6c79 100644 --- a/src/osd/OSD.cc +++ b/src/osd/OSD.cc @@ -2324,7 +2324,9 @@ 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, bufferlist& out) override { + std::string_view format, + std::ostream& ss, + bufferlist& out) override { ceph_abort("should use async hook"); } void call_async( @@ -2333,7 +2335,6 @@ public: std::string_view format, const bufferlist& inbl, std::function on_finish) override { - stringstream ss; try { osd->asok_command(prefix, cmdmap, format, inbl, on_finish); } catch (const bad_cmd_get& e) { @@ -3091,16 +3092,18 @@ 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, bufferlist& out) override { - stringstream ss; + std::string_view format, + std::ostream& errss, + bufferlist& out) override { int r = 0; + stringstream outss; try { - test_ops(service, store, command, cmdmap, ss); + test_ops(service, store, command, cmdmap, outss); + out.append(outss); } catch (const bad_cmd_get& e) { - ss << e.what(); + errss << e.what(); r = -EINVAL; } - out.append(ss); return r; } void test_ops(OSDService *service, ObjectStore *store, diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc index 6acece251ac..1b1d6b10f29 100644 --- a/src/osdc/Objecter.cc +++ b/src/osdc/Objecter.cc @@ -177,7 +177,9 @@ class Objecter::RequestStateHook : public AdminSocketHook { public: explicit RequestStateHook(Objecter *objecter); int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, ceph::buffer::list& out) override; + std::string_view format, + std::ostream& ss, + ceph::buffer::list& out) override; }; /** @@ -4710,6 +4712,7 @@ Objecter::RequestStateHook::RequestStateHook(Objecter *objecter) : int Objecter::RequestStateHook::call(std::string_view command, const cmdmap_t& cmdmap, std::string_view format, + std::ostream& ss, ceph::buffer::list& out) { Formatter *f = Formatter::create(format, "json-pretty", "json-pretty"); diff --git a/src/rbd_replay/Replayer.cc b/src/rbd_replay/Replayer.cc index 9cbcc348aa2..335c0c50b0a 100644 --- a/src/rbd_replay/Replayer.cc +++ b/src/rbd_replay/Replayer.cc @@ -302,7 +302,8 @@ void Replayer::erase_image(imagectx_id_t imagectx_id) { cmdmap_t cmdmap; string format = "json-pretty"; bufferlist out; - g_ceph_context->do_command(command, cmdmap, format, &out); + stringstream ss; + g_ceph_context->do_command(command, cmdmap, format, ss, &out); out.write_stream(cout); cout << std::endl; cout.flush(); @@ -371,7 +372,8 @@ void Replayer::clear_images() { cmdmap_t cmdmap; string format = "json-pretty"; bufferlist out; - g_ceph_context->do_command(command, cmdmap, format, &out); + stringstream ss; + g_ceph_context->do_command(command, cmdmap, format, ss, &out); out.write_stream(cout); cout << std::endl; } diff --git a/src/rgw/rgw_coroutine.cc b/src/rgw/rgw_coroutine.cc index 60d29bde9d0..79a8b2f7cae 100644 --- a/src/rgw/rgw_coroutine.cc +++ b/src/rgw/rgw_coroutine.cc @@ -853,15 +853,14 @@ int RGWCoroutinesManagerRegistry::hook_to_admin_command(const string& command) } int RGWCoroutinesManagerRegistry::call(std::string_view command, - const cmdmap_t& cmdmap, - std::string_view format, - bufferlist& out) { + const cmdmap_t& cmdmap, + std::string_view format, + std::ostream& ss, + bufferlist& out) { std::shared_lock rl{lock}; - stringstream ss; JSONFormatter f; ::encode_json("cr_managers", *this, &f); - f.flush(ss); - out.append(ss); + f.flush(out); return 0; } diff --git a/src/rgw/rgw_coroutine.h b/src/rgw/rgw_coroutine.h index 9e7ac5673e5..8a8334f2718 100644 --- a/src/rgw/rgw_coroutine.h +++ b/src/rgw/rgw_coroutine.h @@ -560,7 +560,9 @@ public: int hook_to_admin_command(const string& command); int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) override; + std::string_view format, + std::ostream& ss, + bufferlist& out) override; void dump(Formatter *f) const; }; diff --git a/src/rgw/rgw_sync_trace.cc b/src/rgw/rgw_sync_trace.cc index d086c06e987..dee57b224d9 100644 --- a/src/rgw/rgw_sync_trace.cc +++ b/src/rgw/rgw_sync_trace.cc @@ -196,7 +196,9 @@ string RGWSyncTraceManager::get_active_names() } int RGWSyncTraceManager::call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) { + std::string_view format, + std::ostream& ss, + bufferlist& out) { bool show_history = (command == "sync trace history"); bool show_short = (command == "sync trace active_short"); @@ -211,7 +213,6 @@ int RGWSyncTraceManager::call(std::string_view command, const cmdmap_t& cmdmap, shunique_lock rl(lock, ceph::acquire_shared); - stringstream ss; JSONFormatter f(true); f.open_object_section("result"); @@ -233,7 +234,7 @@ int RGWSyncTraceManager::call(std::string_view command, const cmdmap_t& cmdmap, } else { dump_node(entry.get(), show_history, f); } - f.flush(ss); + f.flush(out); } f.close_section(); @@ -246,13 +247,12 @@ int RGWSyncTraceManager::call(std::string_view command, const cmdmap_t& cmdmap, continue; } dump_node(entry.get(), show_history, f); - f.flush(ss); + f.flush(out); } f.close_section(); f.close_section(); - f.flush(ss); - out.append(ss); + f.flush(out); return 0; } diff --git a/src/rgw/rgw_sync_trace.h b/src/rgw/rgw_sync_trace.h index 9d5fb51b617..935f844d04c 100644 --- a/src/rgw/rgw_sync_trace.h +++ b/src/rgw/rgw_sync_trace.h @@ -134,7 +134,9 @@ public: int hook_to_admin_command(); int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) override; + std::string_view format, + 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 ea2833134a7..d5b7f8b4aad 100644 --- a/src/rgw/services/svc_sys_obj_cache.cc +++ b/src/rgw/services/svc_sys_obj_cache.cc @@ -518,7 +518,9 @@ public: void shutdown(); int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) override; + std::string_view format, + std::ostream& ss, + bufferlist& out) override; }; int RGWSI_SysObj_Cache_ASocketHook::start() @@ -543,7 +545,9 @@ void RGWSI_SysObj_Cache_ASocketHook::shutdown() int RGWSI_SysObj_Cache_ASocketHook::call( std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) + std::string_view format, + std::ostream& ss, + bufferlist& out) { if (command == "cache list"sv) { std::optional filter; @@ -558,7 +562,7 @@ int RGWSI_SysObj_Cache_ASocketHook::call( f->flush(out); return 0; } else { - out.append("Unable to create Formatter.\n"); + ss << "Unable to create Formatter.\n"; return -EINVAL; } } else if (command == "cache inspect"sv) { @@ -569,11 +573,11 @@ int RGWSI_SysObj_Cache_ASocketHook::call( f->flush(out); return 0; } else { - out.append("Unable to find entry "s + target + ".\n"); + ss << "Unable to find entry "s + target + ".\n"; return -ENOENT; } } else { - out.append("Unable to create Formatter.\n"); + ss << "Unable to create Formatter.\n"; return -EINVAL; } } else if (command == "cache erase"sv) { @@ -581,7 +585,7 @@ int RGWSI_SysObj_Cache_ASocketHook::call( if (svc->asocket.call_erase(target)) { return 0; } else { - out.append("Unable to find entry "s + target + ".\n"); + ss << "Unable to find entry "s + target + ".\n"; return -ENOENT; } } else if (command == "cache zap"sv) { diff --git a/src/test/admin_socket.cc b/src/test/admin_socket.cc index dfca141964e..eba1a30f1bd 100644 --- a/src/test/admin_socket.cc +++ b/src/test/admin_socket.cc @@ -116,7 +116,9 @@ TEST(AdminSocket, SendTooLongRequest) { class MyTest : public AdminSocketHook { int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& result) override { + std::string_view format, + std::ostream& ss, + bufferlist& result) override { std::vector args; cmd_getval(g_ceph_context, cmdmap, "args", args); result.append(command); @@ -149,7 +151,9 @@ TEST(AdminSocket, RegisterCommand) { class MyTest2 : public AdminSocketHook { int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& result) override { + std::string_view format, + std::ostream& ss, + bufferlist& result) override { std::vector args; cmd_getval(g_ceph_context, cmdmap, "args", args); result.append(command); @@ -162,6 +166,7 @@ class MyTest2 : public AdminSocketHook { resultstr += *it; } result.append(resultstr); + ss << "error stream"; return 0; } }; @@ -203,7 +208,9 @@ public: BlockingHook() = default; int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& result) override { + std::string_view format, + std::ostream& ss, + bufferlist& result) override { std::unique_lock l{_lock}; _cond.wait(l); return 0; diff --git a/src/test/common/test_context.cc b/src/test/common/test_context.cc index 9aec8911445..f0f70dbdb31 100644 --- a/src/test/common/test_context.cc +++ b/src/test/common/test_context.cc @@ -39,22 +39,47 @@ TEST(CephContext, do_command) cmdmap["var"] = key; { + stringstream ss; bufferlist out; - cct->do_command("config get", cmdmap, "xml", &out); + cct->do_command("config get", cmdmap, "xml", ss, &out); string s(out.c_str(), out.length()); EXPECT_EQ("" + value + "", s); } { + stringstream ss; bufferlist out; - cct->do_command("config get", cmdmap, "UNSUPPORTED", &out); + cmdmap_t bad_cmdmap; // no 'var' field + int r = cct->do_command("config get", bad_cmdmap, "xml", ss, &out); + string s(out.c_str(), out.length()); + EXPECT_EQ(-EINVAL, r); + EXPECT_EQ("", s); + EXPECT_EQ("", ss.str()); // no error string :/ + } + { + stringstream ss; + bufferlist out; + cmdmap_t bad_cmdmap; + bad_cmdmap["var"] = string("doesnotexist123"); + int r = cct->do_command("config help", bad_cmdmap, "xml", ss, &out); + string s(out.c_str(), out.length()); + EXPECT_EQ(-ENOENT, r); + EXPECT_EQ("", s); + EXPECT_EQ("Setting not found: 'doesnotexist123'", ss.str()); + } + + { + 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", &out); + cct->do_command("config diff get", cmdmap, "xml", ss, &out); string s(out.c_str(), out.length()); EXPECT_EQ("" + value + "value6161", s); } diff --git a/src/tools/rbd_mirror/ImageDeleter.cc b/src/tools/rbd_mirror/ImageDeleter.cc index aa49e24aadc..1b16ac77e08 100644 --- a/src/tools/rbd_mirror/ImageDeleter.cc +++ b/src/tools/rbd_mirror/ImageDeleter.cc @@ -106,14 +106,16 @@ public: } int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) override { + std::string_view format, + std::ostream& errss, + bufferlist& out) override { Commands::const_iterator i = commands.find(command); ceph_assert(i != commands.end()); Formatter *f = Formatter::create(format); - stringstream ss; - int r = i->second->call(f, &ss); + stringstream dss; + int r = i->second->call(f, &dss); delete f; - out.append(ss); + out.append(dss); return r; } diff --git a/src/tools/rbd_mirror/ImageReplayer.cc b/src/tools/rbd_mirror/ImageReplayer.cc index d87cb20f030..117f4f30369 100644 --- a/src/tools/rbd_mirror/ImageReplayer.cc +++ b/src/tools/rbd_mirror/ImageReplayer.cc @@ -196,14 +196,16 @@ public: } int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) override { + std::string_view format, + std::ostream& errss, + bufferlist& out) override { auto i = commands.find(command); ceph_assert(i != commands.end()); Formatter *f = Formatter::create(format); - stringstream ss; - int r = i->second->call(f, &ss); + stringstream dss; + int r = i->second->call(f, &dss); delete f; - out.append(ss); + out.append(dss); return r; } diff --git a/src/tools/rbd_mirror/Mirror.cc b/src/tools/rbd_mirror/Mirror.cc index 980c6c713db..6df32fb6455 100644 --- a/src/tools/rbd_mirror/Mirror.cc +++ b/src/tools/rbd_mirror/Mirror.cc @@ -322,14 +322,16 @@ public: } int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) override { + std::string_view format, + std::ostream& errss, + bufferlist& out) override { Commands::const_iterator i = commands.find(command); ceph_assert(i != commands.end()); Formatter *f = Formatter::create(format); - stringstream ss; - int r = i->second->call(f, &ss); + stringstream dss; + int r = i->second->call(f, &dss); delete f; - out.append(ss); + out.append(dss); return r; } diff --git a/src/tools/rbd_mirror/PoolReplayer.cc b/src/tools/rbd_mirror/PoolReplayer.cc index 85afd0f0d1d..8844faa0527 100644 --- a/src/tools/rbd_mirror/PoolReplayer.cc +++ b/src/tools/rbd_mirror/PoolReplayer.cc @@ -186,14 +186,16 @@ public: } int call(std::string_view command, const cmdmap_t& cmdmap, - std::string_view format, bufferlist& out) override { + std::string_view format, + std::ostream& ss, + bufferlist& out) override { auto i = commands.find(command); ceph_assert(i != commands.end()); Formatter *f = Formatter::create(format); - stringstream ss; - int r = i->second->call(f, &ss); + stringstream dss; + int r = i->second->call(f, &dss); delete f; - out.append(ss); + out.append(dss); return r; } -- 2.39.5