From 76a9460d44b03a86691a8481544f4525bb43610a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 11 Sep 2018 12:56:06 +0900 Subject: [PATCH 1/8] core: fix assert() about number of built environment variables Follow-up for 4b58153dd22172d817055d2a09a0cdf3f4bd9db3 and fd63e712b2025d235ce4bfbb512fada10e2690b5. --- src/core/execute.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/execute.c b/src/core/execute.c index 501b367eae..5a460b1678 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1728,7 +1728,7 @@ static int build_environment( } our_env[n_env++] = NULL; - assert(n_env <= 12); + assert(n_env <= 14); *ret = TAKE_PTR(our_env); From 7c1cb6f1989074e144b1625607950fce80c951ec Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 11 Sep 2018 13:18:33 +0900 Subject: [PATCH 2/8] core: add one more assert() --- src/core/execute.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/execute.c b/src/core/execute.c index 5a460b1678..2cdb688c3f 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1620,6 +1620,7 @@ static int build_environment( assert(u); assert(c); + assert(p); assert(ret); our_env = new0(char*, 14); From 2b9a7d2e96f5f852cdf8cc704930ea2ee456f6a1 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 11 Sep 2018 13:22:31 +0900 Subject: [PATCH 3/8] strv: introduce strv_join_prefix() --- src/basic/strv.c | 10 +++++++--- src/basic/strv.h | 5 ++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/basic/strv.c b/src/basic/strv.c index abf3fc4c7b..493fad16a1 100644 --- a/src/basic/strv.c +++ b/src/basic/strv.c @@ -339,21 +339,22 @@ int strv_split_extract(char ***t, const char *s, const char *separators, Extract return (int) n; } -char *strv_join(char **l, const char *separator) { +char *strv_join_prefix(char **l, const char *separator, const char *prefix) { char *r, *e; char **s; - size_t n, k; + size_t n, k, m; if (!separator) separator = " "; k = strlen(separator); + m = strlen_ptr(prefix); n = 0; STRV_FOREACH(s, l) { if (s != l) n += k; - n += strlen(*s); + n += m + strlen(*s); } r = new(char, n+1); @@ -365,6 +366,9 @@ char *strv_join(char **l, const char *separator) { if (s != l) e = stpcpy(e, separator); + if (prefix) + e = stpcpy(e, prefix); + e = stpcpy(e, *s); } diff --git a/src/basic/strv.h b/src/basic/strv.h index 51d03db940..34a660cb92 100644 --- a/src/basic/strv.h +++ b/src/basic/strv.h @@ -71,7 +71,10 @@ char **strv_split_newlines(const char *s); int strv_split_extract(char ***t, const char *s, const char *separators, ExtractFlags flags); -char *strv_join(char **l, const char *separator); +char *strv_join_prefix(char **l, const char *separator, const char *prefix); +static inline char *strv_join(char **l, const char *separator) { + return strv_join_prefix(l, separator, NULL); +} char **strv_parse_nulstr(const char *s, size_t l); char **strv_split_nulstr(const char *s); From 474a595af7544a670de70e1eb873bcee1bb00044 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 11 Sep 2018 13:35:27 +0900 Subject: [PATCH 4/8] test: add tests for strv_join_prefix() --- src/test/test-strv.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/test/test-strv.c b/src/test/test-strv.c index 340b5628f9..6b024ff6a0 100644 --- a/src/test/test-strv.c +++ b/src/test/test-strv.c @@ -144,6 +144,38 @@ static void test_strv_join(void) { assert_se(streq(w, "")); } +static void test_strv_join_prefix(void) { + _cleanup_free_ char *p = NULL, *q = NULL, *r = NULL, *s = NULL, *t = NULL, *v = NULL, *w = NULL; + + p = strv_join_prefix((char **)input_table_multiple, ", ", "foo"); + assert_se(p); + assert_se(streq(p, "fooone, footwo, foothree")); + + q = strv_join_prefix((char **)input_table_multiple, ";", "foo"); + assert_se(q); + assert_se(streq(q, "fooone;footwo;foothree")); + + r = strv_join_prefix((char **)input_table_multiple, NULL, "foo"); + assert_se(r); + assert_se(streq(r, "fooone footwo foothree")); + + s = strv_join_prefix((char **)input_table_one, ", ", "foo"); + assert_se(s); + assert_se(streq(s, "fooone")); + + t = strv_join_prefix((char **)input_table_none, ", ", "foo"); + assert_se(t); + assert_se(streq(t, "")); + + v = strv_join_prefix((char **)input_table_two_empties, ", ", "foo"); + assert_se(v); + assert_se(streq(v, "foo, foo")); + + w = strv_join_prefix((char **)input_table_one_empty, ", ", "foo"); + assert_se(w); + assert_se(streq(w, "foo")); +} + static void test_strv_unquote(const char *quoted, char **list) { _cleanup_strv_free_ char **s; _cleanup_free_ char *j; @@ -726,6 +758,7 @@ int main(int argc, char *argv[]) { test_strv_find_prefix(); test_strv_find_startswith(); test_strv_join(); + test_strv_join_prefix(); test_strv_unquote(" foo=bar \"waldo\" zzz ", STRV_MAKE("foo=bar", "waldo", "zzz")); test_strv_unquote("", STRV_MAKE_EMPTY); From a2917d3d2a3ce926f74b63aa60a47f838a8e1f83 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 11 Sep 2018 13:41:09 +0900 Subject: [PATCH 5/8] test: replace swear words by 'hoge' --- src/test/test-cgroup-util.c | 2 +- src/test/test-strv.c | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/test-cgroup-util.c b/src/test/test-cgroup-util.c index d49356315e..081ef1be63 100644 --- a/src/test/test-cgroup-util.c +++ b/src/test/test-cgroup-util.c @@ -296,7 +296,7 @@ static void test_shift_path(void) { test_shift_path_one("/foobar/waldo", "/", "/foobar/waldo"); test_shift_path_one("/foobar/waldo", "", "/foobar/waldo"); test_shift_path_one("/foobar/waldo", "/foobar", "/waldo"); - test_shift_path_one("/foobar/waldo", "/fuckfuck", "/foobar/waldo"); + test_shift_path_one("/foobar/waldo", "/hogehoge", "/foobar/waldo"); } static void test_mask_supported(void) { diff --git a/src/test/test-strv.c b/src/test/test-strv.c index 6b024ff6a0..5b0b9547b7 100644 --- a/src/test/test-strv.c +++ b/src/test/test-strv.c @@ -284,18 +284,18 @@ static void test_strv_split_nulstr(void) { static void test_strv_parse_nulstr(void) { _cleanup_strv_free_ char **l = NULL; - const char nulstr[] = "fuck\0fuck2\0fuck3\0\0fuck5\0\0xxx"; + const char nulstr[] = "hoge\0hoge2\0hoge3\0\0hoge5\0\0xxx"; l = strv_parse_nulstr(nulstr, sizeof(nulstr)-1); assert_se(l); puts("Parse nulstr:"); strv_print(l); - assert_se(streq(l[0], "fuck")); - assert_se(streq(l[1], "fuck2")); - assert_se(streq(l[2], "fuck3")); + assert_se(streq(l[0], "hoge")); + assert_se(streq(l[1], "hoge2")); + assert_se(streq(l[2], "hoge3")); assert_se(streq(l[3], "")); - assert_se(streq(l[4], "fuck5")); + assert_se(streq(l[4], "hoge5")); assert_se(streq(l[5], "")); assert_se(streq(l[6], "xxx")); } From fb2042dd55de5019f55931b4f20a44700ec1222b Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 11 Sep 2018 14:05:08 +0900 Subject: [PATCH 6/8] core: add new environment variable $RUNTIME_DIRECTORY= or friends The variable is generated from RuntimeDirectory= or friends. If multiple directories are set, then they are concatenated with the separator ':'. --- TODO | 9 --------- src/core/execute.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/TODO b/TODO index 4bf66bd217..84f0514559 100644 --- a/TODO +++ b/TODO @@ -249,15 +249,6 @@ Features: for all units. It should be both a way to pin units into memory as well as a wait to retrieve their exit data. -* maybe set a new set of env vars for services, based on RuntimeDirectory=, - StateDirectory=, LogsDirectory=, CacheDirectory= and ConfigurationDirectory= - automatically. For example, there could be $RUNTIME_DIRECTORY, - $STATE_DIRECTORY, $LOGS_DIRECTORY=, $CACHE_DIRECTORY and - $CONFIGURATION_DIRECTORY or so. This could be useful to write services that - can adapt to varying directories for these purposes. Special care has to be - taken if multiple dirs are configured. Maybe avoid setting the env vars in - that case? - * expose IO accounting data on the bus, show it in systemd-run --wait and log about it in the resource log message diff --git a/src/core/execute.c b/src/core/execute.c index 2cdb688c3f..fe75417a58 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1602,6 +1602,8 @@ static void do_idle_pipe_dance(int idle_pipe[4]) { idle_pipe[3] = safe_close(idle_pipe[3]); } +static const char *exec_directory_env_name_to_string(ExecDirectoryType t); + static int build_environment( const Unit *u, const ExecContext *c, @@ -1615,6 +1617,7 @@ static int build_environment( char ***ret) { _cleanup_strv_free_ char **our_env = NULL; + ExecDirectoryType t; size_t n_env = 0; char *x; @@ -1623,7 +1626,7 @@ static int build_environment( assert(p); assert(ret); - our_env = new0(char*, 14); + our_env = new0(char*, 14 + _EXEC_DIRECTORY_TYPE_MAX); if (!our_env) return -ENOMEM; @@ -1728,8 +1731,37 @@ static int build_environment( our_env[n_env++] = x; } + for (t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) { + _cleanup_free_ char *pre = NULL, *joined = NULL; + const char *n; + + if (!p->prefix[t]) + continue; + + if (strv_isempty(c->directories[t].paths)) + continue; + + n = exec_directory_env_name_to_string(t); + if (!n) + continue; + + pre = strjoin(p->prefix[t], "/"); + if (!pre) + return -ENOMEM; + + joined = strv_join_prefix(c->directories[t].paths, ":", pre); + if (!joined) + return -ENOMEM; + + x = strjoin(n, "=", joined); + if (!x) + return -ENOMEM; + + our_env[n_env++] = x; + } + our_env[n_env++] = NULL; - assert(n_env <= 14); + assert(n_env <= 14 + _EXEC_DIRECTORY_TYPE_MAX); *ret = TAKE_PTR(our_env); @@ -5128,6 +5160,16 @@ static const char* const exec_directory_type_table[_EXEC_DIRECTORY_TYPE_MAX] = { DEFINE_STRING_TABLE_LOOKUP(exec_directory_type, ExecDirectoryType); +static const char* const exec_directory_env_name_table[_EXEC_DIRECTORY_TYPE_MAX] = { + [EXEC_DIRECTORY_RUNTIME] = "RUNTIME_DIRECTORY", + [EXEC_DIRECTORY_STATE] = "STATE_DIRECTORY", + [EXEC_DIRECTORY_CACHE] = "CACHE_DIRECTORY", + [EXEC_DIRECTORY_LOGS] = "LOGS_DIRECTORY", + [EXEC_DIRECTORY_CONFIGURATION] = "CONFIGURATION_DIRECTORY", +}; + +DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(exec_directory_env_name, ExecDirectoryType); + static const char* const exec_keyring_mode_table[_EXEC_KEYRING_MODE_MAX] = { [EXEC_KEYRING_INHERIT] = "inherit", [EXEC_KEYRING_PRIVATE] = "private", From 6088662d57bbd81167bd272d385fdd1044b287ec Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 11 Sep 2018 14:07:53 +0900 Subject: [PATCH 7/8] test-execute: add tests for $RUNTIME_DIRECTORY= or friends --- .../test-execute/exec-dynamicuser-statedir-migrate-step1.service | 1 + .../test-execute/exec-dynamicuser-statedir-migrate-step2.service | 1 + test/test-execute/exec-dynamicuser-statedir.service | 1 + test/test-execute/exec-runtimedirectory-mode.service | 1 + test/test-execute/exec-runtimedirectory.service | 1 + 5 files changed, 5 insertions(+) diff --git a/test/test-execute/exec-dynamicuser-statedir-migrate-step1.service b/test/test-execute/exec-dynamicuser-statedir-migrate-step1.service index 72e6d7686f..5efc5483b8 100644 --- a/test/test-execute/exec-dynamicuser-statedir-migrate-step1.service +++ b/test/test-execute/exec-dynamicuser-statedir-migrate-step1.service @@ -10,6 +10,7 @@ ExecStart=test -d /var/lib/test-dynamicuser-migrate ExecStart=test -d /var/lib/test-dynamicuser-migrate2/hoge ExecStart=touch /var/lib/test-dynamicuser-migrate/yay ExecStart=touch /var/lib/test-dynamicuser-migrate2/hoge/yayyay +ExecStart=/bin/sh -x -c 'test "$$STATE_DIRECTORY" = "%S/test-dynamicuser-migrate:%S/test-dynamicuser-migrate2/hoge"' Type=oneshot DynamicUser=no diff --git a/test/test-execute/exec-dynamicuser-statedir-migrate-step2.service b/test/test-execute/exec-dynamicuser-statedir-migrate-step2.service index edb0be7ef8..c72302ffd5 100644 --- a/test/test-execute/exec-dynamicuser-statedir-migrate-step2.service +++ b/test/test-execute/exec-dynamicuser-statedir-migrate-step2.service @@ -18,6 +18,7 @@ ExecStart=touch /var/lib/test-dynamicuser-migrate/yay ExecStart=touch /var/lib/test-dynamicuser-migrate2/hoge/yayyay ExecStart=touch /var/lib/private/test-dynamicuser-migrate/yay ExecStart=touch /var/lib/private/test-dynamicuser-migrate2/hoge/yayyay +ExecStart=/bin/sh -x -c 'test "$$STATE_DIRECTORY" = "%S/test-dynamicuser-migrate:%S/test-dynamicuser-migrate2/hoge"' Type=oneshot DynamicUser=yes diff --git a/test/test-execute/exec-dynamicuser-statedir.service b/test/test-execute/exec-dynamicuser-statedir.service index f459f3c1eb..2fb7b8660b 100644 --- a/test/test-execute/exec-dynamicuser-statedir.service +++ b/test/test-execute/exec-dynamicuser-statedir.service @@ -10,6 +10,7 @@ ExecStart=test -f /var/lib/waldo/yay ExecStart=test -f /var/lib/quux/pief/yayyay ExecStart=test -f /var/lib/private/waldo/yay ExecStart=test -f /var/lib/private/quux/pief/yayyay +ExecStart=/bin/sh -x -c 'test "$$STATE_DIRECTORY" = "%S/waldo:%S/quux/pief"' # Make sure that /var/lib/private/waldo is really the only writable directory besides the obvious candidates ExecStart=sh -x -c 'test $$(find / \( -path /var/tmp -o -path /tmp -o -path /proc -o -path /dev/mqueue -o -path /dev/shm -o -path /sys/fs/bpf \) -prune -o -type d -writable -print 2>/dev/null | sort -u | tr -d '\\\\n') = /var/lib/private/quux/pief/var/lib/private/waldo' diff --git a/test/test-execute/exec-runtimedirectory-mode.service b/test/test-execute/exec-runtimedirectory-mode.service index 480f904155..85ae5161c4 100644 --- a/test/test-execute/exec-runtimedirectory-mode.service +++ b/test/test-execute/exec-runtimedirectory-mode.service @@ -3,6 +3,7 @@ Description=Test for RuntimeDirectoryMode [Service] ExecStart=/bin/sh -x -c 'mode=$$(stat -c %%a %t/test-exec_runtimedirectory-mode); test "$$mode" = "750"' +ExecStart=/bin/sh -x -c 'test "$$RUNTIME_DIRECTORY" = "%t/test-exec_runtimedirectory-mode"' Type=oneshot RuntimeDirectory=test-exec_runtimedirectory-mode RuntimeDirectoryMode=0750 diff --git a/test/test-execute/exec-runtimedirectory.service b/test/test-execute/exec-runtimedirectory.service index 6a4383110f..a33044d23c 100644 --- a/test/test-execute/exec-runtimedirectory.service +++ b/test/test-execute/exec-runtimedirectory.service @@ -4,6 +4,7 @@ Description=Test for RuntimeDirectory [Service] ExecStart=/bin/sh -x -c 'test -d %t/test-exec_runtimedirectory' ExecStart=/bin/sh -x -c 'test -d %t/test-exec_runtimedirectory2/hogehoge' +ExecStart=/bin/sh -x -c 'test "$$RUNTIME_DIRECTORY" = "%t/test-exec_runtimedirectory:%t/test-exec_runtimedirectory2/hogehoge"' Type=oneshot RuntimeDirectory=test-exec_runtimedirectory RuntimeDirectory=./test-exec_runtimedirectory2///./hogehoge/. From d491e65e74a92898d6e7f95032b5b037c6e3cb60 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Tue, 11 Sep 2018 14:24:47 +0900 Subject: [PATCH 8/8] man: document RUNTIME_DIRECTORY= or friends --- man/systemd.exec.xml | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/man/systemd.exec.xml b/man/systemd.exec.xml index 4cee4a508a..bc1c36fdfb 100644 --- a/man/systemd.exec.xml +++ b/man/systemd.exec.xml @@ -814,15 +814,18 @@ CapabilityBoundingSet=~CAP_B CAP_C These options take a whitespace-separated list of directory names. The specified directory names must be relative, and may not include ... If set, one or more directories by the specified names will be created (including their parents) below the locations - defined in the following table, when the unit is started. + defined in the following table, when the unit is started. Also, the corresponding environment variable + is defined with the full path of directories. If multiple directories are set, then int the environment variable + the paths are concatenated with colon (:). - Automatic directory creation - + Automatic directory creation and environment variables + Locations for system for users + Environment variable @@ -830,26 +833,31 @@ CapabilityBoundingSet=~CAP_B CAP_CRuntimeDirectory=/run$XDG_RUNTIME_DIR + $RUNTIME_DIRECTORY StateDirectory= /var/lib $XDG_CONFIG_HOME + $STATE_DIRECTORY CacheDirectory= /var/cache $XDG_CACHE_HOME + $CACHE_DIRECTORY LogsDirectory= /var/log $XDG_CONFIG_HOME/log + $LOGS_DIRECTORY ConfigurationDirectory= /etc $XDG_CONFIG_HOME + $CONFIGURATION_DIRECTORY @@ -899,7 +907,13 @@ CapabilityBoundingSet=~CAP_B CAP_C/run/foo/bar, and /run/baz. The directories /run/foo/bar and /run/baz except /run/foo are owned by the user and group specified in User= and Group=, and removed - when the service is stopped. + when the service is stopped. + + Example: if a system service unit has the following, + RuntimeDirectory=foo/bar +StateDirectory=aaa/bbb ccc + then the environment variable RUNTIME_DIRECTORY is set with /run/foo/bar, and + STATE_DIRECTORY is set with /var/lib/aaa/bbb:/var/lib/ccc.