diff mbox

[ovs-dev,v2,2/2] utilities: ovs-ctl: Handle start up errors

Message ID 20160912090757.23511-2-mchandras@suse.de
State Accepted
Headers show

Commit Message

Markos Chandras Sept. 12, 2016, 9:07 a.m. UTC 
Make sure we take the return values into consideration so we can
break early in case of failures. This makes the ovs-ctl helper more
accurate in reporting the real status of its managing processes.

Cc: Aaron Conole <aconole@redhat.com>
Signed-off-by: Markos Chandras <mchandras@suse.de>
---
 utilities/ovs-ctl.in | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

Comments

Aaron Conole Sept. 12, 2016, 12:49 p.m. UTC | #1 
Markos Chandras <mchandras@suse.de> writes:

> Make sure we take the return values into consideration so we can
> break early in case of failures. This makes the ovs-ctl helper more
> accurate in reporting the real status of its managing processes.
>
> Cc: Aaron Conole <aconole@redhat.com>
> Signed-off-by: Markos Chandras <mchandras@suse.de>

I like this change.  I haven't tested all the corner-cases with it, but
it's on my agenda - basically what happens during restart case when one
of the inner spawns causes failure.  I have tested some with it, and
like that it indicates status results properly.  I think you will need
to add something for the restart case, but that can be a follow on patch.

Acked-by: Aaron Conole <aconole@redhat.com>
Ben Pfaff Oct. 4, 2016, 6:54 p.m. UTC | #2 
On Mon, Sep 12, 2016 at 10:07:57AM +0100, Markos Chandras wrote:
> Make sure we take the return values into consideration so we can
> break early in case of failures. This makes the ovs-ctl helper more
> accurate in reporting the real status of its managing processes.
> 
> Cc: Aaron Conole <aconole@redhat.com>
> Signed-off-by: Markos Chandras <mchandras@suse.de>

Thanks for the patches.  I applied these to master.  I did not backport
them to any release branches because I am nervous about possibly
unexpected effects in corner cases.  However, if this fixes some
specific problem you were seeing with 2.6, let me know and I'll backport
them to branch-2.6.

Thanks,

Ben.
diff mbox

Patch

diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index dc275f8..ce3fb58 100755
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -190,8 +190,9 @@  do_start_ovsdb () {
 
 start_ovsdb() {
     if test X"$OVSDB_SERVER" = Xyes; then
-        do_start_ovsdb
+        do_start_ovsdb || return 1
     fi
+    return 0
 }
 
 add_managers () {
@@ -238,14 +239,16 @@  do_start_forwarding () {
         if test X"$SELF_CONFINEMENT" = Xno; then
             set "$@" --no-self-confinement
         fi
-        start_daemon "$OVS_VSWITCHD_PRIORITY" "$OVS_VSWITCHD_WRAPPER" "$@"
+        start_daemon "$OVS_VSWITCHD_PRIORITY" "$OVS_VSWITCHD_WRAPPER" "$@" ||
+            return 1
     fi
 }
 
 start_forwarding () {
     if test X"$OVS_VSWITCHD" = Xyes; then
-        do_start_forwarding
+        do_start_forwarding || return 1
     fi
+    return 0
 }
 
 ## ---- ##
@@ -364,7 +367,7 @@  force_reload_kmod () {
     # Restart the database first, since a large database may take a
     # while to load, and we want to minimize forwarding disruption.
     stop_ovsdb
-    start_ovsdb
+    start_ovsdb || return 1
 
     stop_forwarding
 
@@ -395,7 +398,7 @@  force_reload_kmod () {
 
     # Start vswitchd by asking it to wait till flow restore is finished.
     flow_restore_wait
-    start_forwarding
+    start_forwarding || return 1
 
     # Restore saved flows and inform vswitchd that we are done.
     restore_flows
@@ -422,13 +425,13 @@  restart () {
     # Restart the database first, since a large database may take a
     # while to load, and we want to minimize forwarding disruption.
     stop_ovsdb
-    start_ovsdb
+    start_ovsdb || return 1
 
     stop_forwarding
 
     # Start vswitchd by asking it to wait till flow restore is finished.
     flow_restore_wait
-    start_forwarding
+    start_forwarding || return 1
 
     # Restore saved flows and inform vswitchd that we are done.
     restore_flows
@@ -686,7 +689,7 @@  done
 case $command in
     start)
         start_ovsdb || exit 1
-        start_forwarding
+        start_forwarding || exit 1
         add_managers
         ;;
     stop)