Make systemctl is-enabled work for templated units

Make systemctl is-enabled work for templated units

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 682 bytes --]

Hello guys,

in this mail, you will find a patch which resolves the problem, that
'systemctl is-enabled' does not work for templated units.

See also https://bugs.freedesktop.org/show_bug.cgi?id=55318 for
reference.

Without this patch, systemctl is-enabled something(a)abc.service
returned "No such file or directory", because it was checked
if /usr/lib/systemd/system/something(a)abc.service, etc. existed,
which is obviously not the case.

If systemctl is-enabled is called for templated units, this
check should be omitted and it should be searched for symlinks
in the /etc paths right away.

I would like to request that this is merged to the systemd git tree.

Best,
-Michael

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-systemctl-is-enabled-work-for-templated-units.patch --]
[-- Type: text/x-patch, Size: 2981 bytes --]

>From 633b5ecfdec9d588ad6f97a20c50089200632e24 Mon Sep 17 00:00:00 2001
From: Michael Tremer <michael.tremer@ipfire.org>
Date: Sun, 19 May 2013 15:28:49 +0200
Subject: [PATCH] Make systemctl is-enabled work for templated units.

Without this patch, systemctl is-enabled something@abc.service
returned "No such file or directory", because it was checked
if /usr/lib/systemd/system/something@abc.service, etc. existed,
which is obviously not the case.

If systemctl is-enabled is called for templated units, this
check should be omitted and it should be searched for symlinks
in the /etc paths right away.

This patch fixes the broken behaviour and resolves
  https://bugs.freedesktop.org/show_bug.cgi?id=55318.
---
 src/shared/install.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/src/shared/install.c b/src/shared/install.c
index edf4d2a..a9e2d7b 100644
--- a/src/shared/install.c
+++ b/src/shared/install.c
@@ -1609,25 +1609,31 @@ UnitFileState unit_file_get_state(
                 if (!path)
                         return -ENOMEM;
 
-                if (lstat(path, &st) < 0) {
-                        r = -errno;
-                        if (errno == ENOENT)
-                                continue;
+                /*
+                 * Search for a unit file in our default paths, to
+                 * be sure, that there are no broken symlinks.
+                 */
+                if (!unit_name_is_instance(name)) {
+                        if (lstat(path, &st) < 0) {
+                                r = -errno;
+                                if (errno == ENOENT)
+                                        continue;
 
-                        return -errno;
-                }
+                                return -errno;
+                        }
 
-                if (!S_ISREG(st.st_mode) && !S_ISLNK(st.st_mode))
-                        return -ENOENT;
+                        if (!S_ISREG(st.st_mode) && !S_ISLNK(st.st_mode))
+                                return -ENOENT;
 
-                r = null_or_empty_path(path);
-                if (r < 0 && r != -ENOENT)
-                        return r;
-                else if (r > 0) {
-                        state = path_startswith(*i, "/run") ?
-                                UNIT_FILE_MASKED_RUNTIME : UNIT_FILE_MASKED;
-                        return state;
-                }
+                        r = null_or_empty_path(path);
+                        if (r < 0 && r != -ENOENT)
+                                return r;
+                        else if (r > 0) {
+                                state = path_startswith(*i, "/run") ?
+                                        UNIT_FILE_MASKED_RUNTIME : UNIT_FILE_MASKED;
+                                return state;
+                        }
+		}
 
                 r = find_symlinks_in_scope(scope, root_dir, name, &state);
                 if (r < 0)
-- 
1.8.1

Re: [systemd-devel] Make systemctl is-enabled work for templated units

[Zbigniew Jędrzejewski-Szmek]

[-- Attachment #1: Type: text/plain, Size: 1132 bytes --]

On Sun, May 19, 2013 at 03:45:48PM +0200, Michael Tremer wrote:
> Hello guys,
> 
> in this mail, you will find a patch which resolves the problem, that
> 'systemctl is-enabled' does not work for templated units.
Applied!

> See also https://bugs.freedesktop.org/show_bug.cgi?id=55318 for
> reference.
> 
> Without this patch, systemctl is-enabled something(a)abc.service
> returned "No such file or directory", because it was checked
> if /usr/lib/systemd/system/something(a)abc.service, etc. existed,
> which is obviously not the case.
Actually, we can mask instance units, so "obviously" is not true.
And we can also "concretize" instance units by adding a normal
unit file for the instance. I tweaked your patch to just
ignore ENOENT for instance units, but perform the checks.

> If systemctl is-enabled is called for templated units, this
> check should be omitted and it should be searched for symlinks
> in the /etc paths right away.
Actually links can appear not just in /etc, so this part of the
description is misleading.

> I would like to request that this is merged to the systemd git tree.
This is implied :)

Zbyszek

Re: [systemd-devel] Make systemctl is-enabled work for templated units

[Michael Tremer]

[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]

On Tue, 2013-05-21 at 15:30 +0200, Zbigniew Jędrzejewski-Szmek wrote:
> On Sun, May 19, 2013 at 03:45:48PM +0200, Michael Tremer wrote:
> > Hello guys,
> > 
> > in this mail, you will find a patch which resolves the problem, that
> > 'systemctl is-enabled' does not work for templated units.
> Applied!

Thanks.

> > See also https://bugs.freedesktop.org/show_bug.cgi?id=55318 for
> > reference.
> > 
> > Without this patch, systemctl is-enabled something(a)abc.service
> > returned "No such file or directory", because it was checked
> > if /usr/lib/systemd/system/something(a)abc.service, etc. existed,
> > which is obviously not the case.
> Actually, we can mask instance units, so "obviously" is not true.
> And we can also "concretize" instance units by adding a normal
> unit file for the instance. I tweaked your patch to just
> ignore ENOENT for instance units, but perform the checks.

Okay.

> 
> > If systemctl is-enabled is called for templated units, this
> > check should be omitted and it should be searched for symlinks
> > in the /etc paths right away.
> Actually links can appear not just in /etc, so this part of the
> description is misleading.

I just wrote /etc to keep the message short. It should have been more
precise, indeed.

Thanks anyway for looking into that and amending the patch.

-Michael