Bug 714683

As far as I know, unit files do not provide any way to load kernel modules based on user configuration.

Upstream expects init script to handle this, storing configuration in an environment file under /etc/fcoe/*.  We'll need to create a one-shot script to prepare the environment, working much like the current init script, and then the real unit file, depending on it.

For some reason, it makes me laugh. Almost.

hum.. 

Afaik modules should be loaded via autoloading based on bus information, or via /etc/modules-load.d/*.conf. and unloading a module from the kernel should not be done except for debugging purposes so the question remains if this should not be fixed in the process?

If that cant be done cant users use /etc/sysconfig/fcoe with "FCOE_MODULES=" for that and you would put 

EnvironmentFile=-/etc/sysconfig/fcoe
ExecStartPre=-/sbin/modprobe -qab $FCOE_MODULES"

In the service file?

(In reply to comment #2)
> hum.. 
> 
> Afaik modules should be loaded via autoloading based on bus information, or via
> /etc/modules-load.d/*.conf. and unloading a module from the kernel should not
> be done except for debugging purposes so the question remains if this should
> not be fixed in the process?

Is there some policy for this?  I remember people complaining about a service not unloading its modules when it was stopped.  Is that no longer the case?

> 
> If that cant be done cant users use /etc/sysconfig/fcoe with "FCOE_MODULES="
> for that and you would put 
> 
> EnvironmentFile=-/etc/sysconfig/fcoe
> ExecStartPre=-/sbin/modprobe -qab $FCOE_MODULES"
> 
> In the service file?

/etc/fcoe/config currently contains SUPPORTED_DRIVERS, this could be used like that. That's a good point.

There are two other options in the Fedora config file -- whether to use syslog (which doesn't make sense now) and a DEBUG boolean.  This might change in the future and we might need some (not only) parsing script anyway.  For example, there's a TIMEOUT option (Bug 658076) in RHEL6 which I'd like to see in Fedora as well.  I wonder if it's possible to do something like this with systemd.

I'll go through the systemd documentation to see if there's a way out...

Created attachment 505631 [details]
Native systemd service file for fcoe.service

Just sample for you to work with

Created attachment 505632 [details]
/etc/sysconfig/fcoe

Sample fcoe config file

(In reply to comment #3)
> (In reply to comment #2)
> > hum.. 
> > 
> > Afaik modules should be loaded via autoloading based on bus information, or via
> > /etc/modules-load.d/*.conf. and unloading a module from the kernel should not
> > be done except for debugging purposes so the question remains if this should
> > not be fixed in the process?
> 
> Is there some policy for this?  I remember people complaining about a service
> not unloading its modules when it was stopped.  Is that no longer the case?

I'm not aware of anything written but if you get complaints from users you can add ExecStopPost=-/sbin/modprobe -qabr $FCOE_MODULES to the service file

> 
> > 
> > If that cant be done cant users use /etc/sysconfig/fcoe with "FCOE_MODULES="
> > for that and you would put 
> > 
> > EnvironmentFile=-/etc/sysconfig/fcoe
> > ExecStartPre=-/sbin/modprobe -qab $FCOE_MODULES"
> > 
> > In the service file?
> 
> /etc/fcoe/config currently contains SUPPORTED_DRIVERS, this could be used like
> that. That's a good point.
> 
> There are two other options in the Fedora config file -- whether to use syslog
> (which doesn't make sense now) and a DEBUG boolean.  This might change in the
> future and we might need some (not only) parsing script anyway.  For example,
> there's a TIMEOUT option (Bug 658076) in RHEL6 which I'd like to see in Fedora
> as well.  I wonder if it's possible to do something like this with systemd.
> 
> I'll go through the systemd documentation to see if there's a way out...

I added two timeout options to the service file which you can uncomment the one you will end up using and and remove the other one and you can test it by adding the the service file to /lib/systemd/system and the fcoe file to /etc/sysconfig/fcoe ( or add those option to /etc/fcoe/config and adjust the service file to use that instead if you prefer ). 

Then run 

systemctl daemon-reload 

And 

systemctl start/status/stop fcoe.service

Created attachment 505655 [details]
Sample conf file to load fcoe module at boot

Basically you would drop the modprobe line from the systemd service script and administrators copy the file from /usr/lib to /etc/modules-load.d and add any additional modules to load in that file should the need arise.

See man modules-load.d for further details...

Created attachment 505720 [details]
native systemd service file for lldapd.service...

(In reply to comment #5)
> Created attachment 505632 [details]
> /etc/sysconfig/fcoe
> 
> Sample fcoe config file

So we will provide another configuration file in addition to the upstream one?  Or would you like to remove the upstream configuration and drop SysV compatibility altogether?  I hope not.


(In reply to comment #6)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > hum.. 
> > > 
> > > Afaik modules should be loaded via autoloading based on bus information, or via
> > > /etc/modules-load.d/*.conf. and unloading a module from the kernel should not
> > > be done except for debugging purposes so the question remains if this should
> > > not be fixed in the process?
> > 
> > Is there some policy for this?  I remember people complaining about a service
> > not unloading its modules when it was stopped.  Is that no longer the case?
> 
> I'm not aware of anything written but if you get complaints from users you can
> add ExecStopPost=-/sbin/modprobe -qabr $FCOE_MODULES to the service file
> 

Thanks, looks okay.

> > 
> > > 
> > > If that cant be done cant users use /etc/sysconfig/fcoe with "FCOE_MODULES="
> > > for that and you would put 
> > > 
> > > EnvironmentFile=-/etc/sysconfig/fcoe
> > > ExecStartPre=-/sbin/modprobe -qab $FCOE_MODULES"
> > > 
> > > In the service file?
> > 
> > /etc/fcoe/config currently contains SUPPORTED_DRIVERS, this could be used like
> > that. That's a good point.
> > 
> > There are two other options in the Fedora config file -- whether to use syslog
> > (which doesn't make sense now) and a DEBUG boolean.  This might change in the
> > future and we might need some (not only) parsing script anyway.  For example,
> > there's a TIMEOUT option (Bug 658076) in RHEL6 which I'd like to see in Fedora
> > as well.  I wonder if it's possible to do something like this with systemd.
> > 
> > I'll go through the systemd documentation to see if there's a way out...
> 
> I added two timeout options to the service file which you can uncomment the one
> you will end up using and and remove the other one and you can test it by
> adding the the service file to /lib/systemd/system and the fcoe file to
> /etc/sysconfig/fcoe ( or add those option to /etc/fcoe/config and adjust the
> service file to use that instead if you prefer ). 

None of those does what we need.  The case here is: although the service is started successfully, it waits afterwards for the '_netdev' flagged devices in /etc/fstab to appear, which may take some time (to get an idea, 65s is the default).  If they don't make it, the service fails.  If they make it sooner, we continue immediately.

I suppose there's a way to tell systemd to delay netfs.service but we most certainly don't want to wait every time, only when it's needed.


(In reply to comment #8)
> Created attachment 505720 [details]
> native systemd service file for lldapd.service...

This is nice, thank you!

Just a thought:

netfs.service could depend on *.device/*.path _netdev units, leaving fcoe.service out of that.

(In reply to comment #9)
> (In reply to comment #5)
> > Created attachment 505632 [details]
> > /etc/sysconfig/fcoe
> > 
> > Sample fcoe config file
> 
> So we will provide another configuration file in addition to the upstream one? 

That or add the FCOE_MODULES="libfc fcoe" and FCOEMON_OPTS="--syslog" and change EnvironmentFile=-/etc/fcoe/config since you need to pass the actual options to the daemon as opposed to be using yes or no which is stupid from my pov anyway since the admin could just as well unhash a line that contains the actual option being passed to the daemon. 

I thought /etc/sysconfig/foo was the standard but apparently it's not we have one debian another and suse probably the third etc..

> Or would you like to remove the upstream configuration and drop SysV
> compatibility altogether?  I hope not.

Nope certainly not but be advised that per the https://fedoraproject.org/wiki/Packaging:Guidelines:Systemd the old sysv init scripts need to be packed separately. 

> 
> None of those does what we need.  The case here is: although the service is
> started successfully, it waits afterwards for the '_netdev' flagged devices in
> /etc/fstab to appear, which may take some time (to get an idea, 65s is the
> default).  If they don't make it, the service fails.  If they make it sooner,
> we continue immediately.
> 
> I suppose there's a way to tell systemd to delay netfs.service but we most
> certainly don't want to wait every time, only when it's needed.

systemd is extremly flexible in that regard but I would think the smartest is for services in general is to signal the daemon when it's up or (re)start the service when it is depending on the needed behaviour there as opposed to have the daemon wait until it's up. 

I need to take a look at netfs.service to see what you guys are dealing with

(In reply to comment #10)
> Just a thought:
> 
> netfs.service could depend on *.device/*.path _netdev units, leaving
> fcoe.service out of that.

Looks like a smarter approach if doable.

By the way, I've created bug 714874 for lldpad.

Just out of curiosity is the fcoe service applicant for hardware activation unfortunately our guidelines are a bit short on how to do that 

"Hardware activation

Hardware activation occurs when a service is installed but only turns on if a
certain type of hardware is installed. Enabling of the service is normally done
with a udev rule. At this time we do not have further guidance on how to write
those udev rules. The service itself installs its .service files in the normal
places and are installed by the normal systemd scriptlets. These services
should never be enabled by the package as they will be enabled by udev. "

But you can read man systemd.device and look at the bluetooth service which is
doing that I believe..

(In reply to comment #14)
> Just out of curiosity is the fcoe service applicant for hardware activation
> unfortunately our guidelines are a bit short on how to do that 
> 
> "Hardware activation
> 
> Hardware activation occurs when a service is installed but only turns on if a
> certain type of hardware is installed. Enabling of the service is normally done
> with a udev rule. At this time we do not have further guidance on how to write
> those udev rules. The service itself installs its .service files in the normal
> places and are installed by the normal systemd scriptlets. These services
> should never be enabled by the package as they will be enabled by udev. "
> 
> But you can read man systemd.device and look at the bluetooth service which is
> doing that I believe..

That's an interesting idea but difficult to implement unless CEE-enabled NICs report those capabilities somehow.

A nice to have feature but I don't find it essential.

(By the way, bluetooth.service doesn't do this.)

> That's an interesting idea but difficult to implement unless CEE-enabled NICs
> report those capabilities somehow.

I'm not so sure that would be necessary but have mentioned you dont see it in the service file but udev rules instead which triggers the start of the bluetooth service. 

See

/lib/udev/rules.d/99-systemd.rules

/lib/udev/rules.d/99-systemd.rules:SUBSYSTEM=="bluetooth", TAG+="systemd", ENV{SYSTEMD_WANTS}="bluetooth.target"

Another example in /etc/udev/rules.d/99-hdapsd.rules

/etc/udev/rules.d/99-hdapsd.rules:SUBSYSTEM=="block", KERNEL=="sd[ab]", ATTRS{removable}=="0", TAG="systemd", ENV{SYSTEMD_WANTS}="hdapsd@%k.service"

Vendor based would probably look something like this

ENV{ID_VENDOR_ID}=="$foo",ENV{ID_MODEL_ID}=="$bar" TAG+="systemd", ENV{SYSTEMD_WANTS}="foo.target or bar.service" 

 
> (By the way, bluetooth.service doesn't do this.)

yeah I should have been a bit more clear that the service file looks normal but is activated by udev rule I think the general idea here is if hardware is present it trigger the service startup so there is no longer the need to "enable" the service it will just get activated if relevant hardware is present.

I'll try to ping Lennart/Kay to see if they cant blog about it or something I personally think this is an nifty udev+systemd feature which is not getting the attention it deserves unfortunately documentation about this is scarce as is with so many other things ..

So.. What's the current status of this one?

(In reply to comment #17)
> So.. What's the current status of this one?

Not much.

Let's just settle with a simple service file, like the one you proposed, reading fcoemon options from an environment file in /etc/sysconfig.  We'll differ from upstream, though.

This will cover the syslog and debug options.  Supported drivers will be handled by modules configuration elsewhere.  And as for the timeout option... chances are it won't be needed.

Created attachment 513608 [details]
New /etc/sysconfig/fcoe without the modules

Created attachment 513609 [details]
New fcoe.service without the loading of modules

I've decided to keep the modprobe stuff in, using the 'current' SUPPORTED_DRIVERS variable, now located in /etc/sysconfig/fcoe.

I'll attach the final unit and configuration files.

Created attachment 513621 [details]
FCoE service unit file, used in 1.0.20-2.fc16

Created attachment 513622 [details]
FCoE configuration file, used in 1.0.20-2.fc16

Quick q: fcoe-utils depends on lldpad, but fcoe.service only says After=lldpad.service . That means 'run after lldpad.service if lldpad.service is enabled, but we don't *need* lldpad.service'. Does fcoe.service require lldpad.service to do anything useful? If so, it should also have Requires=lldpad.service .

(In reply to comment #24)
> Quick q: fcoe-utils depends on lldpad, but fcoe.service only says
> After=lldpad.service . That means 'run after lldpad.service if lldpad.service
> is enabled, but we don't *need* lldpad.service'. Does fcoe.service require
> lldpad.service to do anything useful? If so, it should also have
> Requires=lldpad.service .

It depends on the hardware.  I'd say lldpad is highly recommended for most users but some won't need it or may actually want to disable it.