diff mbox

[11/13] srp_daemon: Add the debian initscripts as an option

Message ID 1474658228-5390-12-git-send-email-jgunthorpe@obsidianresearch.com (mailing list archive)
State Accepted
Headers show

Commit Message

Jason Gunthorpe Sept. 23, 2016, 7:17 p.m. UTC
Necessary to reproduce the Debian packaging.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
---
 CMakeLists.txt                       |  3 ++
 srp_daemon/srp_daemon/CMakeLists.txt | 29 ++++++++----
 srp_daemon/srptools.default          | 14 ++++++
 srp_daemon/srptools.init             | 89 ++++++++++++++++++++++++++++++++++++
 4 files changed, 126 insertions(+), 9 deletions(-)
 create mode 100644 srp_daemon/srptools.default
 create mode 100644 srp_daemon/srptools.init

Comments

Doug Ledford Sept. 28, 2016, 6:27 p.m. UTC | #1
On 9/23/16 3:17 PM, Jason Gunthorpe wrote:
> Necessary to reproduce the Debian packaging.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> ---
>  CMakeLists.txt                       |  3 ++
>  srp_daemon/srp_daemon/CMakeLists.txt | 29 ++++++++----
>  srp_daemon/srptools.default          | 14 ++++++
>  srp_daemon/srptools.init             | 89 ++++++++++++++++++++++++++++++++++++

Would it be best to have directories for related install files for a
specific OS?  For instance, srp_daemon/debian/ and srp_daemon/redhat/?
It's not that I expect to put a redhat init script for srpd in place,
but the script for debian is likely to not work properly on redhat, so
having it in an OS directory would at least make that clear.
Jason Gunthorpe Sept. 28, 2016, 6:47 p.m. UTC | #2
On Wed, Sep 28, 2016 at 02:27:01PM -0400, Doug Ledford wrote:
> On 9/23/16 3:17 PM, Jason Gunthorpe wrote:
> > Necessary to reproduce the Debian packaging.
> > 
> > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> >  CMakeLists.txt                       |  3 ++
> >  srp_daemon/srp_daemon/CMakeLists.txt | 29 ++++++++----
> >  srp_daemon/srptools.default          | 14 ++++++
> >  srp_daemon/srptools.init             | 89 ++++++++++++++++++++++++++++++++++++
> 
> Would it be best to have directories for related install files for a
> specific OS?  For instance, srp_daemon/debian/ and srp_daemon/redhat/?
> It's not that I expect to put a redhat init script for srpd in place,
> but the script for debian is likely to not work properly on redhat, so
> having it in an OS directory would at least make that clear.

To be clear, there is already an initscript that (perhaps?) is for
RedHat - but it isn't even close to the Debian version. So this patch
introduces two scripts for srp_dameon, which I hated doing..

I'll drop this patch from the series and we can go ahead with the
other patches in the series.

For now I'll put the initscript in the Debian packaging patch and we
can think about what to do later. I think it can be moved to the
debian/ directory as well.

Going forward I think we need to make some decisions..

1) Do we want to do something with the initscripts so distros can use
   them? Is that even possible? I think Debian uses the bundled
   acm initscript, didn't look at suse.

   Is Debian the only major distro that still ships init scripts?
   Does FC/RH exclusively ship systemd unit files now?

   Maybe we should delete the initscripts entirely.

2) I'd like to support cross-distro systemd unit files upstream.
   Is that feasible? We are short unit files, could you
   contribute yours?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Ledford Sept. 28, 2016, 6:55 p.m. UTC | #3
On 9/28/16 2:47 PM, Jason Gunthorpe wrote:
> On Wed, Sep 28, 2016 at 02:27:01PM -0400, Doug Ledford wrote:
>> On 9/23/16 3:17 PM, Jason Gunthorpe wrote:
>>> Necessary to reproduce the Debian packaging.
>>>
>>> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>>>  CMakeLists.txt                       |  3 ++
>>>  srp_daemon/srp_daemon/CMakeLists.txt | 29 ++++++++----
>>>  srp_daemon/srptools.default          | 14 ++++++
>>>  srp_daemon/srptools.init             | 89 ++++++++++++++++++++++++++++++++++++
>>
>> Would it be best to have directories for related install files for a
>> specific OS?  For instance, srp_daemon/debian/ and srp_daemon/redhat/?
>> It's not that I expect to put a redhat init script for srpd in place,
>> but the script for debian is likely to not work properly on redhat, so
>> having it in an OS directory would at least make that clear.
> 
> To be clear, there is already an initscript that (perhaps?) is for
> RedHat - but it isn't even close to the Debian version. So this patch
> introduces two scripts for srp_dameon, which I hated doing..
> 
> I'll drop this patch from the series and we can go ahead with the
> other patches in the series.
> 
> For now I'll put the initscript in the Debian packaging patch and we
> can think about what to do later. I think it can be moved to the
> debian/ directory as well.
> 
> Going forward I think we need to make some decisions..
> 
> 1) Do we want to do something with the initscripts so distros can use
>    them? Is that even possible? I think Debian uses the bundled
>    acm initscript, didn't look at suse.

