On 01/24/2017 04:53 PM, Boris Fiuczynski wrote: > 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. > > Signed-off-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx> > Reviewed-by: Viktor Mihajlovski <mihajlov@xxxxxxxxxxxxxxxxxx> > Reviewed-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxxxxxxx> > --- > 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) Isn't 120 too much? It's 2 minutes. Such heavy loaded machine is not suitable for virtualization. > +]) > + > +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]) This doesn't need to be here. > + 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); > > There are other occurrences of --timeout=: src/util/virnetdevopenvswitch.c:303: cmd = virCommandNewArgList(OVSVSCTL, "--timeout=5", src/util/virnetdevopenvswitch.c:318: cmd = virCommandNewArgList(OVSVSCTL, "--timeout=5", src/util/virnetdevopenvswitch.c:349: cmd = virCommandNewArgList(OVSVSCTL, "--timeout=5", src/util/virnetdevopenvswitch.c:416: cmd = virCommandNewArgList(OVSVSCTL, "--timeout=5", "get", "Interface", No need to send another version. I can fix this before pushing. I just like to know your ideas to my points. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list