Bug 754811 - Provide native systemd service
Summary: Provide native systemd service
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Fedora
Classification: Fedora
Component: cachefilesd
Version: 19
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: David Howells
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 751869
TreeView+ depends on / blocked
 
Reported: 2011-11-17 19:37 UTC by Jóhann B. Guðmundsson
Modified: 2014-05-26 09:44 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-05-26 09:44:22 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Native systemd service for cachefilesd (215 bytes, text/plain)
2011-11-17 19:41 UTC, Jóhann B. Guðmundsson
no flags Details

Description Jóhann B. Guðmundsson 2011-11-17 19:37:19 UTC
Description of problem:

Let's get the ball rolling on this one...

http://fedoraproject.org/wiki/Features/SysVtoSystemd
https://fedoraproject.org/wiki/Packaging:Guidelines:Systemd
https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd

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


How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:

Comment 1 Jóhann B. Guðmundsson 2011-11-17 19:41:15 UTC
Created attachment 534297 [details]
Native systemd service for cachefilesd

Note kernel modules should be autoloaded if doable if not a conf file for the relevant module be dropped into /etc/modules-load.d/ directory see man modules-load.d for details

Comment 3 Jóhann B. Guðmundsson 2011-11-30 12:25:50 UTC
Note that since you arent going to daemonize cachefiled you dont have to specifically define Type=simple because systemd defaults to it so you can skip that line.

Secondly afaik the kernel support auto-loading of the kernel module ( granted I dont know to which extent that support is ) hence you should skip the ExecStartPre line.

If you cant you really should drop a load module configuration snippet to /etc/modules-load.d/ as opposed to be loading the module in the unit

If you really really *must* for some unexplanable reason for doing this on the 21 century load the module in the unit file like so...

ExecStartPre=-/sbin/modprobe -qab <module> 

Thanks

Comment 4 David Howells 2011-11-30 17:30:59 UTC
The module cannot be autoloaded because it's accessed through a misc device file doesn't exist and can't exist until the module is loaded because the device is a "numberless" misc device - so there's no key (minor device number) to use to tell userspace what it wants.  And userspace can't invent a chardev file in advance because it doesn't know what minor number will get assigned.

The module should not be loaded unless it's needed.  If it's not needed, then it just sits there wasting a chunk of unswappable, unreclaimable kernel memory if the package is installed.  On the other hand, I suppose one could argue that the rpm should not be installed unless you need it.

Comment 5 Jóhann B. Guðmundsson 2011-11-30 18:10:13 UTC
Ah ok thanks for the info. 

we have a bit of modules being loaded in the legacy sysv init scripts when it's no longer necessary.

Anyway going with install argument then the package would drop an file called cachefiles.conf

which would contain...

# Load cachefiles.ko at boot
cachefiles

into the /etc/modules-load.d/ directory at install time and systemd will pick that up and load it at bootup. ( man modules-load.d for details )

cachefilesd.service would wind up looking like this

[Unit]
Description=Local network file caching management daemon

[Service]
ExecStart=/sbin/cachefilesd -n -f /etc/cachefilesd.conf

[Install]
WantedBy=multi-user.target

If loading the module when the unit is run it would look like this

[Unit]
Description=Local network file caching management daemon

[Service]
ExecStartPre=-/sbin/modprobe -qab cachefiles
ExecStart=/sbin/cachefilesd -n -f /etc/cachefilesd.conf

[Install]
WantedBy=multi-user.target

Adding "-qab" is what is considered the "correct" way of loading module(s) in units although that it's nowhere officially documented...

Comment 6 David Howells 2011-12-06 13:27:53 UTC
> into the /etc/modules-load.d/ directory at install time and systemd will pick
> that up and load it at bootup. ( man modules-load.d for details )

It occurs to me that this isn't quite sufficient unless systemd is watching the /etc/modules-load.d directory and loading any modules that get listed there - otherwise you have to reboot or manually modprobe after installing cachefilesd to be able to use it.

