On Tue, Jun 21, 2016 at 7:27 PM, Daniele Di Proietto <diproiet...@vmware.com
> wrote:
> Commit 9b5422a98f81("ovs-lib: Try to call exit before killing.")
> introduced a problem where internal interfaces are destroyed and
> recreated, losing their IP address.
>
> Commit 9aad5a5a96ba("ovs-vswitchd: Preserve datapath ports across
> graceful shutdown.") fixed the problem by changing ovs-vswitchd
> to preserve the ports on `ovs-appctl exit`. Unfortunately, this fix is
> not enough during upgrade from <= 2.5.0, where an old ovs-vswitchd is
> running (without the fix) and a new ovs-lib script is performing the
> restart.
>
> The problem seem to affect both RHEL and ubuntu.
>
> This commit fixes the upgrade by looking at the running daemon
> version and avoid using `ovs-appctl exit` if it's < 2.5.90.
>
> Suggested-by: Gurucharan Shetty <g...@ovn.org>
> Signed-off-by: Daniele Di Proietto <diproiet...@vmware.com>
> ---
> v1->v2:
> `if` condition was broken, plus `-a` and `-o` parameters for `test` are not
> portable. This version uses a more general approach with awk. Also, awk
> is
> now used to get the version string from ovs-appctl.
> ---
> utilities/ovs-lib.in | 37 +++++++++++++++++++++++++++++++++----
> 1 file changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in
> index 773efb3..7fcd734 100644
> --- a/utilities/ovs-lib.in
> +++ b/utilities/ovs-lib.in
> @@ -132,6 +132,22 @@ pid_comm_check () {
> [ "$1" = "`cat /proc/$2/comm`" ]
> }
>
> +# version_geq version_a version_b
> +#
> +# Compare (dot separated) version numbers. Returns true (exit code 0) if
> +# version_a is greater or equal than version_b, otherwise false (exit
> code 1).
> +version_geq() {
> + echo $1 $2 | awk '{
> + n1 = split($1, a, ".");
> + n2 = split($2, b, ".");
> + n = (n1 > n2) ? n1 : n2;
> + for (i = 1; i <= n; i++) {
> + if (a[i]+0 < b[i]+0) exit 1
> + if (a[i]+0 > b[i]+0) exit 0
> + }
> + }'
> +}
> +
> start_daemon () {
> priority=$1
> wrapper=$2
> @@ -202,10 +218,23 @@ start_daemon () {
> stop_daemon () {
> if test -e "$rundir/$1.pid"; then
> if pid=`cat "$rundir/$1.pid"`; then
> - for action in EXIT .1 .25 .65 1 \
> - TERM .1 .25 .65 1 1 1 1 \
> - KILL 1 1 1 2 10 15 30 \
> - FAIL; do
> +
> + graceful="EXIT .1 .25 .65 1"
> + actions="TERM .1 .25 .65 1 1 1 1 \
> + KILL 1 1 1 2 10 15 30 \
> + FAIL"
> + version=`ovs-appctl -T 1 -t $rundir/$1.$pid.ctl version \
> + | awk 'NR==1{print $NF}'`
> +
> + # Use `ovs-appctl exit` only if the running daemon version
> + # is >= 2.5.90. This script might be used during upgrade to
> + # stop older versions of daemons which do not behave correctly
> + # with `ovs-appctl exit` (e.g. ovs-vswitchd <= 2.5.0 deletes
> + # internal ports).
> + if version_geq "$version" "2.5.90"; then
>
1) Is it normal in this code base to embed specific version numbers in a
generic library file ?
2) If coming from < 2.5.90 then the problem that Commit 9b5422a98f81 was
trying to fix
will exist ?
In general, do you need to document this somewhere at user level (
install.md or somewhere else) ?
> + actions="$graceful $actions"
> + fi
> + for action in $actions; do
> if pid_exists "$pid" >/dev/null 2>&1; then :; else
> return 0
> fi
> --
> 2.8.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
