[ovs-dev] [rhel --user v2 5/7] ovs-lib: add directory_check()

[Flavio Leitner]

On Fri, Nov 20, 2015 at 03:33:18AM -0800, Andy Zhou wrote:
> Rafactor common directory existence check and ownership check into
> a common function. Move daemon's default directory to $RUNDIR, since
> the process may not able to write core file to "/" anymore after the
> user change.
> 
> Signed-off-by: Andy Zhou <azhou at ovn.org>
> 
> ---
> v1->v2:  * Drop using 'stat -c"
>          * ADD $OVS_GROUP != root in addition to $OVS_USER != root check
> ---
>  utilities/ovs-lib.in | 37 ++++++++++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> index ad223c0..ad9c9f4 100644
> --- a/utilities/ovs-lib.in
> +++ b/utilities/ovs-lib.in
> @@ -70,8 +70,6 @@ ovs_ctl () {
>  
>  VERSION='@VERSION@'
>  
> -DAEMON_CWD=/
> -
>  LC_ALL=C; export LC_ALL
>  
>  ## ------------- ##
> @@ -154,6 +152,23 @@ pid_comm_check () {
>      [ "$1" = "`cat /proc/$2/comm`" ]
>  }
>  
> +# Make sure the directory '$1' exits. If not, crate it. If yes, make sure
> +# its group ownership agrees with $OVS_GROUP. If not, chown on all files
> +# within it.  We don't enforce $OVS_USER to allow for multiple users that
> +# shares $OVS_GROUP.
> +directory_check() {
> +    dir=$1
> +
> +    if test -d "$dir"; then
> +        # Change the ownership of the top level directory and the first
> +        # level files below it.
> +       chown "$OVS_USER":"$OVS_GROUP" "$dir"
> +       find "$dir" -maxdepth 1 -type f -exec chown "$OVS_USER":"$OVS_GROUP" {} \;
> +    else
> +        install -d -m 775 -o "$OVS_USER" -g "$OVS_GROUP" "$dir"
> +    fi
> +}
> +
>  start_daemon () {
>      priority=$1
>      wrapper=$2
> @@ -161,20 +176,24 @@ start_daemon () {
>      daemon=$1
>      strace=""
>  
> -    # drop core files in a sensible place
> -    test -d "$DAEMON_CWD" || install -d -m 755 -o "$OVS_USER" -g "$OVS_GROUP" "$DAEMON_CWD"
> -    set "$@" --no-chdir
> -    cd "$DAEMON_CWD"
> -
>      # log file
> -    test -d "$logdir" || install -d -m 755 -o "$OVS_USER" -g "$OVS_GROUP" "$logdir"
> +    directory_check "$logdir"
>      set "$@" --log-file="$logdir/$daemon.log"
>  
>      # pidfile and monitoring
> -    test -d "$rundir" || install -d -m 755 -o "$OVS_USER" -g "$OVS_GROUP" "$rundir"
> +    directory_check "$rundir"
>      set "$@" --pidfile="$rundir/$daemon.pid"
>      set "$@" --detach --monitor
>  
> +    # drop core files in a sensible place
> +    cd "$rundir"
> +    set "$@" --no-chdir

This depends on many things.  One is that systemd-coredump(8) handles
core dump properly.  Another is that core(5) which might point to
something else different.

The systemd also provides WorkingDirectory= to set specific workdir,
but we can't use that if the initialization enforces something else.

Anyway, this patch isn't changing anything other the workdir
from / to $rundir, which makes more sense.

> +
> +    # add --user for non root user
> +    if test "$OVS_USER" != "root" || test "$OVS_GROUP" != "root"; then
> +        set "$@" --user="$OVS_USER":"$OVS_GROUP"
> +    fi
> +
>      # wrapper
>      case $wrapper in
>          valgrind)

Acked-by: Flavio Leitner <fbl at sysclose.org>



> -- 
> 1.8.3.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev