[ovs-dev] [PATCH 2/2] xenserver: Factor redundancy out of /etc/init.d/openvswitch.

Justin Pettit jpettit at nicira.com
Mon Apr 26 23:29:59 UTC 2010


On Apr 26, 2010, at 2:18 PM, Ben Pfaff wrote:

> +    elif test -n "$strace_log"; then
> +        local mode=strace
> +        set -- $nice strace -o "$strace_log" $strace_opt "$@"

I think you want to blow away this "$nice", since the niceness gets set later.

> +    elif test -n "$valgrind_log"; then
> +        local mode=valgrind
> +        set -- $nice valgrind --log-file="$valgrind_log" $valgrind_opt "$@"

Here, too.

>     else
> -        action "Starting ovsdb-server" nice -n "$OVSDB_SERVER_PRIORITY" "$ovsdb_server" "$OVSDB_SERVER_DB" --pidfile="$OVSDB_SERVER_PIDFILE" --detach $monitor_opt --no-chdir -vANY:CONSOLE:EMER $syslog_opt $logfile_level_opt $logfile_file_opt $leak_opt $remotes $ssl_opts
> +        local mode=production
> +        eval local pidfile=\$${DAEMON}_PIDFILE
> +        install -d -m 755 -o root -g root `dirname $VSWITCHD_PIDFILE`

Did you want that to be "$pidfile" instead of "$VSWITCHD_PIDFILE"?

> +        set -- $nice "$@" --pidfile="$pidfile" --detach $monitor_opt --no-chdir

> "$VSWITCHD_RUN_DIR"

Did you want to use "$run_dir" here?  Also, there's another "$nice".

> +    # Configure niceness.
> +    eval local priority=\$${DAEMON}_PRIORITY
> +    if test -n "$priority"; then
> +        set -- nice -n $priority "$@"
>     fi
> -    cd "$VSWITCHD_RUN_DIR"
> 
> -    install -d -m 755 -o root -g root `dirname $VSWITCHD_LOGFILE`
> -    if [ -n "$VSWITCHD_FILE_LOGLEVEL" ]; then
> -        logfile_level_opt="-vANY:FILE:${VSWITCHD_FILE_LOGLEVEL}"
> -        logfile_file_opt="--log-file=$VSWITCHD_LOGFILE"
> +    if test $mode = production; then
> +        action "Starting `basename $BINARY`" "$@"
> +    else
> +        # Start in background and force a "success" message
> +        action "Starting ovs-vswitchd with $mode debugging" true

Did you want to print "`basename $BINARY`" instead of "ovs-vswitchd"?

> function start_xenserverd {
> @@ -306,38 +259,16 @@ function start_xenserverd {
> 
>     install -d -m 755 -o root -g root `dirname $XENSERVERD_PIDFILE`
>     action "Starting ovs-xenserverd" "$xenserverd" --no-chdir --pidfile=$XENSERVERD_PIDFILE --detach $monitor_opt -vANY:CONSOLE:EMER
> -    fi
> -}

xenserverd isn't using this fancy new start_daemon() stuff.  Is that on purpose?

--Justin






More information about the dev mailing list