I'm in favor of providing a good, reliable, correct set of startup files
for each of the major distro flavors.  One of my main reasons for that
is it makes it possible for us to try and provide some level of startup
script parity and commonality between the distros.  And allows us to fix
bugs across all the distros as once even if we didn't necessarily hit
the bug on each distro.  Of course, the distros may ignore our scripts,
but we can try.

> 
>    Is Debian the only major distro that still ships init scripts?
>    Does FC/RH exclusively ship systemd unit files now?

We're almost entirely systemd now.  We only have EL 6 that still uses
init scripts, and I doubt we will ever put this package into EL 6.  So,
we might as well be all systemd as far as this repo is concerned.

>    Maybe we should delete the initscripts entirely.

Not if Debian still uses it.  And I'm not opposed to providing a Red Hat
init script in case someone wants to put this package on EL 6 themselves.

> 2) I'd like to support cross-distro systemd unit files upstream.
>    Is that feasible?

Maybe....I'm not entirely sure about that.

> We are short unit files, could you
>    contribute yours?

Absolutely.  I'd like to basically import the entire redhat rdma package
into this, but it will take a little sorting things out to get all of
the files and such in the right place.  And that's a precursor to our
srpd unit file as it lists a specific dependency on the rdma unit file.
Bart Van Assche Sept. 28, 2016, 8:18 p.m. UTC | #4
On 09/28/2016 11:47 AM, Jason Gunthorpe wrote:
> To be clear, there is already an initscript that (perhaps?) is for
> RedHat - but it isn't even close to the Debian version. So this patch
> introduces two scripts for srp_dameon, which I hated doing..

The existing init script works reliably on multiple versions of RHEL, 
SLES, Fedora and openSuSE. It is not specific to Red Hat distro's. So 
I'm surprised that a separate initscript is needed for Debian?

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jason Gunthorpe Sept. 28, 2016, 9:01 p.m. UTC | #5
On Wed, Sep 28, 2016 at 01:18:51PM -0700, Bart Van Assche wrote:
> On 09/28/2016 11:47 AM, Jason Gunthorpe wrote:
> >To be clear, there is already an initscript that (perhaps?) is for
> >RedHat - but it isn't even close to the Debian version. So this patch
> >introduces two scripts for srp_dameon, which I hated doing..
> 
> The existing init script works reliably on multiple versions of RHEL, SLES,
> Fedora and openSuSE. It is not specific to Red Hat distro's. So I'm
> surprised that a separate initscript is needed for Debian?

I suspect not being distro specific is the entire problem, eg the
Debian initscript is using /etc/default/srptools to configure
how the daemon is launched and it uses the start-stop-daemon tool.

A good initscript should consistently follow the distro policies for
initscripts..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Doug Ledford Sept. 28, 2016, 9:11 p.m. UTC | #6
On 9/28/16 5:01 PM, Jason Gunthorpe wrote:
> On Wed, Sep 28, 2016 at 01:18:51PM -0700, Bart Van Assche wrote:
>> On 09/28/2016 11:47 AM, Jason Gunthorpe wrote:
>>> To be clear, there is already an initscript that (perhaps?) is for
>>> RedHat - but it isn't even close to the Debian version. So this patch
>>> introduces two scripts for srp_dameon, which I hated doing..
>>
>> The existing init script works reliably on multiple versions of RHEL, SLES,
>> Fedora and openSuSE. It is not specific to Red Hat distro's. So I'm
>> surprised that a separate initscript is needed for Debian?
> 
> I suspect not being distro specific is the entire problem,

Indeed...

> eg the
> Debian initscript is using /etc/default/srptools to configure
> how the daemon is launched and it uses the start-stop-daemon tool.
> 
> A good initscript should consistently follow the distro policies for
> initscripts..

