[systemd-devel] problem with service that uses bash loop

Lennart Poettering lennart at poettering.net
Tue Mar 29 11:48:09 PDT 2011


On Tue, 29.03.11 20:30, Michał Piotrowski (mkkp4x4 at gmail.com) wrote:

> Hi,
> 
> I've got a service
> 
> [Unit]
> Description=lm_sensors for monitoring motherboard sensor values
> After=syslog.target
> 
> [Service]
> EnvironmentFile=/etc/sysconfig/lm_sensors
> Type=oneshot
> ExecStart=/bin/bash -c "for module in $BUS_MODULES $HWMON_MODULES; do
> /sbin/modprobe $module >/dev/null 2>&1; done"
> ExecStart=/usr/bin/sensors -s

Urks. Generally, modules should be loaded via autoloading based on bus
information, or via /etc/modules-load.d/*.conf. 

Adding fifty ways to load your modules, seperate for each subsystem,
doesn't appear so nice to me.

Embedding shell into unit files makes me unhappy too, as we try to
minimize the use of shell.

With a script like yours you will override the user's
blacklist. Probably a bad idea. Better use "modprobe -b".

Instead of "> /dev/null 2>&1" you should use "modprobe -q".

And instead of looping you should use "modprobe -a".

Result:

ExecStart=-/sbin/modprobe -qab $BUS_MODULES $HWMON_MODULES

Much nicer!

> [Install]
> WantedBy=multi-user.target
> 
> It works well until I add
> ExecStop=/bin/bash -c "for module in $HWMON_MODULES $BUS_MODULES; do
> /sbin/modprobe -r $module >/dev/null 2>&1; done"
> 
> Is there some magical way in which I can make a simple loop in
> ExecStop?

Hmm, are you suggesting that this works in ExecStart but not in
ExecStop? That would be a bug.

It is generally considered a bad idea to unload modules from the kernel
except for debug purposes, hence you shouldn't add the ExecStop=
anyway. But that doesn't mean we shouldn't fix our bug here that we
apparently handle ExecStop differently from ExecStart...

Lennart

-- 
Lennart Poettering - Red Hat, Inc.


More information about the systemd-devel mailing list