Hello, This patch fixes some NetBSD problems with xencommons init script: * Added cleanup of xenstore database on init. * Prevent the restart of xenstore. * Added setting Domain-0 name on boot. Regards, Roger. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, 2011-07-15 at 08:38 +0100, Roger Pau Monné wrote:> # HG changeset patch > # User royger > # Date 1310722656 -7200 > # Node ID d3b9e6f9536c84d595276f8c12735736d5593e13 > # Parent 8c7e3cfc33b6151fbd4fc6e1aeae132a9b1ccfa5 > xencommons NetBSD init script:In general it is useful if the first line of the commit message standsalone as a summary, since hg will often display it that way (e.g. in hg log, on hg web etc).> Clean up of xenstore database on init. > Prevent xenstore from restarting. > Set Domain-0 name on init. > > Signed-off-by: Roger Pau Monne <roger.pau@entel.upc.edu> > > diff -r 8c7e3cfc33b6 -r d3b9e6f9536c tools/hotplug/NetBSD/rc.d/xencommons > --- a/tools/hotplug/NetBSD/rc.d/xencommons Fri Jul 15 11:29:21 2011 +0200 > +++ b/tools/hotplug/NetBSD/rc.d/xencommons Fri Jul 15 11:37:36 2011 +0200 > @@ -23,6 +23,9 @@ > XENSTORED_PIDFILE="/var/run/xenstored.pid" > XENCONSOLED_PIDFILE="/var/run/xenconsoled.pid" > XENBACKENDD_PIDFILE="/var/run/xenbackendd.pid" > +#XENBACKENDD_DEBUG=1 > +#XENCONSOLED_TRACE=1 > +#XENSTORED_TRACE="/var/log/xen/xenconsole-trace.log" > > xen_precmd() > { > @@ -33,15 +36,24 @@ > > xen_startcmd() > { > - printf "Starting xenservices: xenstored, xenconsoled, xenbackendd.\n" > - XENSTORED_ARGS=" --pid-file ${XENSTORED_PIDFILE}" > - if [ -n "${XENSTORED_TRACE}" ]; then > - XENSTORED_ARGS="${XENSTORED_ARGS} -T /var/log/xen/xenstored-trace.log" > + xenstored_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${SBINDIR}/xenstored) > + if test -z $xenstored_pid; then > + printf "Cleaning xenstore database.\n" > + if [ -n "${XENSTORED_ROOTDIR}" ]; then > + XENSTORED_ROOTDIR="/var/lib/xenstored" > + fi > + rm -f ${XENSTORED_ROOTDIR}/tdb* &>/dev/null > + printf "Starting xenservices: xenstored, xenconsoled, xenbackendd.\n" > + XENSTORED_ARGS=" --pid-file ${XENSTORED_PIDFILE}" > + if [ -n "${XENSTORED_TRACE}" ]; then > + XENSTORED_ARGS="${XENSTORED_ARGS} -T /var/log/xen/xenstored-trace.log" > + fi > + ${SBINDIR}/xenstored ${XENSTORED_ARGS} > + sleep 5sleep 5 seems a bit drastic. Perhaps steal the checking loop from the Linux xencommons?> > + else > + printf "Starting xenservices: xenconsoled, xenbackendd.\n" > fi > > - ${SBINDIR}/xenstored ${XENSTORED_ARGS} > - sleep 5 > - > XENCONSOLED_ARGS="" > if [ -n "${XENCONSOLED_TRACE}" ]; then > XENCONSOLED_ARGS="${XENCONSOLED_ARGS} --log=${XENCONSOLED_TRACE}" > @@ -55,24 +67,24 @@ > fi > > ${SBINDIR}/xenbackendd ${XENBACKENDD_ARGS} > + > + printf "Setting domain 0 name.\n" > + ${BINDIR}/xenstore-write "/local/domain/0/name" "Domain-0" > } > > xen_stop() > { > pids="" > - printf "Stopping xencommons" > + printf "Stopping xencommons.\n" > + printf "WARNING: Not stopping xenstored, as it cannot be restarted.\n" > > rc_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd) > pids="$pids $rc_pid" > rc_pid=$(check_pidfile ${XENCONSOLED_PIDFILE} ${SBINDIR}/xenconsoled) > pids="$pids $rc_pid" > - rc_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${SBINDIR}/xenstored) > - pids="$pids $rc_pid" > > kill -${sig_stop:-TERM} $pids > wait_for_pids $pids > - > - printf ".\n" > } > > xen_status()_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 07/15/11 10:01, Ian Campbell wrote:> On Fri, 2011-07-15 at 08:38 +0100, Roger Pau Monné wrote: >> # HG changeset patch >> # User royger >> # Date 1310722656 -7200 >> # Node ID d3b9e6f9536c84d595276f8c12735736d5593e13 >> # Parent 8c7e3cfc33b6151fbd4fc6e1aeae132a9b1ccfa5 >> xencommons NetBSD init script: > > In general it is useful if the first line of the commit message > standsalone as a summary, since hg will often display it that way (e.g. > in hg log, on hg web etc). > >> Clean up of xenstore database on init. >> Prevent xenstore from restarting. >> Set Domain-0 name on init. >> >> Signed-off-by: Roger Pau Monne<roger.pau@entel.upc.edu> >> >> diff -r 8c7e3cfc33b6 -r d3b9e6f9536c tools/hotplug/NetBSD/rc.d/xencommons >> --- a/tools/hotplug/NetBSD/rc.d/xencommons Fri Jul 15 11:29:21 2011 +0200 >> +++ b/tools/hotplug/NetBSD/rc.d/xencommons Fri Jul 15 11:37:36 2011 +0200 >> @@ -23,6 +23,9 @@ >> XENSTORED_PIDFILE="/var/run/xenstored.pid" >> XENCONSOLED_PIDFILE="/var/run/xenconsoled.pid" >> XENBACKENDD_PIDFILE="/var/run/xenbackendd.pid" >> +#XENBACKENDD_DEBUG=1 >> +#XENCONSOLED_TRACE=1 >> +#XENSTORED_TRACE="/var/log/xen/xenconsole-trace.log" >> >> xen_precmd() >> { >> @@ -33,15 +36,24 @@ >> >> xen_startcmd() >> { >> - printf "Starting xenservices: xenstored, xenconsoled, xenbackendd.\n" >> - XENSTORED_ARGS=" --pid-file ${XENSTORED_PIDFILE}" >> - if [ -n "${XENSTORED_TRACE}" ]; then >> - XENSTORED_ARGS="${XENSTORED_ARGS} -T /var/log/xen/xenstored-trace.log" >> + xenstored_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${SBINDIR}/xenstored) >> + if test -z $xenstored_pid; then >> + printf "Cleaning xenstore database.\n" >> + if [ -n "${XENSTORED_ROOTDIR}" ]; then >> + XENSTORED_ROOTDIR="/var/lib/xenstored" >> + fi >> + rm -f ${XENSTORED_ROOTDIR}/tdb*&>/dev/null >> + printf "Starting xenservices: xenstored, xenconsoled, xenbackendd.\n" >> + XENSTORED_ARGS=" --pid-file ${XENSTORED_PIDFILE}" >> + if [ -n "${XENSTORED_TRACE}" ]; then >> + XENSTORED_ARGS="${XENSTORED_ARGS} -T /var/log/xen/xenstored-trace.log" >> + fi >> + ${SBINDIR}/xenstored ${XENSTORED_ARGS} >> + sleep 5 > > sleep 5 seems a bit drastic. Perhaps steal the checking loop from the Linux xencommons?I think, the sleep 5 can be reduced to sleep 2 since xenstored launches much faster when there is no xenstored database to check. The sleep is there to delay the next start up script which starts xend. And starting xend fails when xenstored isn''t ready before. Christoph>> >> + else >> + printf "Starting xenservices: xenconsoled, xenbackendd.\n" >> fi >> >> - ${SBINDIR}/xenstored ${XENSTORED_ARGS} >> - sleep 5 >> - >> XENCONSOLED_ARGS="" >> if [ -n "${XENCONSOLED_TRACE}" ]; then >> XENCONSOLED_ARGS="${XENCONSOLED_ARGS} --log=${XENCONSOLED_TRACE}" >> @@ -55,24 +67,24 @@ >> fi >> >> ${SBINDIR}/xenbackendd ${XENBACKENDD_ARGS} >> + >> + printf "Setting domain 0 name.\n" >> + ${BINDIR}/xenstore-write "/local/domain/0/name" "Domain-0" >> } >> >> xen_stop() >> { >> pids="" >> - printf "Stopping xencommons" >> + printf "Stopping xencommons.\n" >> + printf "WARNING: Not stopping xenstored, as it cannot be restarted.\n" >> >> rc_pid=$(check_pidfile ${XENBACKENDD_PIDFILE} ${SBINDIR}/xenbackendd) >> pids="$pids $rc_pid" >> rc_pid=$(check_pidfile ${XENCONSOLED_PIDFILE} ${SBINDIR}/xenconsoled) >> pids="$pids $rc_pid" >> - rc_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${SBINDIR}/xenstored) >> - pids="$pids $rc_pid" >> >> kill -${sig_stop:-TERM} $pids >> wait_for_pids $pids >> - >> - printf ".\n" >> } >> >> xen_status()-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, 2011-07-15 at 09:59 +0100, Christoph Egger wrote:> On 07/15/11 10:01, Ian Campbell wrote: > > On Fri, 2011-07-15 at 08:38 +0100, Roger Pau Monné wrote: > >> # HG changeset patch > >> # User royger > >> # Date 1310722656 -7200 > >> # Node ID d3b9e6f9536c84d595276f8c12735736d5593e13 > >> # Parent 8c7e3cfc33b6151fbd4fc6e1aeae132a9b1ccfa5 > >> xencommons NetBSD init script: > > > > In general it is useful if the first line of the commit message > > standsalone as a summary, since hg will often display it that way (e.g. > > in hg log, on hg web etc). > > > >> Clean up of xenstore database on init. > >> Prevent xenstore from restarting. > >> Set Domain-0 name on init. > >> > >> Signed-off-by: Roger Pau Monne<roger.pau@entel.upc.edu> > >> > >> diff -r 8c7e3cfc33b6 -r d3b9e6f9536c tools/hotplug/NetBSD/rc.d/xencommons > >> --- a/tools/hotplug/NetBSD/rc.d/xencommons Fri Jul 15 11:29:21 2011 +0200 > >> +++ b/tools/hotplug/NetBSD/rc.d/xencommons Fri Jul 15 11:37:36 2011 +0200 > >> @@ -23,6 +23,9 @@ > >> XENSTORED_PIDFILE="/var/run/xenstored.pid" > >> XENCONSOLED_PIDFILE="/var/run/xenconsoled.pid" > >> XENBACKENDD_PIDFILE="/var/run/xenbackendd.pid" > >> +#XENBACKENDD_DEBUG=1 > >> +#XENCONSOLED_TRACE=1 > >> +#XENSTORED_TRACE="/var/log/xen/xenconsole-trace.log" > >> > >> xen_precmd() > >> { > >> @@ -33,15 +36,24 @@ > >> > >> xen_startcmd() > >> { > >> - printf "Starting xenservices: xenstored, xenconsoled, xenbackendd.\n" > >> - XENSTORED_ARGS=" --pid-file ${XENSTORED_PIDFILE}" > >> - if [ -n "${XENSTORED_TRACE}" ]; then > >> - XENSTORED_ARGS="${XENSTORED_ARGS} -T /var/log/xen/xenstored-trace.log" > >> + xenstored_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${SBINDIR}/xenstored) > >> + if test -z $xenstored_pid; then > >> + printf "Cleaning xenstore database.\n" > >> + if [ -n "${XENSTORED_ROOTDIR}" ]; then > >> + XENSTORED_ROOTDIR="/var/lib/xenstored" > >> + fi > >> + rm -f ${XENSTORED_ROOTDIR}/tdb*&>/dev/null > >> + printf "Starting xenservices: xenstored, xenconsoled, xenbackendd.\n" > >> + XENSTORED_ARGS=" --pid-file ${XENSTORED_PIDFILE}" > >> + if [ -n "${XENSTORED_TRACE}" ]; then > >> + XENSTORED_ARGS="${XENSTORED_ARGS} -T /var/log/xen/xenstored-trace.log" > >> + fi > >> + ${SBINDIR}/xenstored ${XENSTORED_ARGS} > >> + sleep 5 > > > > sleep 5 seems a bit drastic. Perhaps steal the checking loop from the Linux xencommons? > > I think, the sleep 5 can be reduced to sleep 2 since xenstored launches > much faster when there is no xenstored database to check. > The sleep is there to delay the next start up script which starts xend. > And starting xend fails when xenstored isn''t ready before.Why hardcode a "2" (which can easily be either too short or too long depending on circumstances) when you could cut-and-paste something better (from tools/hotplug/Linux/init.d/xencommons): local time=0 local timeout=30 ... # Wait for xenstored to actually come up, timing out after 30 seconds while [ $time -lt $timeout ] && ! `xenstore-read -s /local >/dev/null 2>&1` ; do echo -n . time=$(($time+1)) sleep 1 done Is $(( ... )) a bashism? `expr $time + 1` instead? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Thanks for the comments, here is the new patch, changed the commit message and used the method found on Linux script to wait for xenstore to come up. Regards, Roger. 2011/7/15 Ian Campbell <Ian.Campbell@eu.citrix.com>:> On Fri, 2011-07-15 at 09:59 +0100, Christoph Egger wrote: >> On 07/15/11 10:01, Ian Campbell wrote: >> > On Fri, 2011-07-15 at 08:38 +0100, Roger Pau Monné wrote: >> >> # HG changeset patch >> >> # User royger >> >> # Date 1310722656 -7200 >> >> # Node ID d3b9e6f9536c84d595276f8c12735736d5593e13 >> >> # Parent 8c7e3cfc33b6151fbd4fc6e1aeae132a9b1ccfa5 >> >> xencommons NetBSD init script: >> > >> > In general it is useful if the first line of the commit message >> > standsalone as a summary, since hg will often display it that way (e.g. >> > in hg log, on hg web etc). >> > >> >> Clean up of xenstore database on init. >> >> Prevent xenstore from restarting. >> >> Set Domain-0 name on init. >> >> >> >> Signed-off-by: Roger Pau Monne<roger.pau@entel.upc.edu> >> >> >> >> diff -r 8c7e3cfc33b6 -r d3b9e6f9536c tools/hotplug/NetBSD/rc.d/xencommons >> >> --- a/tools/hotplug/NetBSD/rc.d/xencommons Fri Jul 15 11:29:21 2011 +0200 >> >> +++ b/tools/hotplug/NetBSD/rc.d/xencommons Fri Jul 15 11:37:36 2011 +0200 >> >> @@ -23,6 +23,9 @@ >> >> XENSTORED_PIDFILE="/var/run/xenstored.pid" >> >> XENCONSOLED_PIDFILE="/var/run/xenconsoled.pid" >> >> XENBACKENDD_PIDFILE="/var/run/xenbackendd.pid" >> >> +#XENBACKENDD_DEBUG=1 >> >> +#XENCONSOLED_TRACE=1 >> >> +#XENSTORED_TRACE="/var/log/xen/xenconsole-trace.log" >> >> >> >> xen_precmd() >> >> { >> >> @@ -33,15 +36,24 @@ >> >> >> >> xen_startcmd() >> >> { >> >> - printf "Starting xenservices: xenstored, xenconsoled, xenbackendd.\n" >> >> - XENSTORED_ARGS=" --pid-file ${XENSTORED_PIDFILE}" >> >> - if [ -n "${XENSTORED_TRACE}" ]; then >> >> - XENSTORED_ARGS="${XENSTORED_ARGS} -T /var/log/xen/xenstored-trace.log" >> >> + xenstored_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${SBINDIR}/xenstored) >> >> + if test -z $xenstored_pid; then >> >> + printf "Cleaning xenstore database.\n" >> >> + if [ -n "${XENSTORED_ROOTDIR}" ]; then >> >> + XENSTORED_ROOTDIR="/var/lib/xenstored" >> >> + fi >> >> + rm -f ${XENSTORED_ROOTDIR}/tdb*&>/dev/null >> >> + printf "Starting xenservices: xenstored, xenconsoled, xenbackendd.\n" >> >> + XENSTORED_ARGS=" --pid-file ${XENSTORED_PIDFILE}" >> >> + if [ -n "${XENSTORED_TRACE}" ]; then >> >> + XENSTORED_ARGS="${XENSTORED_ARGS} -T /var/log/xen/xenstored-trace.log" >> >> + fi >> >> + ${SBINDIR}/xenstored ${XENSTORED_ARGS} >> >> + sleep 5 >> > >> > sleep 5 seems a bit drastic. Perhaps steal the checking loop from the Linux xencommons? >> >> I think, the sleep 5 can be reduced to sleep 2 since xenstored launches >> much faster when there is no xenstored database to check. >> The sleep is there to delay the next start up script which starts xend. >> And starting xend fails when xenstored isn''t ready before. > > Why hardcode a "2" (which can easily be either too short or too long > depending on circumstances) when you could cut-and-paste something > better (from tools/hotplug/Linux/init.d/xencommons): > local time=0 > local timeout=30 > ... > # Wait for xenstored to actually come up, timing out after 30 seconds > while [ $time -lt $timeout ] && ! `xenstore-read -s /local >/dev/null 2>&1` ; do > echo -n . > time=$(($time+1)) > sleep 1 > done > > Is $(( ... )) a bashism? `expr $time + 1` instead? > > Ian. > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, 2011-07-15 at 10:18 +0100, Roger Pau Monné wrote:> Thanks for the comments, here is the new patch, changed the commit > message and used the method found on Linux script to wait for xenstore > to come up.Looks good to me in the general sense, although I can''t comment on the NetBSD specifics. Acked-by: Ian Campbell <ian.campbell@citrix.com> _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Just a minor fix, &>/dev/null used with rm puts the process in the background, so xenstore might be started before the database was cleaned. Regards, Roger. 2011/7/15 Ian Campbell <Ian.Campbell@eu.citrix.com>:> On Fri, 2011-07-15 at 10:18 +0100, Roger Pau Monné wrote: >> Thanks for the comments, here is the new patch, changed the commit >> message and used the method found on Linux script to wait for xenstore >> to come up. > > Looks good to me in the general sense, although I can''t comment on the > NetBSD specifics. > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, 2011-07-15 at 10:55 +0100, Roger Pau Monné wrote:> Just a minor fix, &>/dev/null used with rm puts the process in the > background, so xenstore might be started before the database was > cleaned.Oh right, because &>/dev/null is a bash-ism.> > Regards, Roger. > > 2011/7/15 Ian Campbell <Ian.Campbell@eu.citrix.com>: > > On Fri, 2011-07-15 at 10:18 +0100, Roger Pau Monné wrote: > >> Thanks for the comments, here is the new patch, changed the commit > >> message and used the method found on Linux script to wait for xenstore > >> to come up. > > > > Looks good to me in the general sense, although I can''t comment on the > > NetBSD specifics. > > > > Acked-by: Ian Campbell <ian.campbell@citrix.com> > > > > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Another one, used -n instead of -z when testing the presence of XENSTORED_ROOTDIR. Sorry for this mess, I''m not really used to make shell init scripts. Regards, Roger. 2011/7/15 Ian Campbell <Ian.Campbell@eu.citrix.com>:> On Fri, 2011-07-15 at 10:55 +0100, Roger Pau Monné wrote: >> Just a minor fix, &>/dev/null used with rm puts the process in the >> background, so xenstore might be started before the database was >> cleaned. > > Oh right, because &>/dev/null is a bash-ism. > >> >> Regards, Roger. >> >> 2011/7/15 Ian Campbell <Ian.Campbell@eu.citrix.com>: >> > On Fri, 2011-07-15 at 10:18 +0100, Roger Pau Monné wrote: >> >> Thanks for the comments, here is the new patch, changed the commit >> >> message and used the method found on Linux script to wait for xenstore >> >> to come up. >> > >> > Looks good to me in the general sense, although I can''t comment on the >> > NetBSD specifics. >> > >> > Acked-by: Ian Campbell <ian.campbell@citrix.com> >> > >> > >> > > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 07/15/11 13:27, Roger Pau Monné wrote:> Another one, used -n instead of -z when testing the presence of > XENSTORED_ROOTDIR. Sorry for this mess, I''m not really used to make > shell init scripts.This version is good. Acked-by: Christoph Egger <Christoph.Egger@amd.com>> > Regards, Roger. > > 2011/7/15 Ian Campbell<Ian.Campbell@eu.citrix.com>: >> On Fri, 2011-07-15 at 10:55 +0100, Roger Pau Monné wrote: >>> Just a minor fix,&>/dev/null used with rm puts the process in the >>> background, so xenstore might be started before the database was >>> cleaned. >> >> Oh right, because&>/dev/null is a bash-ism. >> >>> >>> Regards, Roger. >>> >>> 2011/7/15 Ian Campbell<Ian.Campbell@eu.citrix.com>: >>>> On Fri, 2011-07-15 at 10:18 +0100, Roger Pau Monné wrote: >>>>> Thanks for the comments, here is the new patch, changed the commit >>>>> message and used the method found on Linux script to wait for xenstore >>>>> to come up. >>>> >>>> Looks good to me in the general sense, although I can''t comment on the >>>> NetBSD specifics. >>>> >>>> Acked-by: Ian Campbell<ian.campbell@citrix.com>-- ---to satisfy European Law for business letters: Advanced Micro Devices GmbH Einsteinring 24, 85689 Dornach b. Muenchen Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen Registergericht Muenchen, HRB Nr. 43632 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, Jul 15, Roger Pau Monné wrote:> Another one, used -n instead of -z when testing the presence of > XENSTORED_ROOTDIR. Sorry for this mess, I''m not really used to make > shell init scripts.+ xenstored_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${SBINDIR}/xenstored) + if test -z $xenstored_pid; then Doesnt this expand to ''if test -z ; then'' if check_pidfile returns nothing, causing a shell syntax error? Better do ''if test -z "$variable" ; then''. Olaf _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, 2011-07-15 at 12:44 +0100, Olaf Hering wrote:> On Fri, Jul 15, Roger Pau Monné wrote: > > > Another one, used -n instead of -z when testing the presence of > > XENSTORED_ROOTDIR. Sorry for this mess, I''m not really used to make > > shell init scripts. > > + xenstored_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${SBINDIR}/xenstored) > + if test -z $xenstored_pid; then > > Doesnt this expand to ''if test -z ; then'' if check_pidfile returns > nothing, causing a shell syntax error?Seems not: $ dash -c ''if test -z ; then echo foo ; fi'' foo $ posh -c ''if test -z ; then echo foo ; fi'' foo (maybe NetBSD sh(1) or test(1) differs though)> Better do ''if test -z "$variable" ; then''.That''s still good advice IMHO. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
It works fine on NetBSD too, but just to be on the safe side I''ve changed it. Thanks. 2011/7/15 Ian Campbell <Ian.Campbell@eu.citrix.com>:> On Fri, 2011-07-15 at 12:44 +0100, Olaf Hering wrote: >> On Fri, Jul 15, Roger Pau Monné wrote: >> >> > Another one, used -n instead of -z when testing the presence of >> > XENSTORED_ROOTDIR. Sorry for this mess, I''m not really used to make >> > shell init scripts. >> >> + xenstored_pid=$(check_pidfile ${XENSTORED_PIDFILE} ${SBINDIR}/xenstored) >> + if test -z $xenstored_pid; then >> >> Doesnt this expand to ''if test -z ; then'' if check_pidfile returns >> nothing, causing a shell syntax error? > > Seems not: > $ dash -c ''if test -z ; then echo foo ; fi'' > foo > $ posh -c ''if test -z ; then echo foo ; fi'' > foo > > (maybe NetBSD sh(1) or test(1) differs though) > >> Better do ''if test -z "$variable" ; then''. > > That''s still good advice IMHO. > > Ian. > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Roger Pau Monné writes ("Re: [Xen-devel] [PATCH] Fix xencommons for
NetBSD"):> It works fine on NetBSD too, but just to be on the safe side I''ve
> changed it. Thanks.
Thanks, I have committed this final version of the patch.
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel