[kmscon-devel] [PATCH] systemd: take over for autovt@ when enabled

[David Herrmann]

Hi Tom

On Mon, Jul 22, 2013 at 2:49 PM, David Herrmann <dh.herrmann at gmail.com> wrote:
> Hi
>
> On Mon, Jul 22, 2013 at 2:44 PM, Tom Gundersen <teg at jklm.no> wrote:
>> On Mon, Jul 22, 2013 at 2:35 PM, David Herrmann <dh.herrmann at gmail.com> wrote:
>>> The thing is, "systemctl disable"
>>> doesn't honor "Alias" lines. So if I add this alias and a user does
>>> "systemctl enable kmscon at tty1.service", a following "systemctl disable
>>> kmscon@<whatever>" doesn't remove this alias again.
>>
>> Hm, I'm not able to reproduce this problem:
>>
>> # systemctl enable kmsconvt at tty1.service
>> ln -s '/etc/systemd/system/kmsconvt at .service'
>> '/etc/systemd/system/autovt at .service'
>> ln -s '/etc/systemd/system/kmsconvt at .service'
>> '/etc/systemd/system/getty.target.wants/kmsconvt at tty1.service'
>> # systemctl disable kmsconvt at tty1.service
>> rm '/etc/systemd/system/getty.target.wants/kmsconvt at tty1.service'
>> rm '/etc/systemd/system/autovt at .service'
>>
>> Could this have been fixed recently, or are you seeing something different?
>
> You need to modify your "Alias" line (notice the %i):
>   Alias=autovt@%i.service
> Otherwise a simple "systemctl enable kmsconvt at tty4.service" would
> enable kmscon on _all_ TTYs.
>
> With this in place, the "systemctl disable" no longer works correctly.

I did some deeper analysis and the problematic line is in src/shared/install.c:
remove_marked_symlinks_fd() does for instance-names:
  found = found && strv_contains(files, path_get_file_name(p));

If you look at the code, "files" is the list of names given on the
command line. So obviously, this never removes enabled instances if
they weren't given the exact same way on the command line. It's a nice
way to avoid iterating the list of to-be-removed files but limits the
effects of this command in a bad way, imho.

I am not sure whether removing this line has side-effects for other
users of this helper, so I will postpone this for Brno.

Cheers
David