On Tue, Nov 24, 2015 at 3:25 PM, Ben Pfaff <b...@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 <az...@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 <b...@ovn.org>
>
> Thanks for the review.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
