[systemd-devel] [PATCHv2] install: Assume *.wants symlinks have the same name as their target for scalability.

[Zbigniew Jędrzejewski-Szmek]

Hi,
I'd like to reopen this thread.

I have been looking at our bugs [1-10, there are others too] related to
enabling/disabling of units and following of symlinks. Currently this
is an unintuitive mess and some changes will have to be made.

I think that the general rules should be:
(/lib is short for /usr/lib, /usr/local/lib, etc.,
 .wants is short for .wants or .requires)

1. when given a unit, it must be possible to read information about
   this unit without reading all other units

2. all symlinks in /run and /etc are considered configuration and
   will be removed by systemctl disable

3. symlinks within the configuration directories must have

   (a) the same symlink and target name, or

   (b) the symlink can be a instance and the target a template with
       the same stem and type

   (c) in /lib, the name can be different, but the type must match, and this
       provides an alias

4. systemd must follow the same logic as systemctl and must enforce
   the rules.

This means that:
1. when linking a unit with 'systemctl enable /path/to/file',
   the unit must be linked both into /etc/systemd/system/ and
   /etc/systemd/systemd/whatever.wants/. This is what systemctl enable
   currently does, but we allowed only the second part done
   manually. systemd would be changed to ignore all files in .wants/,
   and it would be changed to ignore symlinks in .wants if the unit
   cannot be loaded by name otherwise.

2. symlinking to units to provide an alias is allowed only in /lib.
   When systemctl disable is executed, all symlinks in /etc and /run will
   be nuked, but /lib will be left alone. I think this is current behaviour.

3. .wants directories and .d directories will only be honored for
   the main unit name. The problem that we have currently is that
   if the alias unit is not pulled in by anything, this extra configuration
   is ignored. But even 'systemctl status' loads the unit, and loads
   this extra configuration. systemd would be changed to warn about this
   and ignore those extra dirs.

4. support for aliases will be provided on a best-effort basis: If
   the alias is done by symlinking in /lib, it is always known.
   If the alias is provided by Alias= configuration directive, it
   is only known if the unit was already loaded.

There's a bug that 'systemctl disable' removes too much stuff for
instance units. I can't find the report atm, but this is a bug that
should be fixed too.

Zbyszek

[1] https://bugs.freedesktop.org/show_bug.cgi?id=72611
    systemctl is-enabled scales poorly with many units (this one)
[2] https://bugs.freedesktop.org/show_bug.cgi?id=63621
    systemctl enable doesn't work for Alias names
[3] https://bugs.freedesktop.org/show_bug.cgi?id=72154
    "systemctl enable named.service" yields error messages used for legacy sysinit scripts
[4] https://bugzilla.redhat.com/show_bug.cgi?id=1014311
    RFE: allow systemctl enable work on symlinked units
[5] https://bugzilla.redhat.com/show_bug.cgi?id=864298
    systemctl is-enabled reports 'static' incorrectly (it appears)
[6] https://bugzilla.redhat.com/show_bug.cgi?id=955379
    systemctl enable fails for user-defined service files in /etc/systemd/system
[7] https://bugs.freedesktop.org/show_bug.cgi?id=65255
    some units can be started when the file is just copied into .wants/
[8] https://bugs.freedesktop.org/show_bug.cgi?id=54560
    systemd does not follow symlinks that point outside of
    /etc/systemd/system or /usr/lib/systemd/system
[9] https://bugzilla.redhat.com/show_bug.cgi?id=1002806
    runlevel command returns "unknown"
[10] https://bugzilla.redhat.com/show_bug.cgi?id=815835
    /sbin/runlevel reports "unknown" where it shouldn't



On Fri, Dec 13, 2013 at 01:03:30PM -0800, david at davidstrauss.net wrote:
> From: David Strauss <david at davidstrauss.net>
> 
> ---
>  src/shared/install.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/shared/install.c b/src/shared/install.c
> index 17e8a75..512e3df 100644
> --- a/src/shared/install.c
> +++ b/src/shared/install.c
> @@ -419,10 +419,15 @@ static int find_symlinks_fd(
>                                  r = q;
>  
>                  } else if (de->d_type == DT_LNK) {
> -                        _cleanup_free_ char *p = NULL, *dest = NULL;
> +                        _cleanup_free_ char *p = NULL, *dest = NULL, *no_inst = NULL;
>                          bool found_path, found_dest, b = false;
>                          int q;
>  
> +                        /* Skip symlinks with a different name the target unit */
> +                        no_inst = unit_name_replace_instance(basename(de->d_name), "");
> +                        if (!streq(no_inst, name))
> +                                continue;
> +
>                          /* Acquire symlink name */
>                          p = path_make_absolute(de->d_name, path);
>                          if (!p)
> -- 
> 1.8.3.1
> 
> _______________________________________________
> systemd-devel mailing list
> systemd-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/systemd-devel
>