Bug 737047 - FIXED_IN_GIT: systemd fails to start the rc-local.service when /etc/rc.d/rc.local is a symlink
Summary: FIXED_IN_GIT: systemd fails to start the rc-local.service when /etc/rc.d/rc.l...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: systemd
Version: 16
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Lennart Poettering
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-09-09 13:10 UTC by Marek Goldmann
Modified: 2012-05-30 21:05 UTC (History)
9 users (show)

Fixed In Version: systemd-36-3.fc16
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-09-30 18:53:14 UTC


Attachments (Terms of Use)

Description Marek Goldmann 2011-09-09 13:10:45 UTC
Description of problem:

Currently I have /etc/rc.d/rc.local symlinked to /etc/rc.local (yes, other way around). Systemd fails to start the service, because it says that preconditions are not meet, meaning that /etc/rc.d/rc.local is not executable. In /lib/systemd/system/rc-local.service we have:

ConditionFileIsExecutable=/etc/rc.d/rc.local

If I remove this condition - rc.local is executed successfully. I think symlink should also work, as long as the file linked to is executable.

Version-Release number of selected component (if applicable):

systemd-35-1.fc16

How reproducible:

Always.

Steps to Reproduce:
Description above.
  
Actual results:

Service start failed.

Expected results:

Successful service start.

Comment 1 Michal Schmidt 2011-09-09 14:32:28 UTC
The test for executability of the file was added to rc-local.service because on most systems rc.local contains nothing and spawning the shell for it is a waste of time. So the idea is that initscripts should ship the file without the x bit and include a comment like: "Please 'chmod +x' this file if you want it to run on boot".

ConditionFileIsExecutable behaves as documented in systemd.unit(5):

  ConditionFileIsExecutable= is similar to ConditionPathExists= but verifies
  whether a certain path exists, is a regular file and marked executable.

Theoretically this could be changed to follow symlinks (s/lstat/stat/ in the implementation of CONDITION_FILE_IS_EXECUTABLE). An argument for the change can be that ConditionFileIsExecutable is used as a replacement for "test -x" (which does follow symlinks). But it would be a change in the documented behaviour.

Comment 2 Bill Nottingham 2011-09-09 17:25:28 UTC
initscripts was changed to just not ship the file, period. See bug 734628.

FYI - I would agree that it should be changed to follow the symlink.

Comment 3 Marek Goldmann 2011-09-10 08:27:06 UTC
(In reply to comment #1)
> The test for executability of the file was added to rc-local.service because on
> most systems rc.local contains nothing and spawning the shell for it is a waste
> of time. So the idea is that initscripts should ship the file without the x bit
> and include a comment like: "Please 'chmod +x' this file if you want it to run
> on boot".
> 
> ConditionFileIsExecutable behaves as documented in systemd.unit(5):
> 
>   ConditionFileIsExecutable= is similar to ConditionPathExists= but verifies
>   whether a certain path exists, is a regular file and marked executable.
> 
> Theoretically this could be changed to follow symlinks (s/lstat/stat/ in the
> implementation of CONDITION_FILE_IS_EXECUTABLE). An argument for the change can
> be that ConditionFileIsExecutable is used as a replacement for "test -x" (which
> does follow symlinks). But it would be a change in the documented behaviour.

Michal,

In this case the condition name is misleading a bit because there are done additional checks other than checking for exec flag. IMHO this should be split into two conditions in that case: ConditionFileIsExecutable and ConditionFileIsRegular.

Comment 4 Marek Goldmann 2011-09-10 08:28:01 UTC
(In reply to comment #2)
> initscripts was changed to just not ship the file, period. See bug 734628.

I am totally cool with that :) BTW, the bug # your provided is wrong.

Comment 5 Bill Nottingham 2011-09-12 20:59:42 UTC
Sorry, bug 734268.

Comment 6 Lennart Poettering 2011-09-19 23:28:32 UTC
Fixed in git.

Comment 7 Michal Schmidt 2011-09-20 11:21:14 UTC
Should ConditionPathIsDirectory follow symlinks too?

Comment 8 Kay Sievers 2011-09-20 12:46:54 UTC
I don't think so. And if, we need to be careful here. We use it to find out if /var/run needs to be bind mounted or is already a symlink.

Comment 9 Michal Schmidt 2011-09-21 00:06:44 UTC
The path tests now behave like test(1) with respect to following symlinks:
http://cgit.freedesktop.org/systemd/commit/?id=8571962ca31a468959eedce26fda278587327ba5

ConditionPathIsSymbolicLink has been added to help var-run.mount and var-lock.mount:
http://cgit.freedesktop.org/systemd/commit/?id=0d60602c3b4d4b65da672d75c6146f2ea4b27f88

Comment 10 Fedora Update System 2011-09-23 17:06:11 UTC
systemd-36-2.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/systemd-36-2.fc16

Comment 11 Fedora Update System 2011-09-24 20:48:52 UTC
Package systemd-36-2.fc16:
* should fix your issue,
* was pushed to the Fedora 16 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing systemd-36-2.fc16'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/systemd-36-2.fc16
then log in and leave karma (feedback).

Comment 12 Fedora Update System 2011-09-30 18:52:30 UTC
systemd-36-3.fc16 has been pushed to the Fedora 16 stable repository.  If problems still persist, please make note of it in this bug report.


Note You need to log in before you can comment on or make changes to this bug.