[ovs-dev] [initscripts 5/5] Refactor initscripts into distro-independent and distro-specific pieces.
Andrew Evans
aevans at nicira.com
Thu Jun 16 22:35:01 UTC 2011
Thanks, this looks like a big improvement. Some minor comments below:
On Wed, 2011-06-15 at 16:42 -0700, Ben Pfaff wrote:
>+The \fBstart\fR command starts up Open vSwitch. It performs the
>+following tasks:
Can we remove the word "up" here?
>+Loads the Open vSwitch kernel module. If this fails, and the Linux
>+bridge module is loaded but no bridges exists, it tries to unload the
Can we strike the "s" in "exists"?
> +If the Open vSwitch database file does not exist, it creates it.
> +If the database does exist, but it has an obsolete version, it
> +upgrades it to the last schema.
I think "last" could be misinterpreted to mean "previous" here. How
about "latest" or "current" instead?
> +\fBOpen_vSwitch\fR table. Specifying this option multiple times adds
> +multiple key-pair pairs.
Should this be "key-value"?
> +.SH "The ``stop'' command"
> +.
> +.PP
> +The \fBstart\fR command starts up Open vSwitch. If
> +\fBovs\-vswitchd\fR is running, kills it and waits for it to
> +terminate, then it does the same for \fBovsdb\-server\fR.
Did you mean "The stop command stops Open vSwitch" instead?
> +Because \fBforce\-kmod\-reload\fR internally stop and starts OVS, it
> +accepts all of the options accepted by the \fBstart\fR command.
Should this read "stops and starts" instead?
> +case $0 in
> + */*) dir0=`echo "$0" | sed 's,/[^/]*$,,'` ;;
> + *) dir0=./ ;;
> +esac
Is there some reason not to use 'dirname'?
> + # Allow GRE traffic.
> + iptables -I INPUT -p gre -j ACCEPT
What if iptables isn't installed, or if the kernel wasn't built with the
necessary netfilter options?
> + # - There is an internal interface for every bridge, whether it
> + # has an Interface record or not and whether the Interface
> + # record's 'type' is properly set or net.
s/or net/or not/ ?
> +etcdir=$sysconfdir/openvswitch # /etc/openvswitch
Incidentally, I applied this patch series and built and installed .debs
on Ubuntu 11/Natty, and my database ended up
in /usr/etc/openvswitch/conf.db rather than /etc/openvswitch/conf.db.
I'm not sure if it's related to these changes.
Also, git-am complained about trailing whitespace in this patch.
More information about the dev
mailing list