From 293b9aa3eb21d3eca4e34d9c6c42c7b6ab716bd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 14 Oct 2022 18:29:14 +0200 Subject: [PATCH 1/3] manager: rename dbus method Fixes #24989. --- man/org.freedesktop.systemd1.xml | 8 ++++---- src/analyze/analyze-dump.c | 4 ++-- src/core/dbus-manager.c | 6 +++--- src/core/org.freedesktop.systemd1.conf | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 9fb215a136..6986ee8b03 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -162,8 +162,8 @@ node /org/freedesktop/systemd1 { Subscribe(); Unsubscribe(); Dump(out s output); - DumpPatterns(in as patterns, - out s output); + DumpUnitsMatchingPatterns(in as patterns, + out s output); DumpByFileDescriptor(out h fd); Reload(); @org.freedesktop.DBus.Method.NoReply("true") @@ -870,7 +870,7 @@ node /org/freedesktop/systemd1 { - + @@ -1342,7 +1342,7 @@ node /org/freedesktop/systemd1 { string guaranteed, and new fields may be added any time, and old fields removed. The general structure may be rearranged drastically between releases. This is exposed by systemd-analyze1's - dump command. Similarly, DumpPatterns() returns the internal + dump command. Similarly, DumpUnitsMatchingPatterns() returns the internal state of units whose names match the glob expressions specified in the patterns argument. The DumpByFileDescriptor() method is identical to Dump() but returns the data serialized into a file descriptor (the client should diff --git a/src/analyze/analyze-dump.c b/src/analyze/analyze-dump.c index 220218e2fe..ac2b31ae26 100644 --- a/src/analyze/analyze-dump.c +++ b/src/analyze/analyze-dump.c @@ -36,7 +36,7 @@ static int dump_patterns(sd_bus *bus, char **patterns) { const char *text; int r; - r = bus_message_new_method_call(bus, &m, bus_systemd_mgr, "DumpPatterns"); + r = bus_message_new_method_call(bus, &m, bus_systemd_mgr, "DumpUnitsMatchingPatterns"); if (r < 0) return bus_log_create_error(r); @@ -58,7 +58,7 @@ static int dump_patterns(sd_bus *bus, char **patterns) { r = sd_bus_call(bus, m, 0, &error, &reply); if (r < 0) - return log_error_errno(r, "Failed to issue method call DumpPatterns: %s", + return log_error_errno(r, "Failed to issue method call DumpUnitsMatchingPatterns: %s", bus_error_message(&error, r)); r = sd_bus_message_read(reply, "s", &text); diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 240408f342..26d19e8ba0 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1392,7 +1392,7 @@ static int method_dump_by_fd(sd_bus_message *message, void *userdata, sd_bus_err return dump_impl(message, userdata, error, NULL, reply_dump_by_fd); } -static int method_dump_patterns(sd_bus_message *message, void *userdata, sd_bus_error *error) { +static int method_dump_units_matching_patterns(sd_bus_message *message, void *userdata, sd_bus_error *error) { _cleanup_strv_free_ char **patterns = NULL; int r; @@ -3027,10 +3027,10 @@ const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_RESULT("s", output), method_dump, SD_BUS_VTABLE_UNPRIVILEGED), - SD_BUS_METHOD_WITH_ARGS("DumpPatterns", + SD_BUS_METHOD_WITH_ARGS("DumpUnitsMatchingPatterns", SD_BUS_ARGS("as", patterns), SD_BUS_RESULT("s", output), - method_dump_patterns, + method_dump_units_matching_patterns, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD_WITH_ARGS("DumpByFileDescriptor", SD_BUS_NO_ARGS, diff --git a/src/core/org.freedesktop.systemd1.conf b/src/core/org.freedesktop.systemd1.conf index 8aa1b03a33..66dd961c44 100644 --- a/src/core/org.freedesktop.systemd1.conf +++ b/src/core/org.freedesktop.systemd1.conf @@ -122,7 +122,7 @@ + send_member="DumpUnitsMatchingPatterns"/> Date: Mon, 17 Oct 2022 14:23:04 +0200 Subject: [PATCH 2/3] manager: add DumpUnitsMatchingPatternsByFileDescriptor() --- man/org.freedesktop.systemd1.xml | 25 ++++++++++++++++--------- src/core/dbus-manager.c | 21 +++++++++++++++++++-- src/core/org.freedesktop.systemd1.conf | 4 ++++ 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 6986ee8b03..2c9fa81e18 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -165,6 +165,8 @@ node /org/freedesktop/systemd1 { DumpUnitsMatchingPatterns(in as patterns, out s output); DumpByFileDescriptor(out h fd); + DumpUnitsMatchingPatternsByFileDescriptor(in as patterns, + out h fd); Reload(); @org.freedesktop.DBus.Method.NoReply("true") Reexecute(); @@ -874,6 +876,8 @@ node /org/freedesktop/systemd1 { + + @@ -1342,15 +1346,18 @@ node /org/freedesktop/systemd1 { string guaranteed, and new fields may be added any time, and old fields removed. The general structure may be rearranged drastically between releases. This is exposed by systemd-analyze1's - dump command. Similarly, DumpUnitsMatchingPatterns() returns the internal - state of units whose names match the glob expressions specified in the patterns - argument. The DumpByFileDescriptor() method is identical to - Dump() but returns the data serialized into a file descriptor (the client should - read the text data from it until hitting EOF). Given the size limits on D-Bus messages and the possibly - large size of the returned string, DumpByFileDescriptor() is usually the - preferable interface, since it ensures the data can be passed reliably from the service manager to the - client. (Note though that DumpByFileDescriptor() cannot work when communicating - with the service manager remotely, as file descriptors are strictly local to a system.) + dump command. Similarly, DumpUnitsMatchingPatterns() returns + the internal state of units whose names match the glob expressions specified in the + patterns argument. The + DumpByFileDescriptor()/DumpUnitsMatchingPatternsByFileDescriptor() + methods are identical to Dump()/DumpUnitsMatchingPatterns(), + but return data serialized into a file descriptor (the client should read the text data from it until + hitting EOF). Given the size limits on D-Bus messages and the possibly large size of the returned + strings, + DumpByFileDescriptor()/DumpUnitsMatchingPatternsByFileDescriptor() + are usually the preferred interface, since it ensures the data can be passed reliably from the service + manager to the client. Note though that they cannot work when communicating with the service manager + remotely, as file descriptors are strictly local to a system. Reload() may be invoked to reload all unit files. diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 26d19e8ba0..e8c407c300 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -1392,7 +1392,11 @@ static int method_dump_by_fd(sd_bus_message *message, void *userdata, sd_bus_err return dump_impl(message, userdata, error, NULL, reply_dump_by_fd); } -static int method_dump_units_matching_patterns(sd_bus_message *message, void *userdata, sd_bus_error *error) { +static int dump_units_matching_patterns( + sd_bus_message *message, + void *userdata, + sd_bus_error *error, + int (*reply)(sd_bus_message *, char *)) { _cleanup_strv_free_ char **patterns = NULL; int r; @@ -1400,7 +1404,15 @@ static int method_dump_units_matching_patterns(sd_bus_message *message, void *us if (r < 0) return r; - return dump_impl(message, userdata, error, patterns, reply_dump); + return dump_impl(message, userdata, error, patterns, reply); +} + +static int method_dump_units_matching_patterns(sd_bus_message *message, void *userdata, sd_bus_error *error) { + return dump_units_matching_patterns(message, userdata, error, reply_dump); +} + +static int method_dump_units_matching_patterns_by_fd(sd_bus_message *message, void *userdata, sd_bus_error *error) { + return dump_units_matching_patterns(message, userdata, error, reply_dump_by_fd); } static int method_refuse_snapshot(sd_bus_message *message, void *userdata, sd_bus_error *error) { @@ -3037,6 +3049,11 @@ const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_RESULT("h", fd), method_dump_by_fd, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD_WITH_ARGS("DumpUnitsMatchingPatternsByFileDescriptor", + SD_BUS_ARGS("as", patterns), + SD_BUS_RESULT("h", fd), + method_dump_units_matching_patterns_by_fd, + SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD_WITH_ARGS("CreateSnapshot", SD_BUS_ARGS("s", name, "b", cleanup), SD_BUS_RESULT("o", unit), diff --git a/src/core/org.freedesktop.systemd1.conf b/src/core/org.freedesktop.systemd1.conf index 66dd961c44..fe9de012b6 100644 --- a/src/core/org.freedesktop.systemd1.conf +++ b/src/core/org.freedesktop.systemd1.conf @@ -124,6 +124,10 @@ send_interface="org.freedesktop.systemd1.Manager" send_member="DumpUnitsMatchingPatterns"/> + + From b08d86dc4a5179ca879074a83de48c69f289612e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 17 Oct 2022 15:03:16 +0200 Subject: [PATCH 3/3] analyze: use DumpUnitsMatchingPatternsByFileDescriptor Similarly to DumpByFileDescriptor vs Dump, DumpUnitsMatchingPatternsByFileDescriptor is used in preference. Dissimilarly, a fallback to DumpUnitsMatchingPatterns is not done on error, because there is no need for backwards compatibility. The code is still more verbose than I'd like, but there are four different code paths with slightly different rules in each case, so it's hard to make this all very brief. Since we have a separate file dedicated to making those calls, the verbose-but-easy-to-follow implementation should be OK. Closes #24989. I only did a quick test that all both variants works locally and over ssh. --- src/analyze/analyze-dump.c | 139 +++++++++++++++++++++++++------------ 1 file changed, 94 insertions(+), 45 deletions(-) diff --git a/src/analyze/analyze-dump.c b/src/analyze/analyze-dump.c index ac2b31ae26..2e838c906f 100644 --- a/src/analyze/analyze-dump.c +++ b/src/analyze/analyze-dump.c @@ -12,14 +12,72 @@ static int dump_fallback(sd_bus *bus) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - const char *text = NULL; + const char *text; int r; assert(bus); r = bus_call_method(bus, bus_systemd_mgr, "Dump", &error, &reply, NULL); if (r < 0) - return log_error_errno(r, "Failed to issue method call Dump: %s", bus_error_message(&error, r)); + return log_error_errno(r, "Failed to call Dump: %s", bus_error_message(&error, r)); + + r = sd_bus_message_read(reply, "s", &text); + if (r < 0) + return bus_log_parse_error(r); + + fputs(text, stdout); + return 0; +} + +static int dump_fd_reply(sd_bus_message *message) { + int fd, r; + + r = sd_bus_message_read(message, "h", &fd); + if (r < 0) + return bus_log_parse_error(r); + + fflush(stdout); + r = copy_bytes(fd, STDOUT_FILENO, UINT64_MAX, 0); + if (r < 0) + return r; + + return 1; /* Success */ +} + +static int dump(sd_bus *bus) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + int r; + + r = bus_call_method(bus, bus_systemd_mgr, "DumpByFileDescriptor", &error, &reply, NULL); + if (IN_SET(r, -EACCES, -EBADR)) + return 0; /* Fall back to non-fd method. We need to do this even if the bus supports sending + * fds to cater to very old managers which didn't have the fd-based method. */ + if (r < 0) + return log_error_errno(r, "Failed to call DumpByFileDescriptor: %s", + bus_error_message(&error, r)); + + return dump_fd_reply(reply); +} + +static int dump_patterns_fallback(sd_bus *bus, char **patterns) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL, *m = NULL; + const char *text; + int r; + + r = bus_message_new_method_call(bus, &m, bus_systemd_mgr, "DumpUnitsMatchingPatterns"); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_message_append_strv(m, patterns); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_call(bus, m, 0, &error, &reply); + if (r < 0) + return log_error_errno(r, "Failed to call DumpUnitsMatchingPatterns: %s", + bus_error_message(&error, r)); r = sd_bus_message_read(reply, "s", &text); if (r < 0) @@ -32,48 +90,50 @@ static int dump_fallback(sd_bus *bus) { static int dump_patterns(sd_bus *bus, char **patterns) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL, *m = NULL; - _cleanup_strv_free_ char **mangled = NULL; - const char *text; int r; - r = bus_message_new_method_call(bus, &m, bus_systemd_mgr, "DumpUnitsMatchingPatterns"); + r = bus_message_new_method_call(bus, &m, bus_systemd_mgr, "DumpUnitsMatchingPatternsByFileDescriptor"); if (r < 0) return bus_log_create_error(r); - STRV_FOREACH(pattern, patterns) { + r = sd_bus_message_append_strv(m, patterns); + if (r < 0) + return bus_log_create_error(r); + + r = sd_bus_call(bus, m, 0, &error, &reply); + if (r < 0) + return log_error_errno(r, "Failed to call DumpUnitsMatchingPatternsByFileDescriptor: %s", + bus_error_message(&error, r)); + + return dump_fd_reply(reply); +} + +static int mangle_patterns(char **args, char ***ret) { + _cleanup_strv_free_ char **mangled = NULL; + int r; + + STRV_FOREACH(arg, args) { char *t; - r = unit_name_mangle_with_suffix(*pattern, NULL, UNIT_NAME_MANGLE_GLOB, ".service", &t); + r = unit_name_mangle_with_suffix(*arg, NULL, UNIT_NAME_MANGLE_GLOB, ".service", &t); if (r < 0) - return log_error_errno(r, "Failed to mangle name: %m"); + return log_error_errno(r, "Failed to mangle name '%s': %m", *arg); r = strv_consume(&mangled, t); if (r < 0) return log_oom(); } - r = sd_bus_message_append_strv(m, mangled); - if (r < 0) - return bus_log_create_error(r); + if (strv_isempty(mangled)) + mangled = strv_free(mangled); - r = sd_bus_call(bus, m, 0, &error, &reply); - if (r < 0) - return log_error_errno(r, "Failed to issue method call DumpUnitsMatchingPatterns: %s", - bus_error_message(&error, r)); - - r = sd_bus_message_read(reply, "s", &text); - if (r < 0) - return bus_log_parse_error(r); - - fputs(text, stdout); - return r; + *ret = TAKE_PTR(mangled); + return 0; } int verb_dump(int argc, char *argv[], void *userdata) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; _cleanup_(sd_bus_flush_close_unrefp) sd_bus *bus = NULL; - int fd = -1; + _cleanup_strv_free_ char **patterns = NULL; int r; r = acquire_bus(&bus, NULL); @@ -82,28 +142,17 @@ int verb_dump(int argc, char *argv[], void *userdata) { pager_open(arg_pager_flags); - if (argc > 1) - return dump_patterns(bus, strv_skip(argv, 1)); - - if (!sd_bus_can_send(bus, SD_BUS_TYPE_UNIX_FD)) - return dump_fallback(bus); - - r = bus_call_method(bus, bus_systemd_mgr, "DumpByFileDescriptor", &error, &reply, NULL); - if (r < 0) { - /* fall back to Dump if DumpByFileDescriptor is not supported */ - if (!IN_SET(r, -EACCES, -EBADR)) - return log_error_errno(r, "Failed to issue method call DumpByFileDescriptor: %s", - bus_error_message(&error, r)); - - return dump_fallback(bus); - } - - r = sd_bus_message_read(reply, "h", &fd); + r = mangle_patterns(strv_skip(argv, 1), &patterns); if (r < 0) - return bus_log_parse_error(r); + return r; - fflush(stdout); - r = copy_bytes(fd, STDOUT_FILENO, UINT64_MAX, 0); + r = sd_bus_can_send(bus, SD_BUS_TYPE_UNIX_FD); + if (r < 0) + return log_error_errno(r, "Unable to determine if bus connection supports fd passing: %m"); + if (r > 0) + r = patterns ? dump_patterns(bus, patterns) : dump(bus); + if (r == 0) /* wasn't supported */ + r = patterns ? dump_patterns_fallback(bus, patterns) : dump_fallback(bus); if (r < 0) return r;