Comment 7 David Howells 2011-12-06 14:28:39 UTC
Review comments applied upstream:

http://git.kernel.org/?p=linux/kernel/git/dhowells/cachefilesd.git;a=commit;h=e2b4ed25c5e483d445d0b22b7fb2df4e230d114b

and put into Rawhide in version 0.10.5

Comment 8 Jóhann B. Guðmundsson 2011-12-07 09:49:04 UTC
(In reply to comment #4)
> The module cannot be autoloaded because it's accessed through a misc device
> file doesn't exist and can't exist until the module is loaded because the
> device is a "numberless" misc device - so there's no key (minor device number)
> to use to tell userspace what it wants.  And userspace can't invent a chardev
> file in advance because it doesn't know what minor number will get assigned.
> 
> The module should not be loaded unless it's needed.  If it's not needed, then
> it just sits there wasting a chunk of unswappable, unreclaimable kernel memory
> if the package is installed.  On the other hand, I suppose one could argue that
> the rpm should not be installed unless you need it.

According to Kay the solution to your problem was fixed long ago [1] and this could easily be made working thus not necessary to load the module in the unit 

1.http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=commitdiff;h=8905aaafb4b5d9764c5b4b54c7d03eb41bb0a7e9

Anyway have a look at the above and ship things as they are now you can introduce fix later via update.

Comment 9 Jóhann B. Guðmundsson 2011-12-07 09:54:03 UTC
(In reply to comment #6)
> > into the /etc/modules-load.d/ directory at install time and systemd will pick
> > that up and load it at bootup. ( man modules-load.d for details )
> 
> It occurs to me that this isn't quite sufficient unless systemd is watching the
> /etc/modules-load.d directory and loading any modules that get listed there -
> otherwise you have to reboot or manually modprobe after installing cachefilesd
> to be able to use it.

Well users/package would have to run systemctl restart systemd-modules-load.service.

Come to think of I think we should be able to restart the service once an configuration is dropped into that directory. 

I'm going to speak with the guys and see if there was any particular reason why we dont have a path unit that just restarts the service once a file has been dropped into that directory.

Comment 10 David Howells 2011-12-07 12:11:00 UTC
(In reply to comment #8)
> According to Kay the solution to your problem was fixed long ago [1] and this
> could easily be made working thus not necessary to load the module in the unit 

This requires a minor number to be assigned to the driver.  I was told that I should use MISC_DYNAMIC_MINOR rather than assigning a specific number.

Comment 11 Jóhann B. Guðmundsson 2011-12-07 14:02:20 UTC
(In reply to comment #10)
> (In reply to comment #8)
> > According to Kay the solution to your problem was fixed long ago [1] and this
> > could easily be made working thus not necessary to load the module in the unit 
> 
> This requires a minor number to be assigned to the driver.  I was told that I
> should use MISC_DYNAMIC_MINOR rather than assigning a specific number.

Oh I see I guess it has it's reasons anyway I should mention that you can ignore the comment in miscdevice.h about the minor numbers table that says allocations are managed by device since it's all managed by the kernel only and with the devtpmfs no coordination is needed granted that it surely was needed to be managed in times of MAKEDEV ir static dev.rpm packages but now nobody cares.

Comment 12 Fedora End Of Life 2013-04-03 20:24:35 UTC
This bug appears to have been reported against 'rawhide' during the Fedora 19 development cycle.
Changing version to '19'.

(As we did not run this process for some time, it could affect also pre-Fedora 19 development
cycle bugs. We are very sorry. It will help us with cleanup during Fedora 19 End Of Life. Thank you.)

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora19

Comment 13 Jóhann B. Guðmundsson 2014-05-26 09:44:22 UTC
Given that I have left the project and a new individual may or may not continue with systemd integration in the project by submitting new feature following whatever demands FPC and FESCo might have and thus new units in the process I'm closing this and all remaining bugs I had submitted for this particular feature as WONTFIX


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