From 3e1db806b0c18fd6138886ce67fac2655f09caef Mon Sep 17 00:00:00 2001 From: Anita Zhang Date: Mon, 4 Nov 2019 18:29:55 -0800 Subject: [PATCH] core: change top-level drop-in from -.service.d to service.d Discussed in #13743, the -.service semantic conflicts with the existing root mount and slice names, making this feature not uniformly extensible to all types. Change the name to be .d instead. Updating to this format also extends the top-level dropin to unit types. --- NEWS | 7 +++---- man/systemd.service.xml | 13 ------------- man/systemd.special.xml | 9 --------- man/systemd.unit.xml | 11 ++++++++--- src/basic/special.h | 4 ---- src/basic/unit-name.c | 30 ------------------------------ src/basic/unit-name.h | 2 -- src/core/service.c | 5 ----- src/shared/dropin.c | 22 ++++++++++++---------- src/test/test-unit-name.c | 19 ------------------- test/TEST-15-DROPIN/test-dropin.sh | 8 ++++---- 11 files changed, 27 insertions(+), 103 deletions(-) diff --git a/NEWS b/NEWS index 56d05ac7ef..c9250c9eeb 100644 --- a/NEWS +++ b/NEWS @@ -33,10 +33,9 @@ CHANGES WITH 244 in spe: during early boot, so in practice this change has very little effect.) - * The special -.service.d dropin directory may be used to add - configuration that affects all services. The "-.service" service name - is now disallowed (though hopefully no one would use such a service - name. -.mount gives us enough grief.) + * Unit files now support top level dropin directories of the form + .d/ (e.g. service.d/) that may be used to add configuration + that affects all corresponding unit files. * The RuntimeMaxSec= setting is now supported by scopes, not just .service units. This is particularly useful for PAM sessions which diff --git a/man/systemd.service.xml b/man/systemd.service.xml index ce7b847420..f57e37ca5b 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -63,19 +63,6 @@ The systemd-run1 command allows creating .service and .scope units dynamically and transiently from the command line. - - In addition to the various drop-in behaviors described in - systemd.unit5, - services also support a top-level drop-in with -.service.d/ that allows - altering or adding to the settings of all services on the system. - The formatting and precedence of applying drop-in configurations follow what is defined in - systemd.unit5. - However, configurations in -.service.d/ have the lowest precedence compared to settings - in the service specific override directories. For example, for foo-bar-baz.service, - drop-ins in foo-bar-baz.service.d/ override the ones in - foo-bar-.service.d/, which override the ones foo-.service.d/, - which override the ones in -.service.d/. - diff --git a/man/systemd.special.xml b/man/systemd.special.xml index afd14b977c..248fb924db 100644 --- a/man/systemd.special.xml +++ b/man/systemd.special.xml @@ -119,15 +119,6 @@ - - -.service - - This is a reserved unit name used to support top-level drop-ins for services. See - systemd.service5 - for details. - - - basic.target diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 5acb5951cd..616106972a 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -192,9 +192,14 @@ over unit files wherever located. Multiple drop-in files with different names are applied in lexicographic order, regardless of which of the directories they reside in. - Service units also support a top-level drop-in directory for modifying the settings of all service units. See - systemd.service5 - for details. + Units also support a top-level drop-in with type.d/, + where type may be e.g. service or socket, + that allows altering or adding to the settings of all corresponding unit files on the system. + The formatting and precedence of applying drop-in configurations follow what is defined above. + Configurations in type.d/ have the lowest precedence + compared to settings in the name specific override directories. So the contents of + foo-.service.d/10-override.conf would override + service.d/10-override.conf. diff --git a/src/basic/special.h b/src/basic/special.h index 6475501078..add1c1d507 100644 --- a/src/basic/special.h +++ b/src/basic/special.h @@ -105,7 +105,3 @@ /* The root directory. */ #define SPECIAL_ROOT_MOUNT "-.mount" - -/* Used to apply settings to all services through drop-ins. - * Should not exist as an actual service. */ -#define SPECIAL_ROOT_SERVICE "-.service" diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index af8dca53cd..680e8eb4ab 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -669,36 +669,6 @@ good: return 0; } -bool service_unit_name_is_valid(const char *name) { - _cleanup_free_ char *prefix = NULL, *s = NULL; - const char *e, *service_name = name; - - if (!unit_name_is_valid(name, UNIT_NAME_ANY)) - return false; - - e = endswith(name, ".service"); - if (!e) - return false; - - /* If it's a template or instance, get the prefix as a service name. */ - if (unit_name_is_valid(name, UNIT_NAME_INSTANCE|UNIT_NAME_TEMPLATE)) { - if (unit_name_to_prefix(name, &prefix) < 0) - return false; - - s = strjoin(prefix, ".service"); - if (!s) - return false; - - service_name = s; - } - - /* Reject reserved service name(s). */ - if (streq(service_name, SPECIAL_ROOT_SERVICE)) - return false; - - return true; -} - int slice_build_parent_slice(const char *slice, char **ret) { char *s, *dash; int r; diff --git a/src/basic/unit-name.h b/src/basic/unit-name.h index 4a6f64cc1c..15ce4e2495 100644 --- a/src/basic/unit-name.h +++ b/src/basic/unit-name.h @@ -58,8 +58,6 @@ static inline int unit_name_mangle(const char *name, UnitNameMangle flags, char return unit_name_mangle_with_suffix(name, NULL, flags, ".service", ret); } -bool service_unit_name_is_valid(const char *name); - int slice_build_parent_slice(const char *slice, char **ret); int slice_build_subslice(const char *slice, const char *name, char **subslice); bool slice_name_is_valid(const char *name); diff --git a/src/core/service.c b/src/core/service.c index ebbd99a3cc..4b17889eb4 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -549,11 +549,6 @@ static int service_verify(Service *s) { assert(s); assert(UNIT(s)->load_state == UNIT_LOADED); - if (!service_unit_name_is_valid(UNIT(s)->id)) { - log_unit_error(UNIT(s), "Service name is invalid or reserved. Refusing."); - return -EINVAL; - } - if (!s->exec_command[SERVICE_EXEC_START] && !s->exec_command[SERVICE_EXEC_STOP] && UNIT(s)->success_action == EMERGENCY_ACTION_NONE) { /* FailureAction= only makes sense if one of the start or stop commands is specified. diff --git a/src/shared/dropin.c b/src/shared/dropin.c index 7328b7adde..ca97285c97 100644 --- a/src/shared/dropin.c +++ b/src/shared/dropin.c @@ -19,7 +19,6 @@ #include "mkdir.h" #include "path-util.h" #include "set.h" -#include "special.h" #include "string-util.h" #include "strv.h" #include "unit-name.h" @@ -164,6 +163,10 @@ static int unit_file_find_dirs( return r; } + /* Return early for top level drop-ins. */ + if (unit_type_from_string(name) >= 0) + return 0; + /* Let's see if there's a "-" prefix for this unit name. If so, let's invoke ourselves for it. This will then * recursively do the same for all our prefixes. i.e. this means given "foo-bar-waldo.service" we'll also * search "foo-bar-.service" and "foo-.service". @@ -244,16 +247,15 @@ int unit_file_find_dropin_paths( name); } - /* Special drop in for -.service. Add this first as it's the most generic + /* Special top level drop in for ".". Add this first as it's the most generic * and should be able to be overridden by more specific drop-ins. */ - if (type == UNIT_SERVICE) - STRV_FOREACH(p, lookup_path) - (void) unit_file_find_dirs(original_root, - unit_path_cache, - *p, - SPECIAL_ROOT_SERVICE, - dir_suffix, - &dirs); + STRV_FOREACH(p, lookup_path) + (void) unit_file_find_dirs(original_root, + unit_path_cache, + *p, + unit_type_to_string(type), + dir_suffix, + &dirs); SET_FOREACH(name, names, i) STRV_FOREACH(p, lookup_path) diff --git a/src/test/test-unit-name.c b/src/test/test-unit-name.c index c65f6ffe08..c9bbce0d2e 100644 --- a/src/test/test-unit-name.c +++ b/src/test/test-unit-name.c @@ -353,24 +353,6 @@ static void test_unit_name_build(void) { free(t); } -static void test_service_unit_name_is_valid(void) { - assert_se(service_unit_name_is_valid("foo.service")); - assert_se(service_unit_name_is_valid("foo@bar.service")); - assert_se(service_unit_name_is_valid("foo@bar@bar.service")); - assert_se(service_unit_name_is_valid("--.service")); - assert_se(service_unit_name_is_valid(".-.service")); - assert_se(service_unit_name_is_valid("-foo-bar.service")); - assert_se(service_unit_name_is_valid("-foo-bar-.service")); - assert_se(service_unit_name_is_valid("foo-bar-.service")); - - assert_se(!service_unit_name_is_valid("-.service")); - assert_se(!service_unit_name_is_valid("")); - assert_se(!service_unit_name_is_valid("foo.slice")); - assert_se(!service_unit_name_is_valid("@.service")); - assert_se(!service_unit_name_is_valid("@bar.service")); - assert_se(!service_unit_name_is_valid("-@.service")); -} - static void test_slice_name_is_valid(void) { assert_se( slice_name_is_valid(SPECIAL_ROOT_SLICE)); assert_se( slice_name_is_valid("foo.slice")); @@ -856,7 +838,6 @@ int main(int argc, char* argv[]) { test_unit_prefix_is_valid(); test_unit_name_change_suffix(); test_unit_name_build(); - test_service_unit_name_is_valid(); test_slice_name_is_valid(); test_build_subslice(); test_build_parent_slice(); diff --git a/test/TEST-15-DROPIN/test-dropin.sh b/test/TEST-15-DROPIN/test-dropin.sh index 75c36df2b8..fe424c1587 100755 --- a/test/TEST-15-DROPIN/test-dropin.sh +++ b/test/TEST-15-DROPIN/test-dropin.sh @@ -101,18 +101,18 @@ test_basic_dropins () { check_ok b Wants c.service systemctl stop a c - echo "*** test -.service.d/ top level drop-in" + echo "*** test service.d/ top level drop-in" create_services a b check_ko a ExecCondition "/bin/echo a" check_ko b ExecCondition "/bin/echo b" - mkdir -p /usr/lib/systemd/system/-.service.d - cat >/usr/lib/systemd/system/-.service.d/override.conf </usr/lib/systemd/system/service.d/override.conf <