[Pkg-systemd-maintainers] Bug#719695: Bug#719695: Bug#719695: Bug#719695: Prefer symlinks over Alias= for non-matching service names

[Lennart Poettering]

On Sun, 02.03.14 02:08, Michael Stapelberg (stapelberg at debian.org) wrote:

Could you please send the patch to the systemd ML for review?

> [+cc lennart]
> 
> Hi Michael,
> 
> Michael Biebl <biebl at debian.org> writes:
> >>> Maybe we could convince Lennart to allow that.
> >> AFAIR, I read that you made some progress with Lennart on this. Could
> >> you update this bug accordingly please? Thanks.
> >
> > [19:50:51] <poettering> mbiebl: so, i'd be fine with allowing
> > enable/disable to follow symlinks in /usr/lib before starting to do its
> > thing
> > [19:50:59] <poettering> mbiebl: however, i am strictly against following
> > them through /etc
> > [19:51:17] <poettering> mbiebl: because symlinks /etc is the stuff the
> > tool manages after all
> > [19:51:22] <mbiebl> nod
> > [19:51:25] <poettering> mbiebl: and it shouldn't get confused by its own
> > doings
> > [19:51:30] <mbiebl> it would be for static symlinks only
> > [19:52:01] <poettering> so, the current logic simply skips all symlinks,
> > and i'd be happy to change that to skip only the symlinks in /etc, /run,
> > and so on
> Alright. After an eternity, I decided to have a look at this issue
> again. When creating /lib/systemd/system/rsync-alias.service →
> rsync.service, with the attached patch I can enable/disable the service:
> 
> midna $ sudo systemctl status rsync-alias.service      
> rsync.service - fast remote file copy program daemon                           
>    Loaded: loaded (/lib/systemd/system/rsync.service; enabled)
>    Active: inactive (dead)
>           start condition failed at Sun 2014-03-02 01:39:20 CET; 23min ago
> 
> Mar 02 01:39:20 midna systemd[1]: Started fast remote file copy program daemon.
> 
> midna $ sudo systemctl disable rsync-alias.service
> rm '/etc/systemd/system/multi-user.target.wants/rsync.service'
> 
> midna $ sudo systemctl status rsync-alias.service
> rsync.service - fast remote file copy program daemon
>    Loaded: loaded (/lib/systemd/system/rsync.service; disabled)
>    Active: inactive (dead)
> 
> Mar 02 01:39:20 midna systemd[1]: Started fast remote file copy program daemon.
> 
> midna $ sudo systemctl enable rsync-alias.service
> ln -s '/lib/systemd/system/rsync.service' '/etc/systemd/system/multi-user.target.wants/rsync.service'
> 
> midna $ sudo systemctl status rsync-alias.service
> rsync.service - fast remote file copy program daemon
>    Loaded: loaded (/lib/systemd/system/rsync.service; enabled)
>    Active: inactive (dead)
> 
> Mar 02 01:39:20 midna systemd[1]: Started fast remote file copy program daemon.
> 
> However, mask and unmask are still using rsync-alias.service, not
> rsync.service.
> 
> Lennart, does the patch look good to you so far? Or are we on the wrong
> path entirely?
> 
> Which other systemctl verbs will need special casing apart from
> enable|disable|mask|unmask?
> 
> Thanks.
> 
> PS: If you wanted to take this patch and hack this feature in, I
> definitely wouldn’t mind :-).
> 

> diff --git i/src/shared/install.c w/src/shared/install.c
> index f57b94d..8f9596b 100644
> --- i/src/shared/install.c
> +++ w/src/shared/install.c
> @@ -1023,6 +1023,8 @@ static int unit_file_load(
>                  (int) strv_length(info->required_by);
>  }
>  
> +#define FOLLOW_MAX 8
> +
>  static int unit_file_search(
>                  InstallContext *c,
>                  InstallInfo *info,
> @@ -1053,11 +1055,44 @@ static int unit_file_search(
>                  if (!path)
>                          return -ENOMEM;
>  
> +                int cnt = 0;
> +                for (;;) {
> +                        if (cnt++ >= FOLLOW_MAX)
> +                                return -ELOOP;
> +
> +                        r = unit_file_load(c, info, path, allow_symlink);
> +
> +                        /* symlinks are always allowed for units in {/usr,}/lib/systemd so that
> +                         * one can alias units without using Alias= (the downside of Alias= is
> +                         * that the alias only exists when the unit is enabled). */
> +                        if (r >= 0)
> +                                break;
> +
> +                        if (r != -ELOOP)
> +                                break;
> +
> +                        if (allow_symlink)
> +                                break;
> +
> +                        if (!path_startswith(path, "/lib/systemd") &&
> +                            !path_startswith(path, "/usr/lib/systemd"))
> +                                break;
> +
> +                        char *target;
> +                        r = readlink_and_make_absolute(path, &target);
> +                        if (r < 0)
> +                                return r;
> +                        free(path);
> +                        path = target;
> +                }
> +
>                  r = unit_file_load(c, info, path, allow_symlink);
>  
> -                if (r >= 0)
> +                if (r >= 0) {
>                          info->path = path;
> -                else {
> +                        free(info->name);
> +                        info->name = strdup(path_get_file_name(path));
> +                } else {
>                          if (r == -ENOENT && unit_name_is_instance(info->name)) {
>                                  /* Unit file doesn't exist, however instance enablement was requested.
>                                   * We will check if it is possible to load template unit file. */



Lennart

-- 
Lennart Poettering, Red Hat