install: allow adding plain templates to .wants/ or .requires/

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.

diff --git a/src/shared/install.c b/src/shared/install.c
index 48e632f4dfe43339a7506c58766fccd778cf6299..b6dc0c6e60b5fb69d3d853f74eb8914524ce9c9f 100644 (file)
--- 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;
                 }