On 10/24/2013 12:39 AM, Cristian Rodríguez wrote:
El 23/10/13 15:10, Lee Duncan escribió:
Advice? Answers?
Ok, I just looked at the patch that implements this bits, some comments...
I appreciate your time.
- systemd unit files are NOT and should never be marked as %config
I have created Bug#847953 to track this issue, as well as one other issue, below.
- why is LOCK_DIR defined in the command line as /etc/iscsi ? that does not look quite right.. use /run/lock/iscsi and provide a tmpfiles.d snippet with d /run/lock/iscsi 0700 root root -
This is not a lock directory but rather the home of the open-iscsi run-time implementation. This directory has the open-iscsi configuration file, interface definitions and configurations, as well as holding the NODE and target persistence database. The term LOCK_DIR is perhaps unfortunate but is historic.
- about starting the service only when needed.. if the presence of an iscsi device happens to trigger a kernel event you could detect such change with udev , create an appropiate rule and use TAG+="systemd", ENV{SYSTEMD_WANTS}+="yourservice.service" to automagically start the needed stuff on demand. I have never used iscsi so I have no idea if this the case.
Kay has made it clear that it is the job of systemd and not udev to handle these device events, which is much of the motiavation for this open-iscsi makeover. There is no need for this extra udev clutter if either (1) the systemd unit files are configure correctly, or (2) the iscsi daemon is just configured to run all the time, as it is now. But the "systemd way" to handle this is via socket-based on-demand starting of the iscsi daemon.
- iscsid.service is missing an "Also=iscsid.socket" in the [Install] section
This is also added to bug#847953. Thanks again for your review. -- Lee Duncan SUSE Labs -- To unsubscribe, e-mail: opensuse-packaging+unsubscribe@opensuse.org To contact the owner, e-mail: opensuse-packaging+owner@opensuse.org