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 160
(ERROR_BAD_ARGUMENTS) on Windows and 65 (EX_DATAERR) on
Linux or BSD 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>
---
v3: Fixed the Windows build issue reported by Ilya.
Return ERROR_BAD_ARGUMENTS on Windows.
v2:
Followed Aaron's suggestion to return EX_DATAERR.
NEWS | 5 +++++
tests/ovs-vsctl.at | 30 ++++++++++++++++++++++++++++--
tests/ovs-vswitchd.at | 6 +++++-
tests/tunnel.at | 8 +++++++-
utilities/ovs-vsctl.8.in | 4 ++++
utilities/ovs-vsctl.c | 35 +++++++++++++++++++++++++++++------
6 files changed, 78 insertions(+), 10 deletions(-)
diff --git a/NEWS b/NEWS
index 6a990c921..8c733e417 100644
--- a/NEWS
+++ b/NEWS
@@ -30,6 +30,11 @@ 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 160 (ERROR_BAD_ARGUMENTS) on Windows or
+ 65 (EX_DATAERR) on other platforms 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..a8274734f 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -1522,7 +1522,11 @@ 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])
+if test "$IS_WIN32" = "yes"; then
+AT_CHECK([ovs-vsctl add-port br0 reserved_name], [160], [], [experr])
+else
+AT_CHECK([ovs-vsctl add-port br0 reserved_name], [65], [], [experr])
+fi
# Prevent race.
OVS_WAIT_UNTIL([test `grep -- "|WARN|" ovs-vswitchd.log | wc -l` -ge 1])
# Detect the warning log message
@@ -1560,7 +1564,11 @@ 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])
+if test "$IS_WIN32" = "yes"; then
+AT_CHECK([ovs-vsctl add-port br0 reserved_name], [160], [], [experr])
+else
+AT_CHECK([ovs-vsctl add-port br0 reserved_name], [65], [], [experr])
+fi
# Prevent race.
OVS_WAIT_UNTIL([test `grep -- "|WARN|" ovs-vswitchd.log | wc -l` -ge 1])
# Detect the warning log message
@@ -1737,3 +1745,21 @@ 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
+
+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
+if test "$IS_WIN32" = "yes"; then
+AT_CHECK([ovs-vsctl add-port br0 gre0 -- set interface gre0 type=ip6gre
options:key=100 options:remote_ip=192.168.0.300], [160], [], [experr])
+else
+AT_CHECK([ovs-vsctl add-port br0 gre0 -- set interface gre0 type=ip6gre
options:key=100 options:remote_ip=192.168.0.300], [65], [], [experr])
+fi
+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..8fcfc6ec1 100644
--- a/tests/ovs-vswitchd.at
+++ b/tests/ovs-vswitchd.at
@@ -222,7 +222,11 @@ 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])
+if test "$IS_WIN32" = "yes"; then
+AT_CHECK([ovs-vsctl add-br b/c -- set bridge b/c datapath-type=dummy], [160],
[], [experr])
+else
+AT_CHECK([ovs-vsctl add-br b/c -- set bridge b/c datapath-type=dummy], [65],
[], [experr])
+fi
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..5550d298d 100644
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -1008,9 +1008,15 @@ AT_SETUP([tunnel - concomitant incompatible tunnels on
the same port])
OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=vxlan \
options:remote_ip=flow ofport_request=1])
+if test "$IS_WIN32" = "yes"; then
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], [160],
[], [ignore])
+else
+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], [65],
+ [], [ignore])
+fi
AT_CHECK([grep 'p2: could not set configuration (File exists)'
ovs-vswitchd.log | sed "s/^.*\(p2:.*\)$/\1/"], [0],
[p2: could not set configuration (File exists)
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 9e319aa1c..4e2c96710 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -892,6 +892,10 @@ 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 "65"
+An error has been reported post OVSDB reload (Linux and BSD).
+.IP "160"
+An error has been reported post OVSDB reload (Windows).
.SH "SEE ALSO"
.
.BR ovsdb\-server (1),
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 2f5ac1a26..fcc01e8ad 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -56,6 +56,15 @@
VLOG_DEFINE_THIS_MODULE(vsctl);
+/* Post OVSDB reload error reported. */
+#ifdef _WIN32
+#include <WinError.h>
+#define EXIT_POSTDB_ERROR ERROR_BAD_ARGUMENTS
+#else
+#include <sysexits.h>
+#define EXIT_POSTDB_ERROR EX_DATAERR
+#endif
+
struct vsctl_context;
/* --db: The database server to contact. */
@@ -115,7 +124,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 +143,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 +211,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 +2691,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 +2724,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 +2834,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 +2846,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 +3010,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.41.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
