From 8a993b61d1cd46a9c48d36fb67818883d80d9bc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 30 Apr 2016 16:21:41 -0400 Subject: [PATCH 1/5] Move no_alias information to shared/ This way it can be used in install.c in subsequent commit. --- src/core/automount.c | 1 - src/core/busname.c | 1 - src/core/load-fragment.c | 2 +- src/core/mount.c | 1 - src/core/scope.c | 1 - src/core/slice.c | 1 - src/core/swap.c | 1 - src/core/unit.c | 4 ++-- src/core/unit.h | 3 --- src/shared/install.c | 10 ++++++++++ src/shared/install.h | 2 ++ 11 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/core/automount.c b/src/core/automount.c index 7c55d7bc49d..7374d50ae8a 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -1050,7 +1050,6 @@ const UnitVTable automount_vtable = { "Automount\0" "Install\0", - .no_alias = true, .no_instances = true, .init = automount_init, diff --git a/src/core/busname.c b/src/core/busname.c index f4f433340c7..4d43bd21e6d 100644 --- a/src/core/busname.c +++ b/src/core/busname.c @@ -1028,7 +1028,6 @@ const UnitVTable busname_vtable = { "Install\0", .private_section = "BusName", - .no_alias = true, .no_instances = true, .init = busname_init, diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 31b995aa6a6..1a8c03904cd 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3612,7 +3612,7 @@ static int load_from_path(Unit *u, const char *path) { /* Hmm, no suitable file found? */ return 0; - if (UNIT_VTABLE(u)->no_alias && set_size(symlink_names) > 1) { + if (!unit_type_may_alias(u->type) && set_size(symlink_names) > 1) { log_unit_warning(u, "Unit type of %s does not support alias names, refusing loading via symlink.", u->id); return -ELOOP; } diff --git a/src/core/mount.c b/src/core/mount.c index cc07873b24b..adc74c3bea0 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1839,7 +1839,6 @@ const UnitVTable mount_vtable = { "Install\0", .private_section = "Mount", - .no_alias = true, .no_instances = true, .init = mount_init, diff --git a/src/core/scope.c b/src/core/scope.c index 7078d1f7e90..3915e5c88c6 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -569,7 +569,6 @@ const UnitVTable scope_vtable = { "Install\0", .private_section = "Scope", - .no_alias = true, .no_instances = true, .can_transient = true, diff --git a/src/core/slice.c b/src/core/slice.c index 63a77c9bca7..96c7c745984 100644 --- a/src/core/slice.c +++ b/src/core/slice.c @@ -309,7 +309,6 @@ const UnitVTable slice_vtable = { "Install\0", .private_section = "Slice", - .no_alias = true, .no_instances = true, .can_transient = true, diff --git a/src/core/swap.c b/src/core/swap.c index d8802470d26..f486a44cf56 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -1465,7 +1465,6 @@ const UnitVTable swap_vtable = { "Install\0", .private_section = "Swap", - .no_alias = true, .no_instances = true, .init = swap_init, diff --git a/src/core/unit.c b/src/core/unit.c index 4a129ffd5e9..0313ee2ad3f 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -202,7 +202,7 @@ int unit_add_name(Unit *u, const char *text) { if (u->type != _UNIT_TYPE_INVALID && !u->instance != !i) return -EINVAL; - if (unit_vtable[t]->no_alias && !set_isempty(u->names)) + if (!unit_type_may_alias(t) && !set_isempty(u->names)) return -EEXIST; if (hashmap_size(u->manager->units) >= MANAGER_MAX_NAMES) @@ -720,7 +720,7 @@ int unit_merge(Unit *u, Unit *other) { if (!u->instance != !other->instance) return -EINVAL; - if (UNIT_VTABLE(u)->no_alias) /* Merging only applies to unit names that support aliases */ + if (!unit_type_may_alias(u->type)) /* Merging only applies to unit names that support aliases */ return -EEXIST; if (other->load_state != UNIT_STUB && diff --git a/src/core/unit.h b/src/core/unit.h index 5909652976a..6ac925a1851 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -416,9 +416,6 @@ struct UnitVTable { /* The strings to print in status messages */ UnitStatusMessageFormats status_message_formats; - /* Can units of this type have multiple names? */ - bool no_alias:1; - /* Instances make no sense for this type */ bool no_instances:1; diff --git a/src/shared/install.c b/src/shared/install.c index 931d3e29078..b92afbc971f 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -68,6 +68,16 @@ typedef struct { static int unit_file_lookup_state(UnitFileScope scope, const LookupPaths *paths, const char *name, UnitFileState *ret); +bool unit_type_may_alias(UnitType type) { + return IN_SET(type, + UNIT_SERVICE, + UNIT_SOCKET, + UNIT_TARGET, + UNIT_DEVICE, + UNIT_TIMER, + UNIT_PATH); +} + static int in_search_path(const LookupPaths *p, const char *path) { _cleanup_free_ char *parent = NULL; char **i; diff --git a/src/shared/install.h b/src/shared/install.h index 4ffc5a21f29..8a8bd09c7c3 100644 --- a/src/shared/install.h +++ b/src/shared/install.h @@ -138,6 +138,8 @@ static inline bool UNIT_FILE_INSTALL_INFO_HAS_ALSO(UnitFileInstallInfo *i) { return !strv_isempty(i->also); } +bool unit_type_may_alias(UnitType type) _const_; + int unit_file_enable( UnitFileScope scope, bool runtime, From a77245890103cae2d3ad69d3e6506cea4f7f9065 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 30 Apr 2016 16:54:37 -0400 Subject: [PATCH 2/5] shared/install: ignore Alias in [Install] of units which don't allow aliases A downside is that a warning about missing [Install] is printed: $ systemctl --root=/ enable mnt-test.mount [/etc/systemd/system/mnt-test.mount:5] Aliases are not allowed for mount units, ignoring. The unit files have no installation config (WantedBy, RequiredBy, Also, Alias settings in the [Install] section, and DefaultInstance for template units). This means they are not meant to be enabled using systemctl. Possible reasons for having this kind of units are: 1) A unit may be statically enabled by being symlinked from another unit's .wants/ or .requires/ directory. 2) A unit's purpose may be to act as a helper for some other unit which has a requirement dependency on it. 3) A unit may be started when needed via activation (socket, path, timer, D-Bus, udev, scripted systemctl call, ...). 4) In case of template units, the unit is meant to be enabled with some instance name specified. That's a bit misleading, but I don't see an easy way to fix this. But the situation is similar for many other parsing errors, so maybe that's OK. --- src/shared/install.c | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/src/shared/install.c b/src/shared/install.c index b92afbc971f..1635f50fdb0 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -908,6 +908,36 @@ fail: return r; } +static int config_parse_alias( + const char *unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + const char *name; + UnitType type; + + assert(filename); + assert(lvalue); + assert(rvalue); + + name = basename(filename); + type = unit_name_to_type(name); + if (!unit_type_may_alias(type)) + return log_syntax(unit, LOG_WARNING, filename, line, 0, + "Aliases are not allowed for %s units, ignoring.", + unit_type_to_string(type)); + + return config_parse_strv(unit, filename, line, section, section_line, + lvalue, ltype, rvalue, data, userdata); +} + static int config_parse_also( const char *unit, const char *filename, @@ -993,7 +1023,7 @@ static int unit_file_load( SearchFlags flags) { const ConfigTableItem items[] = { - { "Install", "Alias", config_parse_strv, 0, &info->aliases }, + { "Install", "Alias", config_parse_alias, 0, &info->aliases }, { "Install", "WantedBy", config_parse_strv, 0, &info->wanted_by }, { "Install", "RequiredBy", config_parse_strv, 0, &info->required_by }, { "Install", "DefaultInstance", config_parse_default_instance, 0, info }, From ce99c68a3362a2905743a0673a4c016f2b8ab1a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 30 Apr 2016 18:34:13 -0400 Subject: [PATCH 3/5] Move no_instances information to shared/ This way it can be used in install.c in subsequent commit. --- src/core/automount.c | 2 -- src/core/busname.c | 2 -- src/core/device.c | 2 -- src/core/mount.c | 2 -- src/core/scope.c | 1 - src/core/slice.c | 1 - src/core/swap.c | 2 -- src/core/unit.c | 2 +- src/core/unit.h | 3 --- src/shared/install.c | 9 +++++++++ src/shared/install.h | 1 + 11 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/core/automount.c b/src/core/automount.c index 7374d50ae8a..1239a0efc69 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -1050,8 +1050,6 @@ const UnitVTable automount_vtable = { "Automount\0" "Install\0", - .no_instances = true, - .init = automount_init, .load = automount_load, .done = automount_done, diff --git a/src/core/busname.c b/src/core/busname.c index 4d43bd21e6d..e7b7b5c0125 100644 --- a/src/core/busname.c +++ b/src/core/busname.c @@ -1028,8 +1028,6 @@ const UnitVTable busname_vtable = { "Install\0", .private_section = "BusName", - .no_instances = true, - .init = busname_init, .done = busname_done, .load = busname_load, diff --git a/src/core/device.c b/src/core/device.c index d01bec53d8d..16e56efcc33 100644 --- a/src/core/device.c +++ b/src/core/device.c @@ -841,8 +841,6 @@ const UnitVTable device_vtable = { "Device\0" "Install\0", - .no_instances = true, - .init = device_init, .done = device_done, .load = unit_load_fragment_and_dropin_optional, diff --git a/src/core/mount.c b/src/core/mount.c index adc74c3bea0..c8a898e4dcd 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -1839,8 +1839,6 @@ const UnitVTable mount_vtable = { "Install\0", .private_section = "Mount", - .no_instances = true, - .init = mount_init, .load = mount_load, .done = mount_done, diff --git a/src/core/scope.c b/src/core/scope.c index 3915e5c88c6..238f63a7299 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -569,7 +569,6 @@ const UnitVTable scope_vtable = { "Install\0", .private_section = "Scope", - .no_instances = true, .can_transient = true, .init = scope_init, diff --git a/src/core/slice.c b/src/core/slice.c index 96c7c745984..c7700b8857d 100644 --- a/src/core/slice.c +++ b/src/core/slice.c @@ -309,7 +309,6 @@ const UnitVTable slice_vtable = { "Install\0", .private_section = "Slice", - .no_instances = true, .can_transient = true, .init = slice_init, diff --git a/src/core/swap.c b/src/core/swap.c index f486a44cf56..c018648d87b 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -1465,8 +1465,6 @@ const UnitVTable swap_vtable = { "Install\0", .private_section = "Swap", - .no_instances = true, - .init = swap_init, .load = swap_load, .done = swap_done, diff --git a/src/core/unit.c b/src/core/unit.c index 0313ee2ad3f..a2726f10a60 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -193,7 +193,7 @@ int unit_add_name(Unit *u, const char *text) { if (r < 0) return r; - if (i && unit_vtable[t]->no_instances) + if (i && !unit_type_may_template(t)) return -EINVAL; /* Ensure that this unit is either instanced or not instanced, diff --git a/src/core/unit.h b/src/core/unit.h index 6ac925a1851..be62e88421f 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -416,9 +416,6 @@ struct UnitVTable { /* The strings to print in status messages */ UnitStatusMessageFormats status_message_formats; - /* Instances make no sense for this type */ - bool no_instances:1; - /* True if transient units of this type are OK */ bool can_transient:1; }; diff --git a/src/shared/install.c b/src/shared/install.c index 1635f50fdb0..cc39aaf6773 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -78,6 +78,15 @@ bool unit_type_may_alias(UnitType type) { UNIT_PATH); } +bool unit_type_may_template(UnitType type) { + return IN_SET(type, + UNIT_SERVICE, + UNIT_SOCKET, + UNIT_TARGET, + UNIT_TIMER, + UNIT_PATH); +} + static int in_search_path(const LookupPaths *p, const char *path) { _cleanup_free_ char *parent = NULL; char **i; diff --git a/src/shared/install.h b/src/shared/install.h index 8a8bd09c7c3..5812447c5ba 100644 --- a/src/shared/install.h +++ b/src/shared/install.h @@ -139,6 +139,7 @@ static inline bool UNIT_FILE_INSTALL_INFO_HAS_ALSO(UnitFileInstallInfo *i) { } bool unit_type_may_alias(UnitType type) _const_; +bool unit_type_may_template(UnitType type) _const_; int unit_file_enable( UnitFileScope scope, From 6597fa61173a16d9d93141a749badad67c4b4aa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 30 Apr 2016 17:08:38 -0400 Subject: [PATCH 4/5] shared/install: warn about DefaultInstance in non-template units [/etc/systemd/system/mnt-test.mount:6] DefaultInstance only makes sense for template units, ignoring. --- src/shared/install.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/shared/install.c b/src/shared/install.c index cc39aaf6773..f89e2c63877 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -1003,6 +1003,7 @@ static int config_parse_default_instance( void *userdata) { UnitFileInstallInfo *i = data; + const char *name; char *printed; int r; @@ -1010,6 +1011,15 @@ static int config_parse_default_instance( assert(lvalue); assert(rvalue); + name = basename(filename); + if (unit_name_is_valid(name, UNIT_NAME_INSTANCE)) + /* When enabling an instance, we might be using a template unit file, + * but we should ignore DefaultInstance silently. */ + return 0; + if (!unit_name_is_valid(name, UNIT_NAME_TEMPLATE)) + return log_syntax(unit, LOG_WARNING, filename, line, 0, + "DefaultInstance only makes sense for template units, ignoring."); + r = install_full_printf(i, rvalue, &printed); if (r < 0) return r; From 133e5b362f862ac9c9b1dd7b5de0b004cbb9af54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sat, 30 Apr 2016 17:52:19 -0400 Subject: [PATCH 5/5] shared/install: refuse template files for non-templateable units $ systemctl --root=/ enable templated@bar.mount Unit type mount cannot be templated. Failed to enable: Invalid argument. --- src/shared/install.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/shared/install.c b/src/shared/install.c index f89e2c63877..f02d81504fa 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -1050,6 +1050,8 @@ static int unit_file_load( {} }; + const char *name; + UnitType type; _cleanup_fclose_ FILE *f = NULL; _cleanup_close_ int fd = -1; struct stat st; @@ -1059,6 +1061,12 @@ static int unit_file_load( assert(info); assert(path); + name = basename(path); + type = unit_name_to_type(name); + if (unit_name_is_valid(name, UNIT_NAME_TEMPLATE|UNIT_NAME_INSTANCE) && + !unit_type_may_template(type)) + return log_error_errno(EINVAL, "Unit type %s cannot be templated.", unit_type_to_string(type)); + if (!(flags & SEARCH_LOAD)) { r = lstat(path, &st); if (r < 0)