Exactly why we often don't use the ones in the upstream package.  Try to
be generically good, and you will be universally, generically bad for
any specific distro.  If you want to provide one as an example, that's
fine.  If you want to provide one that's actually used, then you need
one per distro (or a means of generating a distro specific one from a
template or some such).
Jason Gunthorpe Sept. 28, 2016, 9:13 p.m. UTC | #7
On Wed, Sep 28, 2016 at 02:55:11PM -0400, Doug Ledford wrote:

> I'm in favor of providing a good, reliable, correct set of startup files
> for each of the major distro flavors.

Okay, how about a initscripts/{debian,centos6,suse..} directory tree,
and centralize all the initscripts there? Some one who understands
this can then try and eventually make sense of the duplication?

> > We are short unit files, could you
> >    contribute yours?
> 
> Absolutely.  I'd like to basically import the entire redhat rdma package
> into this, but it will take a little sorting things out to get all of
> the files and such in the right place.  And that's a precursor to our
> srpd unit file as it lists a specific dependency on the rdma unit file.

Yeah, that rdma unit file and udev integration really needs to be
upstream and in all distros if we are going to have any hope of having
upstream unit files...

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 7ae236f88f67..805b25bb8669 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -69,6 +69,9 @@  else()
   set(CMAKE_INSTALL_FULL_RUNDIR "${CMAKE_INSTALL_RUNDIR}")
 endif()
 
+set(DISTRO_FLAVOUR "None" CACHE
+  STRING "Flavour of distribution to install for. This primarily impacts the init.d scripts installed.")
+
 #-------------------------
 # Load CMake components
 set(BUILDLIB "${CMAKE_SOURCE_DIR}/buildlib")
