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

[Michał Piotrowski]

W dniu 29 marca 2011 20:48 użytkownik Lennart Poettering
<lennart at poettering.net> napisał:
> 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.

Right, but in such a way lm_sensors works - I do not want to change
the world I just want to write systemd service :)

>
> 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!

Indeed :)

>
>> [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.

I tried
ExecStop=-/sbin/modprobe -qabr $BUS_MODULES $HWMON_MODULES
too and also does not work.

ExecStop for other services works fine

>
> 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...

I'll write your suggestion when I'll submit service to Fedora
lm_sensor maintainer.

>
> Lennart
>
> --
> Lennart Poettering - Red Hat, Inc.
>

Thanks for the tips!

-- 
Best regards,
Michal

http://eventhorizon.pl/