[Pkg-xen-devel] [PATCH 08/13] xen init script: rewrite xenstored start logic
Ian Jackson
ijackson at chiark.greenend.org.uk
Tue Feb 12 17:21:13 GMT 2019
Hans van Kranenburg writes ("[PATCH 08/13] xen init script: rewrite xenstored start logic"):
> -XENSTORED="$ROOT"/bin/xenstored
> +# In /etc/default/xen, the user can set XENSTORED, which has to be either
> +# 'xenstored' or 'oxenstored'. In here, we add the version specific path.
> +if [ -n "$XENSTORED" ]; then
> + USER_XENSTORED="$ROOT"/bin/$XENSTORED
IMO it would be better to do this only if XENSTORED does not start
with /. So maybe
case "$XENSTORED" in
/*) USER_XENSTORED="$XENSTORED" ;;
?*) USER_XENSTORED="$ROOT"/bin/$XENSTORED ;;
'') USER_XENSTORED="" ;;
esac
?
> + # First, we check if any of xenstored or oxenstored is already running. If
> + # so, we abort and leave it alone, even if the user has switched the value
> + # in /etc/default/xen from one to another.
> + for try_xenstored in "$OXENSTORED" "$CXENSTORED"; do
> + if [ -x $try_xenstored ]; then
> + start-stop-daemon --start --quiet --pidfile "$XENSTORED_PIDFILE" \
> + --exec "$try_xenstored" --test > /dev/null
> + if [ $? -eq 1 ]; then
> + return 1
This exit status handling is confusing. AFAICT from the manpage for
start-stop-daemon, exit status 1 means "--oknodo was not specified and
nothing was done". So I think that would occur when --start does
nothing because the daemon is already running. It might be worth
adding a comment.
> + if [ -n "$USER_XENSTORED" ]; then
> + if [ ! -x "$USER_XENSTORED" ]; then
> + log_failure_msg "Failed to start $USER_XENSTORED: no such executable."
> + return 2
> + else
> + start-stop-daemon --start --quiet \
> + --pidfile "$XENSTORED_PIDFILE" --exec "$USER_XENSTORED" -- \
> + $XENSTORED_ARGS --pid-file="$XENSTORED_PIDFILE"
> + if [ $? -ne 0 ]; then
> + return 2
> + fi
> + fi
> + else
> + for try_xenstored in "$OXENSTORED" "$CXENSTORED"; do
> + if [ -x $try_xenstored ]; then
> + start-stop-daemon --start --quiet \
> + --pidfile "$XENSTORED_PIDFILE" --exec "$try_xenstored" -- \
> + $XENSTORED_ARGS --pid-file="$XENSTORED_PIDFILE"
I really don't like the way all of this is open-coded. There are two
almost-identical runes here. How about this:
case "$XENSTORED" in
/*) USER_XENSTORED="$XENSTORED" ; try_xenstoreds=USER_XENSTORED ;;
?*) USER_XENSTORED="$ROOT"/bin/$XENSTORED ; try_xenstoreds=USER_XENSTORED ;;
'') USER_XENSTORED="" ; try_xenstoreds='OXENSTORED CXENSTORED' ;;
esac
and then
for try_xenstored_var in $try_xenstoreds; do
eval "try_xenstored=\$$try_xenstored_var"
...
Thanks,
Ian.
--
Ian Jackson <ijackson at chiark.greenend.org.uk> These opinions are my own.
If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.
More information about the Pkg-xen-devel
mailing list