diff --git a/srp_daemon/srp_daemon/CMakeLists.txt b/srp_daemon/srp_daemon/CMakeLists.txt
index fe6b41811b92..4ad4edaf8c33 100644
--- a/srp_daemon/srp_daemon/CMakeLists.txt
+++ b/srp_daemon/srp_daemon/CMakeLists.txt
@@ -23,12 +23,23 @@  install(FILES logrotate-srp_daemon DESTINATION "${CMAKE_INSTALL_SYSCONFDIR}/logr
 install(FILES rsyslog-srp_daemon.conf DESTINATION "${CMAKE_INSTALL_SYSCONFDIR}/rsyslog.d" RENAME "srp_daemon.conf")
 install(FILES srp_daemon.conf DESTINATION "${CMAKE_INSTALL_SYSCONFDIR}")
 
-# FIXME: The ib init.d file should really be included in rdma-core as well.
-set(RDMA_SERVICE "openibd" CACHE STRING "init.d file service name to order srpd after")
-# NOTE: These defaults are for CentOS, packagers should override.
-set(SRP_DEFAULT_START "2 3 4 5" CACHE STRING "Default-Start service data for srpd")
-set(SRP_DEFAULT_STOP "0 1 6" CACHE STRING "Default-Stop service data for srpd")
-configure_file(srpd.in "${CMAKE_CURRENT_BINARY_DIR}/srpd")
-install(FILES "${CMAKE_CURRENT_BINARY_DIR}/srpd"
-  DESTINATION "${CMAKE_INSTALL_INITDDIR}"
-  PERMISSIONS OWNER_WRITE OWNER_READ GROUP_READ WORLD_READ OWNER_EXECUTE GROUP_EXECUTE WORLD_EXECUTE)
+if ("${DISTRO_FLAVOUR}" STREQUAL "Debian")
+  # Debian version of the initscript system
+  install(FILES "srptools.init"
+    DESTINATION "${CMAKE_INSTALL_INITDDIR}"
+    RENAME "srptools"
+    PERMISSIONS OWNER_WRITE OWNER_READ GROUP_READ WORLD_READ OWNER_EXECUTE GROUP_EXECUTE WORLD_EXECUTE)
+  install(FILES "srptools.default"
+    DESTINATION "${CMAKE_INSTALL_SYSCONFDIR}/default/"
+    RENAME "srptools")
+else()
+  # FIXME: The ib init.d file should really be included in rdma-core as well.
+  set(RDMA_SERVICE "openibd" CACHE STRING "init.d file service name to order srpd after")
+  # NOTE: These defaults are for CentOS, packagers should override.
+  set(SRP_DEFAULT_START "2 3 4 5" CACHE STRING "Default-Start service data for srpd")
+  set(SRP_DEFAULT_STOP "0 1 6" CACHE STRING "Default-Stop service data for srpd")
+  configure_file(srpd.in "${CMAKE_CURRENT_BINARY_DIR}/srpd")
+  install(FILES "${CMAKE_CURRENT_BINARY_DIR}/srpd"
+    DESTINATION "${CMAKE_INSTALL_INITDDIR}"
+    PERMISSIONS OWNER_WRITE OWNER_READ GROUP_READ WORLD_READ OWNER_EXECUTE GROUP_EXECUTE WORLD_EXECUTE)
+endif()
diff --git a/srp_daemon/srptools.default b/srp_daemon/srptools.default
new file mode 100644
index 000000000000..81e84f4a5cba
--- /dev/null
+++ b/srp_daemon/srptools.default
@@ -0,0 +1,14 @@ 
+#How often should srpdeamon  rescan the fabric (seconds)
+RETRIES=60
+
+#Where should srp-deamon log to
+LOG=/var/log/srp_daemon.log
+
+# What ports should srp-deamon be started on.
+# Format is CA:port
+# ALL or NONE will run on all ports on none
+# respectively
+
+PORTS=NONE
+#PORTS=ALL
+#PORTS="mthca0:1 mlx4_0:2"
diff --git a/srp_daemon/srptools.init b/srp_daemon/srptools.init
new file mode 100644
index 000000000000..2c1a140ccbc3
--- /dev/null
+++ b/srp_daemon/srptools.init
@@ -0,0 +1,89 @@ 
+#!/bin/bash
+### BEGIN INIT INFO
+# Provides:          srptools
+# Required-Start:    $remote_fs $syslog
+# Required-Stop:     $remote_fs $syslog
+# Default-Start:     2 3 4 5
+# Default-Stop:      0 1 6
+# Short-Description: Discovers SRP scsi targets.
+# Description:       Discovers SRP scsi over infiniband targets.
+### END INIT INFO
+
+[ -x /usr/sbin/srp_daemon ] || exit 0
+
+IBDIR=/sys/class/infiniband
+
+PORTS=""
+RETRIES=""
+LOG=""
+
+[ -f /etc/default/srptools ] &&  . /etc/default/srptools
+
+start_daemon () {
+
+if [ "$PORTS" = "NONE" ] ; then
+echo "srptools disabled."
+exit 0
+fi
+
+
+if [ "$PORTS" = "ALL" ]  ; then
+    for HCA_ID in `/bin/ls -1 ${IBDIR}`
+      do
+      for PORT in `/bin/ls -1 ${IBDIR}/${HCA_ID}/ports/`
+        do
+        run_daemon
+      done
+    done
+fi
+
+
+for ADAPTER in $PORTS ; do
+    HCA_ID=`echo $ADAPTER | awk -F: '{print $1}'`
+    PORT=`echo $ADAPTER | awk -F:  '{print $2}'`
+    run_daemon
+done
+}
+
+
+run_daemon() {
+# SRP deamon wedges if we start it on a port which is not up
+
+        STATUS=`/usr/sbin/ibstat $HCA_ID $PORT | grep "State:"`
+
+        if [ "$STATUS" = "State: Active" ] ; then
+            echo "Starting srp on $HCA_ID $PORT"
+
+# srp does not background itself; using the start-stop-daemon background function
+# causes us to lose stdout, which is where it logs to
+            nohup start-stop-daemon --start --quiet -m --pidfile /var/run/srp_daemon.${HCA_ID}.${PORT} \
+            --exec  /usr/sbin/srp_daemon -- -e -c -n -i ${HCA_ID} -p ${PORT} -R ${RETRIES}   >> $LOG 2>&1 &
+            RETVAL=$?
+        fi
+}
+
+stop_daemon () {
+     for HCA_ID in `/bin/ls -1 ${IBDIR}`
+      do
+      for PORT in `/bin/ls -1 ${IBDIR}/${HCA_ID}/ports/`
+        do
+        start-stop-daemon --stop --quiet --oknodo -m --pidfile /var/run/srp_daemon.${HCA_ID}.${PORT}
+        RETVAL=$?
+      done
+    done
+}
+
+
+case "$1" in
+
+start)
+start_daemon
+;;
+stop)
+stop_daemon
+;;
+restart | reload | force-reload )
+stop_daemon
+start_daemon
+;;
+esac