Since a successful completion of the calls to openvswitch is
expected
a long timeout should be chosen to account for heavily loaded systems.
Therefore this patch increases the timeout value from 5 to 120 seconds
as default value and also allows to set the openvswitch timeout value
by specifying with-ovs-timeout when running configure.
Why make it configurable during build? I don't think we do this with any
other type of timeout value or limit. If you think it may need to change
based on circumstances, why not just put it in libvirtd.conf and be done
with it?
In the meantime, I agree with Michal that any machine that takes 120
seconds to get a response from any ovs command is beyond the limits of
usable; we certainly shouldn't cater our defaults to that.
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
Reviewed-by: Viktor Mihajlovski <mihajlov(a)linux.vnet.ibm.com>
Reviewed-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
---
configure.ac | 3 +++
m4/virt-ovs-timeout.m4 | 34 ++++++++++++++++++++++++++++++++++
src/util/virnetdevopenvswitch.c | 9 +++++----
3 files changed, 42 insertions(+), 4 deletions(-)
create mode 100644 m4/virt-ovs-timeout.m4
diff --git a/configure.ac b/configure.ac
index 7efaddb..e70dfdc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -463,6 +463,7 @@ LIBVIRT_ARG_LOADER_NVRAM
LIBVIRT_ARG_LOGIN_SHELL
LIBVIRT_ARG_HOST_VALIDATE
LIBVIRT_ARG_TLS_PRIORITY
+LIBVIRT_ARG_OVS_TIMEOUT
LIBVIRT_ARG_SYSCTL_CONFIG
@@ -477,6 +478,7 @@ LIBVIRT_CHECK_LOADER_NVRAM
LIBVIRT_CHECK_LOGIN_SHELL
LIBVIRT_CHECK_HOST_VALIDATE
LIBVIRT_CHECK_TLS_PRIORITY
+LIBVIRT_CHECK_OVS_TIMEOUT
LIBVIRT_CHECK_SYSCTL_CONFIG
LIBVIRT_CHECK_NSS
@@ -1012,6 +1014,7 @@ LIBVIRT_RESULT_LOADER_NVRAM
LIBVIRT_RESULT_LOGIN_SHELL
LIBVIRT_RESULT_HOST_VALIDATE
LIBVIRT_RESULT_TLS_PRIORITY
+LIBVIRT_RESULT_OVS_TIMEOUT
AC_MSG_NOTICE([])
AC_MSG_NOTICE([Developer Tools])
AC_MSG_NOTICE([])
diff --git a/m4/virt-ovs-timeout.m4 b/m4/virt-ovs-timeout.m4
new file mode 100644
index 0000000..a5174d5
--- /dev/null
+++ b/m4/virt-ovs-timeout.m4
@@ -0,0 +1,34 @@
+dnl The OVS timeout check
+dnl
+dnl Copyright (C) 2017 IBM Corporation
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library. If not, see
+dnl <
http://www.gnu.org/licenses/>.
+dnl
+
+AC_DEFUN([LIBVIRT_ARG_OVS_TIMEOUT], [
+ LIBVIRT_ARG_WITH([OVS_TIMEOUT],
+ [set the default OVS timeout],
+ 120)
+])
+
+AC_DEFUN([LIBVIRT_CHECK_OVS_TIMEOUT], [
+ AC_DEFINE_UNQUOTED([OVS_TIMEOUT], ["$with_ovs_timeout"],
+ [default OVS timeout])
+])
+
+AC_DEFUN([LIBVIRT_RESULT_OVS_TIMEOUT], [
+dnl AC_MSG_NOTICE([OVS timeout: $with_ovs_timeout])
+ LIBVIRT_RESULT([ OVS timeout], [$with_ovs_timeout])
+])
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index e6cb096..ba44b3b 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -35,6 +35,7 @@
#include "virlog.h"
#define VIR_FROM_THIS VIR_FROM_NONE
+#define OVS_TIMEOUT_ARG "--timeout=" OVS_TIMEOUT
VIR_LOG_INIT("util.netdevopenvswitch");
@@ -89,7 +90,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char
*ifname,
cmd = virCommandNew(OVSVSCTL);
- virCommandAddArgList(cmd, "--timeout=5", "--",
"--if-exists", "del-port",
+ virCommandAddArgList(cmd, OVS_TIMEOUT_ARG, "--", "--if-exists",
"del-port",
ifname, "--", "add-port", brname, ifname,
NULL);
if (virtVlan && virtVlan->nTags > 0) {
@@ -183,7 +184,7 @@ int virNetDevOpenvswitchRemovePort(const char *brname
ATTRIBUTE_UNUSED, const ch
virCommandPtr cmd = NULL;
cmd = virCommandNew(OVSVSCTL);
- virCommandAddArgList(cmd, "--timeout=5", "--",
"--if-exists", "del-port", ifname, NULL);
+ virCommandAddArgList(cmd, OVS_TIMEOUT_ARG, "--", "--if-exists",
"del-port", ifname, NULL);
if (virCommandRun(cmd, NULL) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -212,7 +213,7 @@ int virNetDevOpenvswitchGetMigrateData(char **migrate, const char
*ifname)
size_t len;
int ret = -1;
- cmd = virCommandNewArgList(OVSVSCTL, "--timeout=5",
"--if-exists", "get", "Interface",
+ cmd = virCommandNewArgList(OVSVSCTL, OVS_TIMEOUT_ARG, "--if-exists",
"get", "Interface",
ifname, "external_ids:PortData", NULL);
virCommandSetOutputBuffer(cmd, migrate);
@@ -255,7 +256,7 @@ int virNetDevOpenvswitchSetMigrateData(char *migrate, const char
*ifname)
return 0;
}
- cmd = virCommandNewArgList(OVSVSCTL, "--timeout=5", "set",
+ cmd = virCommandNewArgList(OVSVSCTL, OVS_TIMEOUT_ARG, "set",
"Interface", ifname, NULL);
virCommandAddArgFormat(cmd, "external_ids:PortData=%s", migrate);