From b0ec372a9665dab6bb00066bc0fed667940f343e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 4 Jun 2021 15:26:37 +0200 Subject: [PATCH] install: allow adding plain templates to .wants/ or .requires/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #19437. As reported in the bug: > # drkonqi-coredump-processor@.service > ... > [Install] > WantedBy=systemd-coredump@.service > > The plan here is to have a systemd-coredump@ instance start the same %i for > drkonqi-coredump-processor@. Works perfectly when creating the symlink manually > ln -sv /usr/lib/systemd/system/drkonqi-coredump-processor@.service > /etc/systemd/system/systemd-coredump@.service.wants/. When DefaultInstance is set, we replace template references with template@default-inst. But in this case we want to create a symlink for the template name, so that systemd will fill in the instance from the wanting/requiring unit. This is only possible for those units that actually have an instance set, so we create the symlink only from .requires/ or .wants of an instantiated unit (then this specific instance will be used), or a template (than some instance will be inherited later). Specifically: ... [Install] WantedBy=other@.service, fixed.service DefaultInstance=inst → enable foo@.service creates other@.service.wants/foo@inst.service, and other@a.service will want foo@inst.service, and other@b.service will want foo@inst.service, and fixed.service will want foo@inst.service. Without DefaultInstance, → enable foo@.service creates other@.service.wants/foo@.service, and other@a.service would want foo@a.service, and other@b.service would want foo@b.service, but enablement fails because no dependency can be created for fixed.service: Failed to enable unit, unit fixed.service is a non-template unit. --- src/shared/install.c | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/src/shared/install.c b/src/shared/install.c index 48e632f4dfe..b6dc0c6e60b 100644 --- a/src/shared/install.c +++ b/src/shared/install.c @@ -376,6 +376,11 @@ void unit_file_dump_changes(int r, const char *verb, const UnitFileChange *chang verb, changes[i].path); logged = true; break; + case -EIDRM: + log_error_errno(changes[i].type_or_errno, "Failed to %s unit, unit %s is a non-template unit.", + verb, changes[i].path); + logged = true; + break; case -EUCLEAN: log_error_errno(changes[i].type_or_errno, "Failed to %s unit, \"%s\" is not a valid unit name.", @@ -1847,6 +1852,7 @@ static int install_info_symlink_wants( size_t *n_changes) { _cleanup_free_ char *buf = NULL; + UnitNameFlags valid_dst_type = UNIT_NAME_ANY; const char *n; char **s; int r = 0, q; @@ -1858,15 +1864,17 @@ static int install_info_symlink_wants( if (strv_isempty(list)) return 0; - if (unit_name_is_valid(i->name, UNIT_NAME_TEMPLATE)) { + if (unit_name_is_valid(i->name, UNIT_NAME_PLAIN | UNIT_NAME_INSTANCE)) + /* Not a template unit. Use the name directly. */ + n = i->name; + + else if (i->default_instance) { UnitFileInstallInfo instance = { .type = _UNIT_FILE_TYPE_INVALID, }; _cleanup_free_ char *path = NULL; - /* If this is a template, and we have no instance, don't do anything */ - if (!i->default_instance) - return 1; + /* If this is a template, and we have a default instance, use it. */ r = unit_name_replace_instance(i->name, i->default_instance, &buf); if (r < 0) @@ -1885,8 +1893,14 @@ static int install_info_symlink_wants( } n = buf; - } else + + } else { + /* We have a template, but no instance yet. When used with an instantiated unit, we will get + * the instance from that unit. Cannot be used with non-instance units. */ + + valid_dst_type = UNIT_NAME_INSTANCE | UNIT_NAME_TEMPLATE; n = i->name; + } STRV_FOREACH(s, list) { _cleanup_free_ char *path = NULL, *dst = NULL; @@ -1895,9 +1909,17 @@ static int install_info_symlink_wants( if (q < 0) return q; - if (!unit_name_is_valid(dst, UNIT_NAME_ANY)) { - unit_file_changes_add(changes, n_changes, -EUCLEAN, dst, NULL); - r = -EUCLEAN; + if (!unit_name_is_valid(dst, valid_dst_type)) { + /* Generate a proper error here: EUCLEAN if the name is generally bad, + * EIDRM if the template status doesn't match. */ + if (unit_name_is_valid(dst, UNIT_NAME_ANY)) { + unit_file_changes_add(changes, n_changes, -EIDRM, dst, n); + r = -EIDRM; + } else { + unit_file_changes_add(changes, n_changes, -EUCLEAN, dst, NULL); + r = -EUCLEAN; + } + continue; }