Re: PATCH: systemd integration

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Hans,

On Sun, 24 Apr 2011 16:17:16 +0200, Hans de Goede wrote:
> The attached patch:
> 1) uses systemd's systemctl to start / stop / enable the lm_sensors service on
> systemd systems (such as Fedora 15)
> 2) recognizes the new /run dir for run time info (as used by systemd systems)
> as a possible place where the udev db can live
> 3) adds a lm_sensor.service file to install under /lib/systemd/system
> 
> Please review, I'll push it to svn myself once acked.

I don't know anything about systemd, but the patch looks good and I'm
all for better integration of upstream lm-sensors so that every
distribution doesn't have to do it all again.

Comments:

> Index: prog/init/lm_sensors.service
> ===================================================================
> --- prog/init/lm_sensors.service	(revision 0)
> +++ prog/init/lm_sensors.service	(revision 0)
> @@ -0,0 +1,14 @@
> +[Unit]
> +Description=lm_sensors for monitoring motherboard sensor values

lm-sensors is not (or shouldn't be) limited to motherboard sensors.
Sensors on CPU, memory modules and graphics cards are supported too, and
hopefully hard disk drive temperatures will be someday too.

> +After=syslog.target
> +
> +[Service]
> +EnvironmentFile=/etc/sysconfig/lm_sensors
> +Type=oneshot
> +RemainAfterExit=yes
> +ExecStart=-/sbin/modprobe -qab $BUS_MODULES $HWMON_MODULES
> +ExecStart=/usr/bin/sensors -s
> +ExecStop=-/sbin/modprobe -qabr $BUS_MODULES $HWMON_MODULES

Please keep in mind that both $BUS_MODULES and $HWMON_MODULES may be
empty, which would cause the above modprobe commands to return an
error. Still it is legitimate to start the service so that "sensors -s"
is executed.

> +
> +[Install]
> +WantedBy=multi-user.target
> Index: prog/detect/sensors-detect
> ===================================================================
> --- prog/detect/sensors-detect	(revision 5939)
> +++ prog/detect/sensors-detect	(working copy)
> @@ -2339,7 +2339,7 @@
>  	if (!$use_udev) {
>  		# Try some known default udev db locations, just in case
>  		if (-e '/dev/.udev.tdb' || -e '/dev/.udev'
> -		 || -e '/dev/.udevdb') {
> +		 || -e '/dev/.udevdb' || -e '/run/udev') {
>  			$use_udev = 1;
>  			$dev_i2c = '/dev/i2c-';
>  		}
> @@ -6378,6 +6378,14 @@
>  		}
>  		close(SYSCONFIG);
>  
> +		if (-x "/bin/systemctl" &&
> +		    -f "/lib/systemd/system/lm_sensors.service") {
> +			system("/bin/systemctl", "enable", "lm_sensors.service");
> +			system("/bin/systemctl", "start", "lm_sensors.service");
> +			# All done, don't check for /etc/init.d/lm_sensors
> +			return;
> +		}
> +
>  		print "Copy prog/init/lm_sensors.init to /etc/init.d/lm_sensors\n".
>  		      "for initialization at boot time.\n"
>  			unless -f "/etc/init.d/lm_sensors";

Don't you want to display a smiliar message for the systemd case? Users
installing manually will have to copy prog/init/lm_sensors.service
to /lib/systemd/system/lm_sensors.service for initialization at boot
time.

> @@ -6433,8 +6441,10 @@
>  		exit -1;
>  	}
>  
> -	if (-x "/sbin/service" && -f "/etc/init.d/lm_sensors" &&
> -	    -f "/var/lock/subsys/lm_sensors") {
> +	if (-x "/bin/systemctl" && -f "/lib/systemd/system/lm_sensors.service") {
> +		system("/bin/systemctl", "stop", "lm_sensors.service");
> +	} elsif (-x "/sbin/service" && -f "/etc/init.d/lm_sensors" &&
> +		 -f "/var/lock/subsys/lm_sensors") {
>  		system("/sbin/service", "lm_sensors", "stop");
>  	}
>  

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors


[Index of Archives]     [Linux Kernel]     [Linux Hardware Monitoring]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux