Today the exit code refers to the execution of the change in the database. However, when not using parameter --no-wait (default), the ovs-vsctl also checks if OVSDB transactions are successfully recorded and reload by ovs-vswitchd. In this case, an error message is printed if there is a problem during the reload, like for example the one below:
# ovs-vsctl add-port br0 gre0 -- \ set interface gre0 type=ip6gre options:key=100 \ options:remote_ip=2001::2 ovs-vsctl: Error detected while setting up 'gre0': could not \ add network device gre0 to ofproto (Address family not supported\ by protocol). See ovs-vswitchd log for details. ovs-vsctl: The default log directory is "/var/log/openvswitch". # echo $? 0 This patch changes to exit with specific error code '3' if errors were reported during the reload. This change may break existing scripts because ovs-vsctl will start to fail when before it was succeeding. However, if an error is printed, then it is likely that the change was not functional anyway. Reported-at: https://bugzilla.redhat.com/1731553 Signed-off-by: Flavio Leitner <f...@sysclose.org> --- NEWS | 3 +++ tests/ovs-vsctl.at | 19 +++++++++++++++++-- tests/ovs-vswitchd.at | 2 +- tests/tunnel.at | 2 +- utilities/ovs-vsctl.8.in | 2 ++ utilities/ovs-vsctl.c | 29 +++++++++++++++++++++++------ 6 files changed, 47 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index cfd466663..f8f7b7655 100644 --- a/NEWS +++ b/NEWS @@ -28,6 +28,9 @@ Post-v3.1.0 * Added new options --[ovsdb-server|ovs-vswitchd]-umask=MODE to set umask value when starting OVS daemons. E.g., use --ovsdb-server-umask=0002 in order to create OVSDB sockets with access mode of 0770. + - ovs-vsctl: + * Exit with error code 3 if errors were reported while checking if OVSDB + transactions are successfully recorded and reload by ovs-vswitchd. - QoS: * Added new configuration option 'jitter' for a linux-netem QoS type. - DPDK: diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at index a368bff6e..2554152df 100644 --- a/tests/ovs-vsctl.at +++ b/tests/ovs-vsctl.at @@ -1522,7 +1522,7 @@ cat >experr <<EOF ovs-vsctl: Error detected while setting up 'reserved_name'. See ovs-vswitchd log for details. ovs-vsctl: The default log directory is "$OVS_RUNDIR". EOF -AT_CHECK([ovs-vsctl add-port br0 reserved_name], [0], [], [experr]) +AT_CHECK([ovs-vsctl add-port br0 reserved_name], [3], [], [experr]) # Prevent race. OVS_WAIT_UNTIL([test `grep -- "|WARN|" ovs-vswitchd.log | wc -l` -ge 1]) # Detect the warning log message @@ -1560,7 +1560,7 @@ cat >experr <<EOF ovs-vsctl: Error detected while setting up 'reserved_name'. See ovs-vswitchd log for details. ovs-vsctl: The default log directory is "$OVS_RUNDIR". EOF -AT_CHECK([ovs-vsctl add-port br0 reserved_name], [0], [], [experr]) +AT_CHECK([ovs-vsctl add-port br0 reserved_name], [3], [], [experr]) # Prevent race. OVS_WAIT_UNTIL([test `grep -- "|WARN|" ovs-vswitchd.log | wc -l` -ge 1]) # Detect the warning log message @@ -1737,3 +1737,18 @@ AT_CHECK([ovs-vsctl --no-wait --bare --columns _uuid,name list bridge tst1], [0] OVS_VSCTL_CLEANUP AT_CLEANUP + +AT_SETUP([ovs-vsctl -- return error if OVSDB reload issues are reported]) +OVS_VSWITCHD_START +dnl check if ovs-vsctl returns error 3 if ovs-vswitchd fails to reload. + +cat >experr << EOF +ovs-vsctl: Error detected while setting up 'gre0': gre0: bad ip6gre 'remote_ip' +gre0: ip6gre type requires valid 'remote_ip' argument. See ovs-vswitchd log for details. +ovs-vsctl: The default log directory is "$OVS_RUNDIR". +EOF +AT_CHECK([ovs-vsctl add-port br0 gre0 -- set interface gre0 type=ip6gre options:key=100 options:remote_ip=192.168.0.300], [3], [], [experr]) +OVS_VSWITCHD_STOP(["/is not a valid IP address/d +/netdev_vport|WARN|gre0: bad ip6gre 'remote_ip'/d +/netdev|WARN|gre0: could not set configuration/d"]) +AT_CLEANUP diff --git a/tests/ovs-vswitchd.at b/tests/ovs-vswitchd.at index 977b2eba1..81604111b 100644 --- a/tests/ovs-vswitchd.at +++ b/tests/ovs-vswitchd.at @@ -222,7 +222,7 @@ cat >experr <<EOF ovs-vsctl: Error detected while setting up 'b/c'. See ovs-vswitchd log for details. ovs-vsctl: The default log directory is "$OVS_RUNDIR". EOF -AT_CHECK([ovs-vsctl add-br b/c -- set bridge b/c datapath-type=dummy], [0], [], [experr]) +AT_CHECK([ovs-vsctl add-br b/c -- set bridge b/c datapath-type=dummy], [3], [], [experr]) AT_CHECK([test ! -e b/c.mgmt]) OVS_VSWITCHD_STOP(['/ignoring bridge with invalid name/d']) diff --git a/tests/tunnel.at b/tests/tunnel.at index ddeb66bc9..cf66cc085 100644 --- a/tests/tunnel.at +++ b/tests/tunnel.at @@ -1009,7 +1009,7 @@ OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=vxlan \ options:remote_ip=flow ofport_request=1]) AT_CHECK([ovs-vsctl add-port br0 p2 -- set Interface p2 type=vxlan \ - options:remote_ip=flow options:exts=gbp options:key=1 ofport_request=2], [0], + options:remote_ip=flow options:exts=gbp options:key=1 ofport_request=2], [3], [], [ignore]) AT_CHECK([grep 'p2: could not set configuration (File exists)' ovs-vswitchd.log | sed "s/^.*\(p2:.*\)$/\1/"], [0], diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in index 9e319aa1c..929285e66 100644 --- a/utilities/ovs-vsctl.8.in +++ b/utilities/ovs-vsctl.8.in @@ -892,6 +892,8 @@ Usage, syntax, or configuration file error. .IP "2" The \fIbridge\fR argument to \fBbr\-exists\fR specified the name of a bridge that does not exist. +.IP "3" +An error has been reported post OVSDB reload. .SH "SEE ALSO" . .BR ovsdb\-server (1), diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c index 2f5ac1a26..8daa1d409 100644 --- a/utilities/ovs-vsctl.c +++ b/utilities/ovs-vsctl.c @@ -56,6 +56,9 @@ VLOG_DEFINE_THIS_MODULE(vsctl); +/* Post OVSDB reload error reported. */ +#define EXIT_POSTDB_ERROR 3 + struct vsctl_context; /* --db: The database server to contact. */ @@ -115,7 +118,7 @@ static void parse_options(int argc, char *argv[], struct shash *local_options); static void run_prerequisites(struct ctl_command[], size_t n_commands, struct ovsdb_idl *); static bool do_vsctl(const char *args, struct ctl_command *, size_t n, - struct ovsdb_idl *); + struct ovsdb_idl *, bool *); /* post_db_reload_check frame work is to allow ovs-vsctl to do additional * checks after OVSDB transactions are successfully recorded and reload by @@ -134,11 +137,13 @@ static bool do_vsctl(const char *args, struct ctl_command *, size_t n, * Current implementation only check for Post OVSDB reload failures on new * interface additions with 'add-br' and 'add-port' commands. * + * post_db_reload_check returns 'true' if a failure is reported. + * * post_db_reload_expect_iface() * * keep track of interfaces to be checked post OVSDB reload. */ static void post_db_reload_check_init(void); -static void post_db_reload_do_checks(const struct vsctl_context *); +static bool post_db_reload_do_checks(const struct vsctl_context *); static void post_db_reload_expect_iface(const struct ovsrec_interface *); static struct uuid *neoteric_ifaces; @@ -200,9 +205,15 @@ main(int argc, char *argv[]) } if (seqno != ovsdb_idl_get_seqno(idl)) { + bool postdb_err; + seqno = ovsdb_idl_get_seqno(idl); - if (do_vsctl(args, commands, n_commands, idl)) { + if (do_vsctl(args, commands, n_commands, idl, &postdb_err)) { free(args); + if (postdb_err) { + exit(EXIT_POSTDB_ERROR); + } + exit(EXIT_SUCCESS); } } @@ -2674,7 +2685,7 @@ post_db_reload_expect_iface(const struct ovsrec_interface *iface) neoteric_ifaces[n_neoteric_ifaces++] = iface->header_.uuid; } -static void +static bool post_db_reload_do_checks(const struct vsctl_context *vsctl_ctx) { bool print_error = false; @@ -2707,6 +2718,8 @@ post_db_reload_do_checks(const struct vsctl_context *vsctl_ctx) if (print_error) { ovs_error(0, "The default log directory is \"%s\".", ovs_logdir()); } + + return print_error; } @@ -2815,7 +2828,7 @@ vsctl_parent_process_info(void) static bool do_vsctl(const char *args, struct ctl_command *commands, size_t n_commands, - struct ovsdb_idl *idl) + struct ovsdb_idl *idl, bool *postdb_err) { struct ovsdb_idl_txn *txn; const struct ovsrec_open_vswitch *ovs; @@ -2827,6 +2840,8 @@ do_vsctl(const char *args, struct ctl_command *commands, size_t n_commands, int64_t next_cfg = 0; char *ppid_info = NULL; + ovs_assert(postdb_err); + *postdb_err = false; txn = the_idl_txn = ovsdb_idl_txn_create(idl); if (dry_run) { ovsdb_idl_txn_set_dry_run(txn); @@ -2989,7 +3004,9 @@ do_vsctl(const char *args, struct ctl_command *commands, size_t n_commands, ovsdb_idl_run(idl); OVSREC_OPEN_VSWITCH_FOR_EACH (ovs, idl) { if (ovs->cur_cfg >= next_cfg) { - post_db_reload_do_checks(&vsctl_ctx); + if (post_db_reload_do_checks(&vsctl_ctx)) { + *postdb_err = true; + } goto done; } } -- 2.40.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev