Bug 737047

Summary:	FIXED_IN_GIT: systemd fails to start the rc-local.service when /etc/rc.d/rc.local is a symlink
Product:	[Fedora] Fedora	Reporter:	Marek Goldmann <mgoldman>
Component:	systemd	Assignee:	Lennart Poettering <lpoetter>
Status:	CLOSED ERRATA	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	16	CC:	harald, johannbg, kay, lpoetter, metherid, mschmidt, notting, plautrba, the.ridikulus.rat
Target Milestone:	---
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	systemd-36-3.fc16	Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2011-09-30 18:53:14 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

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.