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://issues.redhat.com/browse/FDP-793
Signed-off-by: Flavio Leitner <f...@sysclose.org>
---
v5:
Fixed afxdp testsuite.
v4:
Reworded the NEWS entry.
Reworded the man-page change.
Wrapped the 2 long AT_CHECK() lines.
Added the name for the bool argument.
Updated the comment to use post_db_reload_do_checks.
Renamed print_error to reconfig_failed.
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 | 32 +++++++++++++++++++++++++++++--
tests/ovs-vswitchd.at | 6 +++++-
tests/system-afxdp.at | 2 +-
tests/tunnel.at | 8 +++++++-
utilities/ovs-vsctl.8.in | 11 +++++++++++
utilities/ovs-vsctl.c | 41 +++++++++++++++++++++++++++++++---------
7 files changed, 91 insertions(+), 14 deletions(-)
diff --git a/NEWS b/NEWS
index c7bb53f2d..b5af71492 100644
--- a/NEWS
+++ b/NEWS
@@ -37,6 +37,11 @@ Post-v3.5.0
the OVS distribution in the 3.0 release and is no longer present in
any supported versions of OVS. The remaining documentation of this
kernel module relates to topics for older releases of OVS.
+ - ovs-vsctl:
+ * Now exits with error code 160 (ERROR_BAD_ARGUMENTS) on Windows and
+ 65 (EX_DATAERR) on other platforms if it fails while waiting for
+ ovs-vswitchd to finish reconfiguring itself after a successful
+ database transaction.
v3.5.0 - 17 Feb 2025
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index e488e292d..ba00293db 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -1620,7 +1620,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
@@ -1655,7 +1659,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
@@ -1831,3 +1839,23 @@ 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 730363e83..173e62367 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/system-afxdp.at b/tests/system-afxdp.at
index 88f660566..5671b81f0 100644
--- a/tests/system-afxdp.at
+++ b/tests/system-afxdp.at
@@ -12,7 +12,7 @@ ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
AT_CHECK([ovs-vsctl del-port ovs-p0])
AT_CHECK([ovs-vsctl add-port br0 ovs-p0 -- \
set interface ovs-p0 type=afxdp options:n_rxq=42],
- [0], [], [stderr])
+ [65], [], [stderr])
OVS_WAIT_UNTIL([grep "ovs-p0: could not set configuration" ovs-vswitchd.log])
sleep 5
AT_CHECK([grep "ovs-p0: could not set configuration" ovs-vswitchd.log | wc -l],
diff --git a/tests/tunnel.at b/tests/tunnel.at
index 60ed8ba6a..09e3c94c7 100644
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -1041,9 +1041,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 575b7c0bf..e41e6364b 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -909,6 +909,17 @@ 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 (Linux and BSD) or 160 (Windows)"
+The database transaction was committed successfully, but \fBovs\-vsctl\fR
failed
+while waiting for \fBovs\-vswitchd\fR to finish reconfiguring itself to reflect
+the changes. This typically happens when \fB--no\-wait\fR is not specified and
+the daemon does not acknowledge the update in time or encounters an error
+applying it.
+.br
+.sp
+Note that the change remains recorded in the database and is not rolled back.
+This exit code is intended to signal that while the database was updated, the
+system configuration may not yet reflect the intended changes.
.SH "SEE ALSO"
.
.BR ovsdb\-server (1),
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 691b974e3..3ccbcd294 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 *postdb_err);
/* 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_do_checks 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);
}
}
@@ -2824,10 +2841,10 @@ 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;
+ bool reconfig_failed = false;
size_t i;
for (i = 0; i < n_neoteric_ifaces; i++) {
@@ -2849,14 +2866,16 @@ post_db_reload_do_checks(const struct vsctl_context
*vsctl_ctx)
"See ovs-vswitchd log for details.",
iface->name);
}
- print_error = true;
+ reconfig_failed = true;
}
}
}
- if (print_error) {
+ if (reconfig_failed) {
ovs_error(0, "The default log directory is \"%s\".", ovs_logdir());
}
+
+ return reconfig_failed;
}
@@ -2965,7 +2984,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;
@@ -2977,6 +2996,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);
@@ -3139,7 +3160,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.50.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
