From bf25f1657f02137fdd3d5eae31afbdc6dbeafb83 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Nov 2019 17:41:48 +0100 Subject: [PATCH 1/5] cgroup-util: add new cg_remove_xattr() for removing xattr from cgroup --- src/basic/cgroup-util.c | 17 +++++++++++++++++ src/basic/cgroup-util.h | 1 + 2 files changed, 18 insertions(+) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 5b94b02a27..54fc6ecf8b 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -605,6 +605,23 @@ int cg_get_xattr(const char *controller, const char *path, const char *name, voi return (int) n; } +int cg_remove_xattr(const char *controller, const char *path, const char *name) { + _cleanup_free_ char *fs = NULL; + int r; + + assert(path); + assert(name); + + r = cg_get_path(controller, path, NULL, &fs); + if (r < 0) + return r; + + if (removexattr(fs, name) < 0) + return -errno; + + return 0; +} + int cg_pid_get_path(const char *controller, pid_t pid, char **path) { _cleanup_fclose_ FILE *f = NULL; const char *fs, *controller_str; diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index a717029cbe..ad16619063 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -188,6 +188,7 @@ int cg_set_access(const char *controller, const char *path, uid_t uid, gid_t gid int cg_set_xattr(const char *controller, const char *path, const char *name, const void *value, size_t size, int flags); int cg_get_xattr(const char *controller, const char *path, const char *name, void *value, size_t size); +int cg_remove_xattr(const char *controller, const char *path, const char *name); int cg_install_release_agent(const char *controller, const char *agent); int cg_uninstall_release_agent(const char *controller); From 3288ea8f32e6f4742c5ce5ffa260252f7da959cf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Nov 2019 17:42:02 +0100 Subject: [PATCH 2/5] core: set "trusted.delegate" xattr on cgroups that are delegation boundaries Let's mark cgroups that are delegation boundaries to us. This can then be used by tools such as "systemd-cgls" to show where the next manager takes over. --- src/core/cgroup.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/core/cgroup.c b/src/core/cgroup.c index b7718e1966..abcd057d6a 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -617,15 +617,27 @@ static void cgroup_xattr_apply(Unit *u) { if (!MANAGER_IS_SYSTEM(u->manager)) return; - if (sd_id128_is_null(u->invocation_id)) - return; + if (!sd_id128_is_null(u->invocation_id)) { + r = cg_set_xattr(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, + "trusted.invocation_id", + sd_id128_to_string(u->invocation_id, ids), 32, + 0); + if (r < 0) + log_unit_debug_errno(u, r, "Failed to set invocation ID on control group %s, ignoring: %m", u->cgroup_path); + } - r = cg_set_xattr(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, - "trusted.invocation_id", - sd_id128_to_string(u->invocation_id, ids), 32, - 0); - if (r < 0) - log_unit_debug_errno(u, r, "Failed to set invocation ID on control group %s, ignoring: %m", u->cgroup_path); + if (unit_cgroup_delegate(u)) { + r = cg_set_xattr(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, + "trusted.delegate", + "1", 1, + 0); + if (r < 0) + log_unit_debug_errno(u, r, "Failed to set delegate flag on control group %s, ignoring: %m", u->cgroup_path); + } else { + r = cg_remove_xattr(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, "trusted.delegate"); + if (r != -ENODATA) + log_unit_debug_errno(u, r, "Failed to remove delegate flag on control group %s, ignoring: %m", u->cgroup_path); + } } static int lookup_block_device(const char *p, dev_t *ret) { From 74d8ccd451a975b27263226c877b7d3998c62c12 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Nov 2019 17:43:09 +0100 Subject: [PATCH 3/5] cgls: show delegation boundaries by underlining the cgroup in the output This should help visualize where one manager's territory begins and another's starts. Do this by underlining (since it's a "cut" point an underline made most sense to me). Since underlining is not visible on the console let's also show an ellipses for all lines that are delegation boundaries. Unfortunately this all is not as useful as it appears. The "trusted.delegate" xattr is only visible to roo, which means "systemd-cgls" has be called as root to show the boundaries. Unfortunately cgroupfs doesn't support unprivileged xattrs on cgroups. --- src/shared/cgroup-show.c | 50 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/src/shared/cgroup-show.c b/src/shared/cgroup-show.c index 4657e820d9..e07825e21e 100644 --- a/src/shared/cgroup-show.c +++ b/src/shared/cgroup-show.c @@ -17,12 +17,14 @@ #include "locale-util.h" #include "macro.h" #include "output-mode.h" +#include "parse-util.h" #include "path-util.h" #include "process-util.h" #include "sort-util.h" #include "string-util.h" #include "terminal-util.h" #include "unit-name.h" +#include "xattr-util.h" static void show_pid_array( pid_t pids[], @@ -117,6 +119,44 @@ static int show_cgroup_one_by_path( return 0; } +static int show_cgroup_name( + const char *path, + const char *prefix, + const char *glyph) { + + _cleanup_free_ char *b = NULL; + bool delegate = false; + int r; + + r = getxattr_malloc(path, "trusted.delegate", &b, false); + if (r < 0) { + if (r != -ENODATA) + log_debug_errno(r, "Failed to read trusted.delegate extended attribute: %m"); + } else { + r = parse_boolean(b); + if (r < 0) + log_debug_errno(r, "Failed to parse trusted.delegate extended attribute boolean value: %m"); + else + delegate = r > 0; + + b = mfree(b); + } + + b = strdup(basename(path)); + if (!b) + return -ENOMEM; + + printf("%s%s%s%s%s %s%s%s\n", + prefix, glyph, + delegate ? ansi_underline() : "", + cg_unescape(b), + delegate ? ansi_normal() : "", + delegate ? ansi_highlight() : "", + delegate ? special_glyph(SPECIAL_GLYPH_ELLIPSIS) : "", + delegate ? ansi_normal() : ""); + return 0; +} + int show_cgroup_by_path( const char *path, const char *prefix, @@ -125,8 +165,8 @@ int show_cgroup_by_path( _cleanup_free_ char *fn = NULL, *p1 = NULL, *last = NULL, *p2 = NULL; _cleanup_closedir_ DIR *d = NULL; - char *gn = NULL; bool shown_pids = false; + char *gn = NULL; int r; assert(path); @@ -161,7 +201,9 @@ int show_cgroup_by_path( } if (last) { - printf("%s%s%s\n", prefix, special_glyph(SPECIAL_GLYPH_TREE_BRANCH), cg_unescape(basename(last))); + r = show_cgroup_name(last, prefix, special_glyph(SPECIAL_GLYPH_TREE_BRANCH)); + if (r < 0) + return r; if (!p1) { p1 = strjoin(prefix, special_glyph(SPECIAL_GLYPH_TREE_VERTICAL)); @@ -183,7 +225,9 @@ int show_cgroup_by_path( show_cgroup_one_by_path(path, prefix, n_columns, !!last, flags); if (last) { - printf("%s%s%s\n", prefix, special_glyph(SPECIAL_GLYPH_TREE_RIGHT), cg_unescape(basename(last))); + r = show_cgroup_name(last, prefix, special_glyph(SPECIAL_GLYPH_TREE_RIGHT)); + if (r < 0) + return r; if (!p2) { p2 = strjoin(prefix, " "); From a2e361dc278d5663eae68bdc1ca5b954c9076c92 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Nov 2019 17:44:54 +0100 Subject: [PATCH 4/5] cgls: visually separate processes from cgroups Let's show them in grey, since we generally want to focus on showing the cgroups much less than the processes in them. --- src/shared/cgroup-show.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/cgroup-show.c b/src/shared/cgroup-show.c index e07825e21e..208d27df1a 100644 --- a/src/shared/cgroup-show.c +++ b/src/shared/cgroup-show.c @@ -71,7 +71,7 @@ static void show_pid_array( else printf("%s%s", prefix, special_glyph(((more || i < n_pids-1) ? SPECIAL_GLYPH_TREE_BRANCH : SPECIAL_GLYPH_TREE_RIGHT))); - printf("%*"PID_PRI" %s\n", pid_width, pids[i], strna(t)); + printf("%s%*"PID_PRI" %s%s\n", ansi_grey(), pid_width, pids[i], strna(t), ansi_normal()); } } From 7daa88ee5dd4080d1e17cb033f1b741a3ff2b511 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 Nov 2019 17:49:38 +0100 Subject: [PATCH 5/5] update TODO --- TODO | 4 ---- 1 file changed, 4 deletions(-) diff --git a/TODO b/TODO index c6052a7c4c..c3cc999ac4 100644 --- a/TODO +++ b/TODO @@ -166,10 +166,6 @@ Features: * sd-boot: optionally, show boot menu when previous default boot item has non-zero "tries done" count -* maybe set a special xattr on cgroups that have delegate=yes set, to make it - easy to mark cut points, then use this information in "systemd-cgls" to show - them (e.g. color delegated subtrees in a different color) - * introduce an option (or replacement) for "systemctl show" that outputs all properties as JSON, similar to busctl's new JSON output. In contrast to that it should skip the variant type string though.