[ovs-dev] [kernel-reload 8/8] xenserver: Introduce "force-reload-kmod" to reload kernel module.

[Ben Pfaff]

On Tue, Feb 08, 2011 at 03:34:03PM -0800, Ben Pfaff wrote:
> On Tue, Feb 08, 2011 at 03:24:32PM -0800, Ethan Jackson wrote:
> > On Tue, Feb 8, 2011 at 3:18 PM, Ben Pfaff <blp at nicira.com> wrote:
> > > On Tue, Feb 08, 2011 at 03:14:39PM -0800, Ethan Jackson wrote:
> > >> > +if test "$#" = 0; then
> > >> > + ? ?echo "# $0: no parameters given (use \"$0 --help\" for help)"
> > >> > +fi
> > >> Perhaps this should cause it to exit unsuccessfully.
> > >
> > > I did it this way in case the init script doesn't find any interfaces to
> > > save. ?I guess it could avoid invoking it at all in that case.
> > 
> > Yeah I was wondering about this.  A possible approach (which I
> > considered mentioning but decided against it) would be to change the
> > force_reload_kmod init script to give up when ovs-save fails (for
> > example on my system it fails because I don't have iptables-save).  
> 
> We can probably skip iptables entirely, without an error, if
> iptables-save is not present: Red Hat and Debian both put iptables and
> iptables-save in the same package, so if iptables-save is not present
> then it is unlikely that any iptables rules are configured.
> 
> > As part of this change, we could change ovs-save to be more exacting
> > and die with an exit code when things aren't as it expects.  This has
> > the benefit of preventing bugs in ovs-save or configuration errors
> > from hosing the system.
> 
> I thought about that.  I'll take another look at doing it that way.

I made some changes that move in that direction:

diff --git a/utilities/ovs-save b/utilities/ovs-save
index 76f02da..b2c726e 100755
--- a/utilities/ovs-save
+++ b/utilities/ovs-save
@@ -31,6 +31,23 @@ fi
 
 PATH=/sbin:/bin:/usr/sbin:/usr/bin
 
+missing_program () {
+    save_IFS=$IFS
+    IFS=:
+    for dir in $PATH; do
+        IFS=$save_IFS
+        if test -x $dir/$1; then
+            return 1
+        fi
+    done
+    IFS=$save_IFS
+    return 0
+}
+if missing_program ip; then 
+    echo "$0: ip not found in $PATH" >&2
+    exit 1
+fi
+
 if test "$#" = 0; then
     echo "# $0: no parameters given (use \"$0 --help\" for help)"
 fi
@@ -128,7 +145,13 @@ for dev in $devs; do
     echo
 done
 
-echo "# global"
-echo "iptables-restore <<'EOF'"
-iptables-save
-echo "EOF"
+if missing_program iptables-save; then
+    echo "# iptables-save not found in $PATH, not saving iptables state"
+else
+    echo "# global"
+    echo "iptables-restore <<'EOF'"
+    iptables-save
+    echo "EOF"
+fi
+
+exit 0
diff --git a/xenserver/etc_init.d_openvswitch b/xenserver/etc_init.d_openvswitch
index 582a153..a08a95d 100755
--- a/xenserver/etc_init.d_openvswitch
+++ b/xenserver/etc_init.d_openvswitch
@@ -474,7 +474,11 @@ function force_reload_kmod {
 
     script=$(mktemp)
     action "Save interface configuration to $script" true
-    /usr/share/openvswitch/scripts/ovs-save $ifaces > $script
+    if ! /usr/share/openvswitch/scripts/ovs-save $ifaces > $script; then
+        warning "Failed to save configuration, not replacing kernel module"
+        start
+        exit 1
+    fi
     chmod +x $script
 
     action "Destroy datapaths" remove_all_dp