Message ID | 1474658228-5390-12-git-send-email-jgunthorpe@obsidianresearch.com (mailing list archive) |
---|---|
State | Accepted |
Headers | show |
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.
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
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.
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
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
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).
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 --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
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