[ovs-dev] [rhel --user 4/7] utilities: add --user option to ovs-ctl

[Andy Zhou]

On Tue, Nov 24, 2015 at 3:25 PM, Ben Pfaff <blp at ovn.org> wrote:

> On Thu, Nov 19, 2015 at 12:58:39PM -0800, Andy Zhou wrote:
> > Allow ovs-ctl to take --user=USER option. When this option is specified
> > 'USER' will be parsed in the format of 'user:group" and be set into
> > shell variables $OVS_USER and $OVS_GROUP, which will be used
> > by shell functions, such as start_daemon() to run OVS daemons under
> > the specified user.
> >
> > Signed-off-by: Andy Zhou <azhou at ovn.org>
> > ---
> >  utilities/ovs-ctl.8  |  4 ++++
> >  utilities/ovs-ctl.in |  4 ++++
> >  utilities/ovs-lib.in | 22 ++++++++++++++++++++++
> >  3 files changed, 30 insertions(+)
> >
> > diff --git a/utilities/ovs-ctl.8 b/utilities/ovs-ctl.8
> > index 6a9a544..50e3118 100644
> > --- a/utilities/ovs-ctl.8
> > +++ b/utilities/ovs-ctl.8
> > @@ -123,6 +123,10 @@ another string is specified \fBovs\-ctl\fR uses it
> literally.
> >  The following options should be specified if the defaults are not
> >  suitable:
> >  .
> > +.IP "\fB\-\-user=\fIuser[:group]\fR"
>
> I think the right formatting would be:
> .IP "\fB\-\-user=\fIuser\fR[\fB:\fIgroup\fR]"
> i.e. : in bold, [] in roman.
>
O.K. will fix

>
> > +Run OVS daemons as the user specified. When this options is specified,
> OVS
> > +daemons will run with the least privileges necessary.
>
> The above should probably be more specific (mention the group at
> least?).
>
I changed it as follows:

daemons will run with the least privileges necessary.
+daemons will run with the least privileges necessary, and switch the
+deemon process's real and effective user and group to the ones specified.



>
> I think that this should say "--user=USER[:GROUP]":
> > +  --user=USER                    run ovs daemons as the root user of
> ovs user (default: $OVS_USER:$OVS_GROUP)
> >    --ovsdb-server-priority=NICE   set ovsdb-server's niceness (default:
> $OVSDB_SERVER_PRIORITY)
> >    --ovs-vswitchd-priority=NICE   set ovs-vswitchd's niceness (default:
> $OVS_VSWITCHD_PRIORITY)
>
> > +set_ovs_user_group() {
> > +    value=$1   # user spec (e.g. ovs:ovs)
> > +
> > +    case $value in
> > +    [a-z]*:*)
> > +        OVS_USER=`expr X"$value" : 'X\(.*\):.*'`
> > +        OVS_GROUP=`expr X"$value" : 'X[^:]*:\(.*\)'`
> > +        if  test X"$OVS_GROUP" = X; then
> > +           OVS_GROUP=$OVS_USER
> > +        fi
> > +        ;;
> > +    [a-z]*)
> > +        OVS_USER=`expr X"$value" : 'X\(.*\)'`
> > +        OVS_GROUP=$OVS_USER
>
> I think that the above two cases can be combined.  The code for the
> first case handles the second just fine, doesn't it?
>
':' seems to be significant.  'ovs' without the ':' seems to fall into the
3rd case.  Did I miss something?

>
> > +        ;;
> > +    *)
> > +        OVS_USER=root
> > +        OVS_GROUP=root
> > +        ;;
> > +    esac
> > +}
>
> None of this is a big deal, so:
> Acked-by: Ben Pfaff <blp at ovn.org>
>
> Thanks for the review.