From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.windriver.com (mail.windriver.com [147.11.1.11]) by mx1.pokylinux.org (Postfix) with ESMTP id 4E8104C808DE for ; Sun, 15 May 2011 20:34:29 -0500 (CDT) Received: from ALA-HCA.corp.ad.wrs.com (ala-hca [147.11.189.40]) by mail.windriver.com (8.14.3/8.14.3) with ESMTP id p4G1YSjO001492 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=FAIL); Sun, 15 May 2011 18:34:28 -0700 (PDT) Received: from [128.224.163.140] (128.224.163.140) by ALA-HCA.corp.ad.wrs.com (147.11.189.50) with Microsoft SMTP Server id 14.1.255.0; Sun, 15 May 2011 18:34:27 -0700 Message-ID: <4DD07F21.3010808@windriver.com> Date: Mon, 16 May 2011 09:34:25 +0800 From: Robert Yang User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.17) Gecko/20110424 Lightning/1.0b2 Thunderbird/3.1.10 MIME-Version: 1.0 References: <8b4e9efcd34ac73babf6928cb2909ce88c4df69d.1305120861.git.liezhi.yang@windriver.com> <4DCACDDE.1090205@linux.intel.com> <4DCBFFA3.4090907@windriver.com> In-Reply-To: <4DCBFFA3.4090907@windriver.com> Cc: Darren Hart , poky@yoctoproject.org Subject: Re: [PATCH 2/2] Add a skeleton for init scripts X-BeenThere: poky@yoctoproject.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: Poky build system developer discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 May 2011 01:34:29 -0000 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Saul suggested that if can't config busybox to make the pidof support the "-x" option, then we would declare that it doesn't support the "shell script" as a daemon with this skeleton unless the pidof supports "-x"(Then the user should write the init script himself). This may be fine, there are only 3 shell scripts in core-image-minimal(In /bin/, /usr/bin/, /sbin/, /usr/sbin/). // Robert On 05/12/2011 11:41 PM, Robert Yang wrote: > Hi Darren, > > I find that the busybox's pidof can't get the pid of the "shell script" > (opposite to the binary) currently, it doesn't support the "-x" option, > so it seems that we should patch the busybox's pidof.c, is this acceptable > or not? > > Thank you very much > > Robert > > On 05/12/2011 01:56 AM, Darren Hart wrote: >> Hi Robert, >> >> Thank you for doing this. This is a great start. As a preface to my >> comments below, I am going to be incredibly particular about formatting >> and such as this skeleton will be used by literally thousands of recipes >> in the future and now is the time to ensure consistent formatting and to >> eliminate any issues we don't want to see propagated. >> >> >> On 05/11/2011 06:41 AM, Robert Yang wrote: >>> Add a skeleton for init scripts, the original structure is from >>> /etc/init.d/skeleton of Ubuntu 10.10, it is in sysvinit_2.87dsf, >>> so add the COPYRIGHT of sysvinit_2.87dsf, it is GPLv2. I have >>> modified the original skeleton a lot to make it as easy as possible, >>> just use posix shell command, and have tested it in core-image-minimal. >>> >>> * The skeleton implements the following items: >>> - start, stop, restart, status and force-reload. >>> And the force-reload is a alias of restart. >>> - not implements reload and try-restart, since only a few programs >>> have such functions, so just leave them as placeholders. >>> >>> * Use start-stop-daemon to start and stop the program, it is useful to >>> deal with the pidfile, check whether there is already a running >>> program or not, and print relevant messages. >>> >>> * The skeleton will run by default, and output the *simulation* message, >>> here is the testing result: >>> 1) #./skeleton start (test start) >>> Starting skeleton ... >>> >>> 2) #./skeleton start (test start the already running program) >>> Starting skeleton ... >>> /usr/sbin/skeleton already running. >>> 20426 >>> >>> It outputs two lines when start again, one line would be better, this >>> is because of the start-stop-daemon, but I'm not sure whether this is >>> worth. >>> >>> 3) #./skeleton status (test status when running) >>> skeleton is running. >> >> We should use a consistent string for the daemon. You use the full path >> in #2 above and only the basename here in #3. >> >>> >>> 4) #./skeleton stop (test stop) >>> Stopped skeleton (pid 20426). >> >> I prefer this format (with the pid listed on the same line) to that in >> #2. #3 should also list the pid. For example, my dovecot initscript reports: >> >> $ status dovecot >> dovecot start/running, process 18529 >> >> It our case something like the following seems adequate. >> >> $ ./skeleton status >> skeleton is running (18529) >> >>> >>> 5) #./skeleton stop (test stop again) >>> No skeleton found running; none killed. >> >> I'd prefer: >> >> skeleton is not running; none killed. >> >>> >>> 6) #./skeleton status (test status when stopped) >>> skeleton is not running. >>> >>> 7) #./skeleton start (test restart when running) >>> Starting skeleton ... >>> >>> #./skeleton restart >>> Stopped skeleton (pid 20444). >>> Starting skeleton ... >>> >>> 8) #./skeleton stop (test restart when stopped) >>> Stopped skeleton (pid 20444). >>> >>> #./skeleton restart >>> No skeleton found running; none killed. >> >> skeleton is not running; none killed. >> >>> Starting skeleton ... >>> >>> * Have used syslogd to test it in a real world(With both >>> core-image-minimal and core-image-sato) >>> >>> * TODO: >>> - Move the function status_of_proc to /etc/init.d/functions ? >> >> >> It's not a bad idea. If so, that has to be part of the base images, not >> the skeleton. >> >> >>> - Fix start-stop-daemon to get an one line output when start a >>> already runing program? >> >> >> I wouldn't bother. I don't see anything wrong with two lines. >> >> >>> >>> Signed-off-by: Robert Yang >>> --- >>> .../recipes-skeleton/service/service/COPYRIGHT | 19 ++ >>> .../recipes-skeleton/service/service/skeleton | 188 ++++++++++++++++++++ >>> .../recipes-skeleton/service/service_0.1.bb | 20 ++ >>> 3 files changed, 227 insertions(+), 0 deletions(-) >>> create mode 100644 meta-skeleton/recipes-skeleton/service/service/COPYRIGHT >>> create mode 100644 meta-skeleton/recipes-skeleton/service/service/skeleton >>> create mode 100644 meta-skeleton/recipes-skeleton/service/service_0.1.bb >>> >>> diff --git a/meta-skeleton/recipes-skeleton/service/service/COPYRIGHT >>> b/meta-skeleton/recipes-skeleton/service/service/COPYRIGHT >>> new file mode 100644 >>> index 0000000..36703d9 >>> --- /dev/null >>> +++ b/meta-skeleton/recipes-skeleton/service/service/COPYRIGHT >>> @@ -0,0 +1,19 @@ >>> +Sysvinit is Copyright (C) 1991-2004 Miquel van Smoorenburg >>> + >>> + This program is free software; you can redistribute it and/or modify >>> + it under the terms of the GNU General Public License as published by >>> + the Free Software Foundation; either version 2 of the License, or >>> + (at your option) any later version. >>> + >>> + This program is distributed in the hope that it will be useful, >>> + but WITHOUT ANY WARRANTY; without even the implied warranty of >>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >>> + GNU General Public License for more details. >>> + >>> + You should have received a copy of the GNU General Public License >>> + along with this program; if not, write to the Free Software >>> + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA >>> + >>> +On Debian GNU/Linux systems, the complete text of the GNU General >>> +Public License can be found in `/usr/share/common-licenses/GPL'. >> >> >> You don't need to keep this last bit of where to find it on Debian >> systems - we aren't a Debian system. >> >> >>> + >>> diff --git a/meta-skeleton/recipes-skeleton/service/service/skeleton >>> b/meta-skeleton/recipes-skeleton/service/service/skeleton >>> new file mode 100644 >>> index 0000000..58804e4 >>> --- /dev/null >>> +++ b/meta-skeleton/recipes-skeleton/service/service/skeleton >>> @@ -0,0 +1,188 @@ >>> +#! /bin/sh >>> +### BEGIN INIT INFO >>> +# Provides: skeleton >>> +# Required-Start: $local_fs >>> +# Should-Start: >>> +# Required-Stop: $local_fs >>> +# Should-Stop: >>> +# Default-Start: 2 3 4 5 >>> +# Default-Stop: 0 1 6 >>> +# Short-Description: Example initscript >>> +# Description: This file should be used to construct scripts to be >>> +# placed in /etc/init.d. >>> +### END INIT INFO >>> + >>> +# Common steps to convert this skeleton into a real init script >>> +# 1) cp skeleton >>> +# 2) Set DESC and NAME >>> +# 3) Check whether the daemon app is /usr/sbin/$NAME, if not, set it. >>> +# 4) Set DAEMON_ARGS if there is any >>> +# 5) Remove the useless code >>> + >>> +# PATH should only include /usr/* if it runs after the mountnfs.sh script >>> +PATH=/sbin:/usr/sbin:/bin:/usr/bin >> >> >> Paths like usr and etc need to be rewritten at install. I wonder if >> there might not be a better way to represent them in the skeleton. >> >> >>> + >>> +DESC="skeleton" >>> +NAME="skeleton" >>> +DAEMON=/usr/sbin/$NAME >>> +DAEMON_ARGS="" >>> +PIDFILE=/var/run/$NAME.pid >>> +SCRIPTNAME=/etc/init.d/$NAME >>> + >>> +#### Simulation begin >>> +# Simulate the script running, this is only used for printing the >>> +# information by default, it does nothing and should be removed from >>> +# the real init script >> >> >> It should be removed from the skeleton as well. I suggest writing a test >> script which calls "skeleton start; skeleton stop" etc. Especially since >> you're not testing the actual script by running this, and you duplicate >> code. So if you update it in only one spot, this testing will not catch it. >> >> >>> +fake_do_stop () { >>> + if [ -f $PIDFILE ]; then >>> + echo "Stopped $NAME (pid `cat $PIDFILE`)." >>> + rm -f $PIDFILE >>> + else >>> + echo "No $NAME found running; none killed." >>> + fi >>> +} >>> + >>> +if [ "$NAME" = "skeleton" ]; then >>> + case $1 in >>> + start) >>> + echo "Starting $DESC ..." >>> + if [ -f $PIDFILE ]; then >>> + echo "$DAEMON already running." >>> + cat $PIDFILE >>> + else >>> + echo $$>$PIDFILE >>> + fi >>> + ;; >>> + stop) >>> + fake_do_stop >>> + ;; >>> + status) >>> + if [ -f $PIDFILE ]; then >>> + echo "$NAME is running." >>> + else >>> + echo "$NAME is not running." >>> + fi >>> + ;; >>> + restart|force-reload) >>> + fake_do_stop >>> + echo "Starting $DESC ..." >>> + echo $$>$PIDFILE >>> + ;; >>> + *) >>> + echo "Usage: $SCRIPTNAME {start|stop|status|restart|force-reload}">&2 >>> + exit 3 >>> + ;; >>> + esac >>> +exit 0 >>> +fi >>> +#### Simulation end >>> + >>> +# Exit if the package is not installed >>> +[ -x "$DAEMON" ] || exit 0 >>> + >>> +# Read configuration variable file if it is present >>> +[ -r /etc/default/$NAME ]&& . /etc/default/$NAME >>> + >>> +# >>> +# Function that starts the daemon/service >>> +# >>> +do_start() >>> +{ >>> + start-stop-daemon -S --pidfile $PIDFILE --exec $DAEMON -- $DAEMON_ARGS >>> + # Add code here, if necessary, that waits for the process to be ready >>> + # to handle requests from services started subsequently which depend >>> + # on this one. As a last resort, sleep for some time. >>> +} >>> + >>> +# >>> +# Function that stops the daemon/service >>> +# >>> +do_stop() >>> +{ >>> + start-stop-daemon -K -v --pidfile $PIDFILE --name $NAME >>> + # Wait for children to finish too if this is a daemon that forks >>> + # and if the daemon is only ever run from this initscript. >>> + # If the above conditions are not satisfied then add some other code >>> + # that waits for the process to drop all resources that could be >>> + # needed by services started subsequently. A last resort is to >>> + # sleep for some time. >>> + # >>> + #start-stop-daemon -K --oknodo --exec $DAEMON >> >> >> What does this command do? >> >> >>> + >>> + # Many daemons don't delete their pidfiles when they exit. >>> + rm -f $PIDFILE >> >> >> Should STDERR be redirected to /dev/null in case the file does not exist? >> >> >>> +} >>> + >>> +# >>> +# Function that sends a SIGHUP to the daemon/service >>> +# >>> +#do_reload() { >>> + # >>> + # If the daemon can reload its configuration without >>> + # restarting (for example, when it is sent a SIGHUP), >>> + # then implement that here. >>> + # >>> + #start-stop-daemon -K --signal 1 --pidfile $PIDFILE --name $NAME >>> +#} >>> + >>> +# >>> +# Function that shows the daemon/service status >>> +# >>> +status_of_proc () { >>> + pidof $NAME>/dev/null 2>&1 >>> + case $? in >>> + 0) >>> + echo "$NAME is running." >>> + return 0 >>> + ;; >>> + 127) >>> + echo "Can't get the status of $NAME (no pidof found)" >>> + ;; >>> + *) >>> + echo "$NAME is not running.">&2 >>> + return 1 >>> + ;; >>> + esac >>> +} >>> + >>> +case "$1" in >>> + start) >>> + echo "Starting $DESC ..." >>> + # Don't need check the return value, start-stop-daemon would >>> + # print the failed reason automatically. >>> + do_start >>> + ;; >>> + stop) >>> + do_stop >>> + ;; >>> + status) >>> + status_of_proc&& exit 0 || exit $? >>> + ;; >>> + #reload|force-reload) >> >> >> We don't want to have force-reload in two locations, even commented out. >> Going to lead to confusion. >> >> >>> + # >>> + # If do_reload() is not implemented then leave this commented out >>> + # and leave 'force-reload' as an alias for 'restart'. >>> + # >>> + #echo "Reloading $DESC ..." >>> + #do_reload >>> + #;; >>> + restart|force-reload) >>> + # >>> + # If the "reload" option is implemented then remove the >>> + # 'force-reload' alias. >>> + # >>> + do_stop >>> + echo "Starting $DESC ..." >>> + do_start >>> + ;; >>> + #try-restart) >>> + # If do_try_restart() is not implemented then leave this commented out >>> + #echo "Trying to restart $DESC ..." >> >> >> This needs a description of how it should be implemented. i.e. how is >> this different from restart? >> >> >>> + #do_try_restart() >>> + #;; >>> + *) >>> + echo "Usage: $SCRIPTNAME {start|stop|status|restart|force-reload}">&2 >>> + exit 3 >>> + ;; >>> +esac >>> + >>> diff --git a/meta-skeleton/recipes-skeleton/service/service_0.1.bb >>> b/meta-skeleton/recipes-skeleton/service/service_0.1.bb >>> new file mode 100644 >>> index 0000000..88a7a59 >>> --- /dev/null >>> +++ b/meta-skeleton/recipes-skeleton/service/service_0.1.bb >>> @@ -0,0 +1,20 @@ >>> +DESCRIPTION = "The canonical example of init scripts" >>> +SECTION = "base" >>> +LICENSE = "GPLv2" >>> +LIC_FILES_CHKSUM = >>> "file://${WORKDIR}/COPYRIGHT;md5=3900421dc55b9e70428bc522557a66d4" >>> +RDEPENDS_${PN} = "initscripts" >>> +PR = "r0" >>> + >>> +SRC_URI = "file://skeleton \ >>> + file://COPYRIGHT \ >>> + " >>> + >>> +CONFFILES_${PN} += "${sysconfdir}/init.d/skeleton" >>> + >>> +PACKAGE_ARCH = "all" >>> + >>> +do_install () { >>> + install -d ${D}/${sysconfdir}/init.d >>> + install -m 0755 ${WORKDIR}/skeleton ${D}/${sysconfdir}/init.d/ >>> +} >>> + >> >> This installs a file that uses directories like /usr which should be >> replaced with the appropriate setting. For example, in mediatomb (and >> others) we do this like this: >> >> cat ${WORKDIR}/config.xml | \ >> sed -e 's,/etc,${sysconfdir},g' \ >> -e 's,/usr/sbin,${sbindir},g' \ >> -e 's,/var,${localstatedir},g' \ >> -e 's,/usr/bin,${bindir},g' \ >> -e 's,/usr,${prefix},g'> ${D}${sysconfdir}/mediatomb/config.xml >>