Markos Chandras <mchand...@suse.de> writes: > Make sure we communicate failures to the caller when start_daemon fails > to start a process as the caller may not be able to proceed after this. > > Signed-off-by: Markos Chandras <mchand...@suse.de> > --- > utilities/ovs-lib.in | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/utilities/ovs-lib.in b/utilities/ovs-lib.in > index 773efb3..6fd861e 100644 > --- a/utilities/ovs-lib.in > +++ b/utilities/ovs-lib.in > @@ -190,7 +190,7 @@ start_daemon () { > set nice -n "$priority" "$@" > fi > > - action "Starting $daemon" "$@" > + action "Starting $daemon" "$@" || return 1 > > if test X"$strace" != X; then > # Strace doesn't have the -D option so we attach after the fact.
I agree with this change, but I think it needs to be extended all the way (so, that means into ovs-ctl as well). Currently, there's no difference in the script return value when I have your patch applied or not (ie: ovs-ctl --no-ovsdb-server start and then echo $? yields 0 when I have renamed ovs-vswitchd to cause failure). Is it possible for you to post a follow-up patch for ovs-ctl.in that looks something like below (NOTE: not tested at all - caveat emptor)? diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in index dc275f8..562b86d 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 + return do_start_ovsdb 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 + return do_start_forwarding 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) _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev