ovs-vsctl: Try connecting only once for active connections by default.

Until now, ovs-vsctl has kept trying to the database server until it
succeeded or the timeout expired (if one was specified with --timeout).
This meant that if ovsdb-server wasn't running, then ovs-vsctl would hang.
The result was that almost every ovs-vsctl invocation in scripts specified
a timeout on the off-chance that the database server might not be running.
But it's difficult to choose a good timeout.  A timeout that is too short
can cause spurious failures.  A timeout that is too long causes long delays
if the server really isn't running.

This commit should alleviate this problem.  It changes ovs-vsctl's behavior
so that, if it fails to connect to the server, it exits unsuccessfully.
This makes --timeout obsolete for the purpose of avoiding a hang if the
database server isn't running.  (--timeout is still useful to avoid a hang
if ovsdb-server is running but ovs-vswitchd is not, for ovs-vsctl commands
that modify the database.  --no-wait also avoids that issue.)

Bug #2393.
Bug #15594.
Reported-by: Jeff Merrick <jmerrick@vmware.com>
Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/AUTHORS b/AUTHORS
index bd0f499..2723fd2 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -143,6 +143,7 @@ Jan Medved              jmedved@juniper.net
 Janis Hamme             janis.hamme@student.kit.edu
 Jari Sundell            sundell.software@gmail.com
 Jed Daniels             openvswitch@jeddaniels.com
 Janis Hamme             janis.hamme@student.kit.edu
 Jari Sundell            sundell.software@gmail.com
 Jed Daniels             openvswitch@jeddaniels.com

+Jeff Merrick            jmerrick@vmware.com

 Jeongkeun Lee           jklee@hp.com
 Jing Ai                 ai_jing2000@hotmail.com
 Joan Cirer              joan@ev0.net
 Jeongkeun Lee           jklee@hp.com
 Jing Ai                 ai_jing2000@hotmail.com
 Joan Cirer              joan@ev0.net

diff --git a/NEWS b/NEWS
index 432022f..79fde29 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,11 @@
 post-v1.10.0
 ---------------------
 post-v1.10.0
 ---------------------

+    - ovs-vsctl:
+      * Previously ovs-vsctl would retry connecting to the database forever,
+        causing it to hang if ovsdb-server was not running.  Now, ovs-vsctl
+        only tries once by default (use --retry to try forever).  This change
+        means that you may want to remove uses of --timeout to avoid hangs
+        in ovs-vsctl calls.

     - Stable bond mode has been removed.
     - The autopath action has been removed.
     - New support for the data encapsulation format of the LISP tunnel
     - Stable bond mode has been removed.
     - The autopath action has been removed.
     - New support for the data encapsulation format of the LISP tunnel

diff --git a/debian/ifupdown.sh b/debian/ifupdown.sh
index f6a7750..f728f7c 100755 (executable)
--- a/debian/ifupdown.sh
+++ b/debian/ifupdown.sh
@@ -1,6 +1,6 @@
 #! /bin/sh
 
 #! /bin/sh
 

-# Copyright (c) 2012 Nicira, Inc.
+# Copyright (c) 2012, 2013 Nicira, Inc.

 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.


@@ -22,7 +22,7 @@ if [ -z "${IF_OVS_TYPE}" ]; then
 fi
 
 ovs_vsctl() {
 fi
 
 ovs_vsctl() {

-    ovs-vsctl --timeout=5 "$@"
+    ovs-vsctl "$@"

 }
 
 if (ovs_vsctl --version) > /dev/null 2>&1; then :; else
 }
 
 if (ovs_vsctl --version) > /dev/null 2>&1; then :; else

diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
index 50073b6..1ea5398 100644 (file)
--- a/lib/jsonrpc.c
+++ b/lib/jsonrpc.c
@@ -1,5 +1,5 @@
 /*
 /*

- * Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.

  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.


@@ -744,6 +744,7 @@ struct jsonrpc_session {
     struct jsonrpc *rpc;
     struct stream *stream;
     struct pstream *pstream;
     struct jsonrpc *rpc;
     struct stream *stream;
     struct pstream *pstream;

+    int last_error;

     unsigned int seqno;
     uint8_t dscp;
 };
     unsigned int seqno;
     uint8_t dscp;
 };


@@ -752,14 +753,18 @@ struct jsonrpc_session {
  * acceptable to stream_open() or pstream_open().
  *
  * If 'name' is an active connection method, e.g. "tcp:127.1.2.3", the new
  * acceptable to stream_open() or pstream_open().
  *
  * If 'name' is an active connection method, e.g. "tcp:127.1.2.3", the new

- * jsonrpc_session connects and reconnects, with back-off, to 'name'.
+ * jsonrpc_session connects to 'name'.  If 'retry' is true, then the new
+ * session connects and reconnects to 'name', with backoff.  If 'retry' is
+ * false, the new session will only try to connect once and after a connection
+ * failure or a disconnection jsonrpc_session_is_alive() will return false for
+ * the new session.

  *
  * If 'name' is a passive connection method, e.g. "ptcp:", the new
  * jsonrpc_session listens for connections to 'name'.  It maintains at most one
  * connection at any given time.  Any new connection causes the previous one
  * (if any) to be dropped. */
 struct jsonrpc_session *
  *
  * If 'name' is a passive connection method, e.g. "ptcp:", the new
  * jsonrpc_session listens for connections to 'name'.  It maintains at most one
  * connection at any given time.  Any new connection causes the previous one
  * (if any) to be dropped. */
 struct jsonrpc_session *

-jsonrpc_session_open(const char *name)
+jsonrpc_session_open(const char *name, bool retry)

 {
     struct jsonrpc_session *s;
 
 {
     struct jsonrpc_session *s;
 


@@ -772,9 +777,13 @@ jsonrpc_session_open(const char *name)
     s->pstream = NULL;
     s->seqno = 0;
     s->dscp = 0;
     s->pstream = NULL;
     s->seqno = 0;
     s->dscp = 0;

+    s->last_error = 0;

 
     if (!pstream_verify_name(name)) {
         reconnect_set_passive(s->reconnect, true, time_msec());
 
     if (!pstream_verify_name(name)) {
         reconnect_set_passive(s->reconnect, true, time_msec());

+    } else if (!retry) {
+        reconnect_set_max_tries(s->reconnect, 1);
+        reconnect_set_backoff(s->reconnect, INT_MAX, INT_MAX);

     }
 
     if (!stream_or_pstream_needs_probes(name)) {
     }
 
     if (!stream_or_pstream_needs_probes(name)) {


@@ -847,6 +856,8 @@ jsonrpc_session_connect(struct jsonrpc_session *s)
         error = jsonrpc_stream_open(name, &s->stream, s->dscp);
         if (!error) {
             reconnect_connecting(s->reconnect, time_msec());
         error = jsonrpc_stream_open(name, &s->stream, s->dscp);
         if (!error) {
             reconnect_connecting(s->reconnect, time_msec());

+        } else {
+            s->last_error = error;

         }
     } else {
         error = s->pstream ? 0 : jsonrpc_pstream_open(name, &s->pstream,
         }
     } else {
         error = s->pstream ? 0 : jsonrpc_pstream_open(name, &s->pstream,


@@ -908,6 +919,7 @@ jsonrpc_session_run(struct jsonrpc_session *s)
         if (error) {
             reconnect_disconnected(s->reconnect, time_msec(), error);
             jsonrpc_session_disconnect(s);
         if (error) {
             reconnect_disconnected(s->reconnect, time_msec(), error);
             jsonrpc_session_disconnect(s);

+            s->last_error = error;

         }
     } else if (s->stream) {
         int error;
         }
     } else if (s->stream) {
         int error;


@@ -1061,6 +1073,12 @@ jsonrpc_session_get_status(const struct jsonrpc_session *s)
     return s && s->rpc ? jsonrpc_get_status(s->rpc) : 0;
 }
 
     return s && s->rpc ? jsonrpc_get_status(s->rpc) : 0;
 }
 

+int
+jsonrpc_session_get_last_error(const struct jsonrpc_session *s)
+{
+    return s->last_error;
+}
+

 void
 jsonrpc_session_get_reconnect_stats(const struct jsonrpc_session *s,
                                     struct reconnect_stats *stats)
 void
 jsonrpc_session_get_reconnect_stats(const struct jsonrpc_session *s,
                                     struct reconnect_stats *stats)

diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h
index b5acf89..0ae205d 100644 (file)
--- a/lib/jsonrpc.h
+++ b/lib/jsonrpc.h
@@ -1,5 +1,5 @@
 /*
 /*

- * Copyright (c) 2009, 2010, 2012 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2012, 2013 Nicira, Inc.

  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.


@@ -98,7 +98,7 @@ struct json *jsonrpc_msg_to_json(struct jsonrpc_msg *);
 \f
 /* A JSON-RPC session with reconnection. */
 
 \f
 /* A JSON-RPC session with reconnection. */
 

-struct jsonrpc_session *jsonrpc_session_open(const char *name);
+struct jsonrpc_session *jsonrpc_session_open(const char *name, bool retry);

 struct jsonrpc_session *jsonrpc_session_open_unreliably(struct jsonrpc *,
                                                         uint8_t);
 void jsonrpc_session_close(struct jsonrpc_session *);
 struct jsonrpc_session *jsonrpc_session_open_unreliably(struct jsonrpc *,
                                                         uint8_t);
 void jsonrpc_session_close(struct jsonrpc_session *);


@@ -117,6 +117,7 @@ bool jsonrpc_session_is_alive(const struct jsonrpc_session *);
 bool jsonrpc_session_is_connected(const struct jsonrpc_session *);
 unsigned int jsonrpc_session_get_seqno(const struct jsonrpc_session *);
 int jsonrpc_session_get_status(const struct jsonrpc_session *);
 bool jsonrpc_session_is_connected(const struct jsonrpc_session *);
 unsigned int jsonrpc_session_get_seqno(const struct jsonrpc_session *);
 int jsonrpc_session_get_status(const struct jsonrpc_session *);

+int jsonrpc_session_get_last_error(const struct jsonrpc_session *);

 void jsonrpc_session_get_reconnect_stats(const struct jsonrpc_session *,
                                          struct reconnect_stats *);
 
 void jsonrpc_session_get_reconnect_stats(const struct jsonrpc_session *,
                                          struct reconnect_stats *);
 

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 0c644a5..116aa86 100644 (file)
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -157,6 +157,9 @@ static void ovsdb_idl_parse_lock_notify(struct ovsdb_idl *,
  * 'class'.  (Ordinarily 'class' is compiled from an OVSDB schema automatically
  * by ovsdb-idlc.)
  *
  * 'class'.  (Ordinarily 'class' is compiled from an OVSDB schema automatically
  * by ovsdb-idlc.)
  *

+ * Passes 'retry' to jsonrpc_session_open().  See that function for
+ * documentation.
+ *

  * If 'monitor_everything_by_default' is true, then everything in the remote
  * database will be replicated by default.  ovsdb_idl_omit() and
  * ovsdb_idl_omit_alert() may be used to selectively drop some columns from
  * If 'monitor_everything_by_default' is true, then everything in the remote
  * database will be replicated by default.  ovsdb_idl_omit() and
  * ovsdb_idl_omit_alert() may be used to selectively drop some columns from


@@ -168,7 +171,7 @@ static void ovsdb_idl_parse_lock_notify(struct ovsdb_idl *,
  */
 struct ovsdb_idl *
 ovsdb_idl_create(const char *remote, const struct ovsdb_idl_class *class,
  */
 struct ovsdb_idl *
 ovsdb_idl_create(const char *remote, const struct ovsdb_idl_class *class,

-                 bool monitor_everything_by_default)
+                 bool monitor_everything_by_default, bool retry)

 {
     struct ovsdb_idl *idl;
     uint8_t default_mode;
 {
     struct ovsdb_idl *idl;
     uint8_t default_mode;


@@ -180,7 +183,7 @@ ovsdb_idl_create(const char *remote, const struct ovsdb_idl_class *class,
 
     idl = xzalloc(sizeof *idl);
     idl->class = class;
 
     idl = xzalloc(sizeof *idl);
     idl->class = class;

-    idl->session = jsonrpc_session_open(remote);
+    idl->session = jsonrpc_session_open(remote, retry);

     shash_init(&idl->table_by_name);
     idl->tables = xmalloc(class->n_tables * sizeof *idl->tables);
     for (i = 0; i < class->n_tables; i++) {
     shash_init(&idl->table_by_name);
     idl->tables = xmalloc(class->n_tables * sizeof *idl->tables);
     for (i = 0; i < class->n_tables; i++) {


@@ -412,6 +415,18 @@ ovsdb_idl_verify_write_only(struct ovsdb_idl *idl)
 {
     idl->verify_write_only = true;
 }
 {
     idl->verify_write_only = true;
 }

+
+bool
+ovsdb_idl_is_alive(const struct ovsdb_idl *idl)
+{
+    return jsonrpc_session_is_alive(idl->session);
+}
+
+int
+ovsdb_idl_get_last_error(const struct ovsdb_idl *idl)
+{
+    return jsonrpc_session_get_last_error(idl->session);
+}

 \f
 static unsigned char *
 ovsdb_idl_get_mode(struct ovsdb_idl *idl,
 \f
 static unsigned char *
 ovsdb_idl_get_mode(struct ovsdb_idl *idl,

diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
index b428501..6b5e198 100644 (file)
--- a/lib/ovsdb-idl.h
+++ b/lib/ovsdb-idl.h
@@ -44,7 +44,8 @@ struct uuid;
 
 struct ovsdb_idl *ovsdb_idl_create(const char *remote,
                                    const struct ovsdb_idl_class *,
 
 struct ovsdb_idl *ovsdb_idl_create(const char *remote,
                                    const struct ovsdb_idl_class *,

-                                   bool monitor_everything_by_default);
+                                   bool monitor_everything_by_default,
+                                   bool retry);

 void ovsdb_idl_destroy(struct ovsdb_idl *);
 
 void ovsdb_idl_run(struct ovsdb_idl *);
 void ovsdb_idl_destroy(struct ovsdb_idl *);
 
 void ovsdb_idl_run(struct ovsdb_idl *);


@@ -58,6 +59,9 @@ unsigned int ovsdb_idl_get_seqno(const struct ovsdb_idl *);
 bool ovsdb_idl_has_ever_connected(const struct ovsdb_idl *);
 void ovsdb_idl_force_reconnect(struct ovsdb_idl *);
 void ovsdb_idl_verify_write_only(struct ovsdb_idl *);
 bool ovsdb_idl_has_ever_connected(const struct ovsdb_idl *);
 void ovsdb_idl_force_reconnect(struct ovsdb_idl *);
 void ovsdb_idl_verify_write_only(struct ovsdb_idl *);

+
+bool ovsdb_idl_is_alive(const struct ovsdb_idl *);
+int ovsdb_idl_get_last_error(const struct ovsdb_idl *);

 \f
 /* Choosing columns and tables to replicate. */
 
 \f
 /* Choosing columns and tables to replicate. */
 

diff --git a/lib/reconnect.c b/lib/reconnect.c
index 40cc7fc..b914ef6 100644 (file)
--- a/lib/reconnect.c
+++ b/lib/reconnect.c
@@ -1,5 +1,5 @@
 /*
 /*

- * Copyright (c) 2008, 2009, 2010, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2012, 2013 Nicira, Inc.

  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.


@@ -207,7 +207,8 @@ reconnect_get_max_tries(struct reconnect *fsm)
 
 /* Configures the backoff parameters for 'fsm'.  'min_backoff' is the minimum
  * number of milliseconds, and 'max_backoff' is the maximum, between connection
 
 /* Configures the backoff parameters for 'fsm'.  'min_backoff' is the minimum
  * number of milliseconds, and 'max_backoff' is the maximum, between connection

- * attempts.
+ * attempts.  The current backoff is also the duration that 'fsm' is willing to
+ * wait for a given connection to succeed or fail.

  *
  * 'min_backoff' must be at least 1000, and 'max_backoff' must be greater than
  * or equal to 'min_backoff'.
  *
  * 'min_backoff' must be at least 1000, and 'max_backoff' must be greater than
  * or equal to 'min_backoff'.

diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 16c4a76..6e0a6f6 100644 (file)
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -210,7 +210,7 @@ ovsdb_jsonrpc_server_add_remote(struct ovsdb_jsonrpc_server *svr,
     shash_add(&svr->remotes, name, remote);
 
     if (!listener) {
     shash_add(&svr->remotes, name, remote);
 
     if (!listener) {

-        ovsdb_jsonrpc_session_create(remote, jsonrpc_session_open(name));
+        ovsdb_jsonrpc_session_create(remote, jsonrpc_session_open(name, true));

     }
     return remote;
 }
     }
     return remote;
 }

diff --git a/tests/interface-reconfigure.at b/tests/interface-reconfigure.at
index 59507ff..fdfe12e 100644 (file)
--- a/tests/interface-reconfigure.at
+++ b/tests/interface-reconfigure.at
@@ -709,7 +709,7 @@ configure_datapath: bridge      - xenbr2
 configure_datapath: physical    - [u'eth2']
 configure_datapath: extra ports - []
 configure_datapath: extra bonds - []
 configure_datapath: physical    - [u'eth2']
 configure_datapath: extra ports - []
 configure_datapath: extra bonds - []

-/usr/bin/ovs-vsctl --timeout=5 -vconsole:off get-fail-mode xenbr2
+/usr/bin/ovs-vsctl -vconsole:off get-fail-mode xenbr2

 Applying changes to /etc/sysconfig/network-scripts/route-xenbr2 configuration
 Applying changes to /etc/sysconfig/network configuration
 Applying changes to /etc/sysconfig/network-scripts/ifcfg-xenbr2 configuration
 Applying changes to /etc/sysconfig/network-scripts/route-xenbr2 configuration
 Applying changes to /etc/sysconfig/network configuration
 Applying changes to /etc/sysconfig/network-scripts/ifcfg-xenbr2 configuration


@@ -724,7 +724,7 @@ Applying changes to /etc/sysconfig/network-scripts/ifcfg-xenbr2 configuration
     set Bridge xenbr2 fail_mode=secure
     remove Bridge xenbr2 other_config disable-in-band
     br-set-external-id xenbr2 xs-network-uuids d08c8749-0c8f-9e8d-ce25-fd364661ee99
     set Bridge xenbr2 fail_mode=secure
     remove Bridge xenbr2 other_config disable-in-band
     br-set-external-id xenbr2 xs-network-uuids d08c8749-0c8f-9e8d-ce25-fd364661ee99

-/usr/bin/ovs-vsctl --timeout=5 -vconsole:off get interface eth2 ofport
+/usr/bin/ovs-vsctl -vconsole:off get interface eth2 ofport

 /usr/bin/ovs-ofctl add-flow xenbr2 idle_timeout=0,priority=0,in_port=5,arp,nw_proto=1,actions=local
 /usr/bin/ovs-ofctl add-flow xenbr2 idle_timeout=0,priority=0,in_port=local,arp,dl_src=00:15:17:a0:29:80,actions=5
 /usr/bin/ovs-ofctl add-flow xenbr2 idle_timeout=0,priority=0,in_port=5,dl_dst=00:15:17:a0:29:80,actions=local
 /usr/bin/ovs-ofctl add-flow xenbr2 idle_timeout=0,priority=0,in_port=5,arp,nw_proto=1,actions=local
 /usr/bin/ovs-ofctl add-flow xenbr2 idle_timeout=0,priority=0,in_port=local,arp,dl_src=00:15:17:a0:29:80,actions=5
 /usr/bin/ovs-ofctl add-flow xenbr2 idle_timeout=0,priority=0,in_port=5,dl_dst=00:15:17:a0:29:80,actions=local

diff --git a/tests/ovs-monitor-ipsec.at b/tests/ovs-monitor-ipsec.at
index e66c943..bd150cf 100644 (file)
--- a/tests/ovs-monitor-ipsec.at
+++ b/tests/ovs-monitor-ipsec.at
@@ -33,7 +33,7 @@ chmod +x usr/sbin/setkey
 touch etc/racoon/certs/ovs-stale.pem
 
 ovs_vsctl () {
 touch etc/racoon/certs/ovs-stale.pem
 
 ovs_vsctl () {

-    ovs-vsctl --timeout=5 --no-wait -vreconnect:emer --db=unix:socket "$@"
+    ovs-vsctl --no-wait -vreconnect:emer --db=unix:socket "$@"

 }
 trim () {  # Removes blank lines and lines starting with # from input.
     sed -e '/^#/d' -e '/^[       ]*$/d' "$@"
 }
 trim () {  # Removes blank lines and lines starting with # from input.
     sed -e '/^#/d' -e '/^[       ]*$/d' "$@"

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 326d5a4..c5c3f47 100644 (file)
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -15,17 +15,17 @@ dnl RUN_OVS_VSCTL(COMMAND, ...)
 dnl
 dnl Executes each ovs-vsctl COMMAND.
 m4_define([RUN_OVS_VSCTL],
 dnl
 dnl Executes each ovs-vsctl COMMAND.
 m4_define([RUN_OVS_VSCTL],

-  [m4_foreach([command], [$@], [ovs-vsctl --timeout=5 --no-wait -vreconnect:emer --db=unix:socket command
+  [m4_foreach([command], [$@], [ovs-vsctl --no-wait -vreconnect:emer --db=unix:socket command

 ])])
 m4_define([RUN_OVS_VSCTL_ONELINE],
 ])])
 m4_define([RUN_OVS_VSCTL_ONELINE],

-  [m4_foreach([command], [$@], [ovs-vsctl --timeout=5 --no-wait -vreconnect:emer --db=unix:socket --oneline -- command
+  [m4_foreach([command], [$@], [ovs-vsctl --no-wait -vreconnect:emer --db=unix:socket --oneline -- command

 ])])
 
 dnl RUN_OVS_VSCTL_TOGETHER(COMMAND, ...)
 dnl
 dnl Executes each ovs-vsctl COMMAND in a single run of ovs-vsctl.
 m4_define([RUN_OVS_VSCTL_TOGETHER],
 ])])
 
 dnl RUN_OVS_VSCTL_TOGETHER(COMMAND, ...)
 dnl
 dnl Executes each ovs-vsctl COMMAND in a single run of ovs-vsctl.
 m4_define([RUN_OVS_VSCTL_TOGETHER],

-  [ovs-vsctl --timeout=5 --no-wait -vreconnect:emer --db=unix:socket --oneline dnl
+  [ovs-vsctl --no-wait -vreconnect:emer --db=unix:socket --oneline dnl

 m4_foreach([command], [$@], [ -- command])])
 
 dnl CHECK_BRIDGES([BRIDGE, PARENT, VLAN], ...)
 m4_foreach([command], [$@], [ -- command])])
 
 dnl CHECK_BRIDGES([BRIDGE, PARENT, VLAN], ...)


@@ -139,6 +139,40 @@ m4_define([CHECK_IFACES],
 ],
                [], [OVS_VSCTL_CLEANUP])])])
 
 ],
                [], [OVS_VSCTL_CLEANUP])])])
 

+dnl ----------------------------------------------------------------------
+AT_BANNER([ovs-vsctl unit tests])
+
+AT_SETUP([ovs-vsctl connection retry])
+OVS_RUNDIR=$PWD; export OVS_RUNDIR
+
+dnl Without --retry, there should be no retry for active connections.
+AT_CHECK([ovs-vsctl --db=unix:foo --timeout=10 -vreconnect:emer -- init],
+  [1], [], [stderr])
+AT_CHECK([[sed 's/([^()]*)/(...reason...)/' stderr]], [0],
+  [ovs-vsctl: unix:foo: database connection failed (...reason...)
+])
+
+dnl With --retry, we should retry for active connections.
+AT_CHECK(
+  [ovs-vsctl --db=unix:foo --timeout=1 --retry -vreconnect:emer -vPATTERN:console:'%c|%p|%m' -- init
+   echo $? > status],
+  [0], [], [stderr])
+AT_CHECK([grep -c 'terminating with signal' stderr], [0], [1
+])
+AT_CHECK([kill -l `cat status`], [0], [ALRM
+])
+
+dnl Without --retry, we should retry for passive connections.
+AT_CHECK(
+  [ovs-vsctl --db=punix:foo --timeout=1 -vreconnect:emer -vPATTERN:console:'%c|%p|%m' -- init
+   echo $? > status],
+  [0], [], [stderr])
+AT_CHECK([grep -c 'terminating with signal' stderr], [0], [1
+])
+AT_CHECK([kill -l `cat status`], [0], [ALRM
+])
+AT_CLEANUP
+

 dnl ----------------------------------------------------------------------
 AT_BANNER([ovs-vsctl unit tests -- real bridges])
 
 dnl ----------------------------------------------------------------------
 AT_BANNER([ovs-vsctl unit tests -- real bridges])
 


@@ -833,7 +867,7 @@ AT_CHECK(
 
 ])
 m4_define([VSCTL_CHECK_FIND],
 
 ])
 m4_define([VSCTL_CHECK_FIND],

-  [AT_CHECK([echo `ovs-vsctl --bare --timeout=5 --no-wait -vreconnect:emer --db=unix:socket -- --columns=name find bridge '$1' | sort`], [0], [$2
+  [AT_CHECK([echo `ovs-vsctl --bare --no-wait -vreconnect:emer --db=unix:socket -- --columns=name find bridge '$1' | sort`], [0], [$2

 ])])
 
 # Arithmetic relational operators without keys.
 ])])
 
 # Arithmetic relational operators without keys.


@@ -1044,19 +1078,19 @@ AT_SETUP([unreferenced record warnings])
 AT_KEYWORDS([ovs-vsctl])
 OVS_VSCTL_SETUP
 AT_CHECK(
 AT_KEYWORDS([ovs-vsctl])
 OVS_VSCTL_SETUP
 AT_CHECK(

-  [ovs-vsctl -vPATTERN:console:'%c|%p|%m' --timeout=5 --no-wait -vreconnect:emer --db=unix:socket \
+  [ovs-vsctl -vPATTERN:console:'%c|%p|%m' --no-wait -vreconnect:emer --db=unix:socket \

      -- create Bridge name=br0 | $srcdir/uuidfilt.pl],
   [0], [<0>
 ], [vsctl|WARN|applying "create" command to table Bridge without --id option will have no effect
 ], [OVS_VSCTL_CLEANUP])
 AT_CHECK(
      -- create Bridge name=br0 | $srcdir/uuidfilt.pl],
   [0], [<0>
 ], [vsctl|WARN|applying "create" command to table Bridge without --id option will have no effect
 ], [OVS_VSCTL_CLEANUP])
 AT_CHECK(

-  [ovs-vsctl -vPATTERN:console:'%c|%p|%m' --timeout=5 --no-wait -vreconnect:emer --db=unix:socket \
+  [ovs-vsctl -vPATTERN:console:'%c|%p|%m' --no-wait -vreconnect:emer --db=unix:socket \

      -- --id=@br0 create Bridge name=br0 | $srcdir/uuidfilt.pl],
   [0], [<0>
 ], [vsctl|WARN|row id "@br0" was created but no reference to it was inserted, so it will not actually appear in the database
 ], [OVS_VSCTL_CLEANUP])
 AT_CHECK(
      -- --id=@br0 create Bridge name=br0 | $srcdir/uuidfilt.pl],
   [0], [<0>
 ], [vsctl|WARN|row id "@br0" was created but no reference to it was inserted, so it will not actually appear in the database
 ], [OVS_VSCTL_CLEANUP])
 AT_CHECK(

-  [ovs-vsctl -vPATTERN:console:'%c|%p|%m' --timeout=5 --no-wait -vreconnect:emer --db=unix:socket \
+  [ovs-vsctl -vPATTERN:console:'%c|%p|%m' --no-wait -vreconnect:emer --db=unix:socket \

      -- --id=@eth0_iface create Interface name=eth0 \
      -- --id=@eth0 create Port name=eth0 interfaces=@eth0_iface \
      -- --id=@m0 create Mirror name=m0 output_port=@eth0 \
      -- --id=@eth0_iface create Interface name=eth0 \
      -- --id=@eth0 create Port name=eth0 interfaces=@eth0_iface \
      -- --id=@m0 create Mirror name=m0 output_port=@eth0 \

diff --git a/tests/ovs-xapi-sync.at b/tests/ovs-xapi-sync.at
index 29d4737..b55eecd 100644 (file)
--- a/tests/ovs-xapi-sync.at
+++ b/tests/ovs-xapi-sync.at
@@ -22,7 +22,7 @@ mkdir var var/run
 touch var/run/xapi_init_complete.cookie
 
 ovs_vsctl () {
 touch var/run/xapi_init_complete.cookie
 
 ovs_vsctl () {

-    ovs-vsctl --timeout=5 --no-wait -vreconnect:emer --db=unix:socket "$@"
+    ovs-vsctl --no-wait -vreconnect:emer --db=unix:socket "$@"

 }
 
 # Start ovsdb-server.
 }
 
 # Start ovsdb-server.

diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index d448109..81b5f9f 100644 (file)
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -1,5 +1,5 @@
 /*
 /*

- * Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.

  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.


@@ -1880,7 +1880,7 @@ do_idl(int argc, char *argv[])
 
     idltest_init();
 
 
     idltest_init();
 

-    idl = ovsdb_idl_create(argv[1], &idltest_idl_class, true);
+    idl = ovsdb_idl_create(argv[1], &idltest_idl_class, true, true);

     if (argc > 2) {
         struct stream *stream;
 
     if (argc > 2) {
         struct stream *stream;
 

diff --git a/utilities/bugtool/ovs-bugtool-vsctl-show b/utilities/bugtool/ovs-bugtool-vsctl-show
index fe433d8..26e46e5 100755 (executable)
--- a/utilities/bugtool/ovs-bugtool-vsctl-show
+++ b/utilities/bugtool/ovs-bugtool-vsctl-show
@@ -14,6 +14,6 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
 # USA
 #
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
 # USA
 #

-# Copyright (C) 2012 Nicira, Inc.
+# Copyright (C) 2012, 2013 Nicira, Inc.

 
 

-ovs-vsctl --timeout=5 show
+ovs-vsctl show

diff --git a/utilities/ovs-ctl.in b/utilities/ovs-ctl.in
index 6bad187..5ad5f26 100755 (executable)
--- a/utilities/ovs-ctl.in
+++ b/utilities/ovs-ctl.in
@@ -57,7 +57,7 @@ insert_mod_if_required () {
 }
 
 ovs_vsctl () {
 }
 
 ovs_vsctl () {

-    ovs-vsctl --no-wait --timeout=5 "$@"
+    ovs-vsctl --no-wait "$@"

 }
 
 ovsdb_tool () {
 }
 
 ovsdb_tool () {

diff --git a/utilities/ovs-save b/utilities/ovs-save
index 16ac879..b46f98d 100755 (executable)
--- a/utilities/ovs-save
+++ b/utilities/ovs-save
@@ -1,6 +1,6 @@
 #! /bin/sh
 
 #! /bin/sh
 

-# Copyright (c) 2011 Nicira, Inc.
+# Copyright (c) 2011, 2013 Nicira, Inc.

 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.


@@ -177,7 +177,7 @@ save_flows () {
 }
 
 ovs_vsctl () {
 }
 
 ovs_vsctl () {

-    ovs-vsctl --no-wait --timeout=1 "$@"
+    ovs-vsctl --no-wait "$@"

 }
 
 save_ofports ()
 }
 
 save_ofports ()


@@ -191,7 +191,7 @@ save_ofports ()
         count=0
         for iface in `ovs_vsctl list-ifaces ${bridge}`; do
             ofport=`ovs_vsctl get interface ${iface} ofport`
         count=0
         for iface in `ovs_vsctl list-ifaces ${bridge}`; do
             ofport=`ovs_vsctl get interface ${iface} ofport`

-            [ "${count}" -eq 0 ] && cmd="ovs-vsctl --no-wait --timeout=1"
+            [ "${count}" -eq 0 ] && cmd="ovs-vsctl --no-wait"

             cmd="${cmd} -- --if-exists set interface "${iface}" \
                      ofport_request="${ofport}""
 
             cmd="${cmd} -- --if-exists set interface "${iface}" \
                      ofport_request="${ofport}""
 

diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index cf6cf88..85149a9 100644 (file)
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -127,6 +127,20 @@ to approximately \fIsecs\fR seconds.  If the timeout expires,
 would normally happen only if the database cannot be contacted, or if
 the system is overloaded.)
 .
 would normally happen only if the database cannot be contacted, or if
 the system is overloaded.)
 .

+.IP "\fB\-\-retry\fR"
+Without this option, if \fBovs\-vsctl\fR connects outward to the
+database server (the default) then \fBovs\-vsctl\fR will try to
+connect once and exit with an error if the connection fails (which
+usually means that \fBovsdb\-server\fR is not running).
+.IP
+With this option, or if \fB\-\-db\fR specifies that \fBovs\-vsctl\fR
+should listen for an incoming connection from the database server,
+then \fBovs\-vsctl\fR will wait for a connection to the database
+forever.
+.IP
+Regardless of this setting, \fB\-\-timeout\fR always limits how long
+\fBovs\-vsctl\fR will wait.
+.

 .SS "Table Formatting Options"
 These options control the format of output from the \fBlist\fR and
 \fBfind\fR commands.
 .SS "Table Formatting Options"
 These options control the format of output from the \fBlist\fR and
 \fBfind\fR commands.

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 1ba8588..18ace60 100644 (file)
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -114,6 +114,16 @@ static bool wait_for_reload = true;
 /* --timeout: Time to wait for a connection to 'db'. */
 static int timeout;
 
 /* --timeout: Time to wait for a connection to 'db'. */
 static int timeout;
 

+/* --retry: If true, ovs-vsctl will retry connecting to the database forever.
+ * If false and --db says to use an active connection method (e.g. "unix:",
+ * "tcp:", "ssl:"), then ovs-vsctl will try to connect once and exit with an
+ * error if the database server cannot be contacted (e.g. ovsdb-server is not
+ * running).
+ *
+ * Regardless of this setting, --timeout always limits how long ovs-vsctl will
+ * wait. */
+static bool retry;
+

 /* Format for table output. */
 static struct table_style table_style = TABLE_STYLE_DEFAULT;
 
 /* Format for table output. */
 static struct table_style table_style = TABLE_STYLE_DEFAULT;
 


@@ -186,7 +196,7 @@ main(int argc, char *argv[])
     }
 
     /* Initialize IDL. */
     }
 
     /* Initialize IDL. */

-    idl = the_idl = ovsdb_idl_create(db, &ovsrec_idl_class, false);
+    idl = the_idl = ovsdb_idl_create(db, &ovsrec_idl_class, false, retry);

     run_prerequisites(commands, n_commands, idl);
 
     /* Execute the commands.
     run_prerequisites(commands, n_commands, idl);
 
     /* Execute the commands.


@@ -199,6 +209,11 @@ main(int argc, char *argv[])
     seqno = ovsdb_idl_get_seqno(idl);
     for (;;) {
         ovsdb_idl_run(idl);
     seqno = ovsdb_idl_get_seqno(idl);
     for (;;) {
         ovsdb_idl_run(idl);

+        if (!ovsdb_idl_is_alive(idl)) {
+            int retval = ovsdb_idl_get_last_error(idl);
+            vsctl_fatal("%s: database connection failed (%s)",
+                        db, ovs_retval_to_string(retval));
+        }

 
         if (seqno != ovsdb_idl_get_seqno(idl)) {
             seqno = ovsdb_idl_get_seqno(idl);
 
         if (seqno != ovsdb_idl_get_seqno(idl)) {
             seqno = ovsdb_idl_get_seqno(idl);


@@ -247,6 +262,7 @@ parse_options(int argc, char *argv[], struct shash *local_options)
         OPT_DRY_RUN,
         OPT_PEER_CA_CERT,
         OPT_LOCAL,
         OPT_DRY_RUN,
         OPT_PEER_CA_CERT,
         OPT_LOCAL,

+        OPT_RETRY,

         VLOG_OPTION_ENUMS,
         TABLE_OPTION_ENUMS
     };
         VLOG_OPTION_ENUMS,
         TABLE_OPTION_ENUMS
     };


@@ -257,6 +273,7 @@ parse_options(int argc, char *argv[], struct shash *local_options)
         {"dry-run", no_argument, NULL, OPT_DRY_RUN},
         {"oneline", no_argument, NULL, OPT_ONELINE},
         {"timeout", required_argument, NULL, 't'},
         {"dry-run", no_argument, NULL, OPT_DRY_RUN},
         {"oneline", no_argument, NULL, OPT_ONELINE},
         {"timeout", required_argument, NULL, 't'},

+        {"retry", no_argument, NULL, OPT_RETRY},

         {"help", no_argument, NULL, 'h'},
         {"version", no_argument, NULL, 'V'},
         VLOG_LONG_OPTIONS,
         {"help", no_argument, NULL, 'h'},
         {"version", no_argument, NULL, 'V'},
         VLOG_LONG_OPTIONS,


@@ -384,6 +401,10 @@ parse_options(int argc, char *argv[], struct shash *local_options)
             }
             break;
 
             }
             break;
 

+        case OPT_RETRY:
+            retry = true;
+            break;
+

         VLOG_OPTION_HANDLERS
         TABLE_OPTION_HANDLERS(&table_style)
 
         VLOG_OPTION_HANDLERS
         TABLE_OPTION_HANDLERS(&table_style)
 


@@ -662,6 +683,7 @@ Options:\n\
   --db=DATABASE               connect to DATABASE\n\
                               (default: %s)\n\
   --no-wait                   do not wait for ovs-vswitchd to reconfigure\n\
   --db=DATABASE               connect to DATABASE\n\
                               (default: %s)\n\
   --no-wait                   do not wait for ovs-vswitchd to reconfigure\n\

+  --retry                     keep trying to connect to server forever\n\

   -t, --timeout=SECS          wait at most SECS seconds for ovs-vswitchd\n\
   --dry-run                   do not commit changes to database\n\
   --oneline                   print exactly one line of output per command\n",
   -t, --timeout=SECS          wait at most SECS seconds for ovs-vswitchd\n\
   --dry-run                   do not commit changes to database\n\
   --oneline                   print exactly one line of output per command\n",

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 1fd431c..9449879 100644 (file)
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -318,7 +318,7 @@ void
 bridge_init(const char *remote)
 {
     /* Create connection to database. */
 bridge_init(const char *remote)
 {
     /* Create connection to database. */

-    idl = ovsdb_idl_create(remote, &ovsrec_idl_class, true);
+    idl = ovsdb_idl_create(remote, &ovsrec_idl_class, true, true);

     idl_seqno = ovsdb_idl_get_seqno(idl);
     ovsdb_idl_set_lock(idl, "ovs_vswitchd");
     ovsdb_idl_verify_write_only(idl);
     idl_seqno = ovsdb_idl_get_seqno(idl);
     ovsdb_idl_set_lock(idl, "ovs_vswitchd");
     ovsdb_idl_verify_write_only(idl);

diff --git a/xenserver/etc_xapi.d_plugins_openvswitch-cfg-update b/xenserver/etc_xapi.d_plugins_openvswitch-cfg-update
index 02927f8..2df838a 100755 (executable)
--- a/xenserver/etc_xapi.d_plugins_openvswitch-cfg-update
+++ b/xenserver/etc_xapi.d_plugins_openvswitch-cfg-update
@@ -4,7 +4,7 @@
 # ovs-vswitchd configuration that are managed in the xapi database when 
 # integrated with Citrix management tools.
 
 # ovs-vswitchd configuration that are managed in the xapi database when 
 # integrated with Citrix management tools.
 

-# Copyright (C) 2009, 2010, 2011, 2012 Nicira, Inc.
+# Copyright (C) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.

 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.


@@ -213,7 +213,7 @@ def setControllerCfg(controller):
                    "--", "set-manager", 'ssl:' + controller + ':6632'])
 
 def vswitchCfgQuery(action_args):
                    "--", "set-manager", 'ssl:' + controller + ':6632'])
 
 def vswitchCfgQuery(action_args):

-    cmd = [vsctl, "--timeout=5", "-vconsole:off"] + action_args
+    cmd = [vsctl, "-vconsole:off"] + action_args

     output = subprocess.Popen(cmd, stdout=subprocess.PIPE).communicate()
     if len(output) == 0 or output[0] == None:
         output = ""
     output = subprocess.Popen(cmd, stdout=subprocess.PIPE).communicate()
     if len(output) == 0 or output[0] == None:
         output = ""

diff --git a/xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py b/xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py
index 971f918..1379fb4 100644 (file)
--- a/xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py
+++ b/xenserver/opt_xensource_libexec_InterfaceReconfigureVswitch.py
@@ -1,5 +1,5 @@
 # Copyright (c) 2008,2009,2011 Citrix Systems, Inc.
 # Copyright (c) 2008,2009,2011 Citrix Systems, Inc.

-# Copyright (c) 2009,2010,2011,2012 Nicira, Inc.
+# Copyright (c) 2009,2010,2011,2012,2013 Nicira, Inc.

 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU Lesser General Public License as published
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU Lesser General Public License as published


@@ -721,7 +721,7 @@ class DatapathVswitch(Datapath):
 
 def vswitchCfgQuery(action_args):
     cmd = ['%s/usr/bin/ovs-vsctl' % root_prefix(),
 
 def vswitchCfgQuery(action_args):
     cmd = ['%s/usr/bin/ovs-vsctl' % root_prefix(),

-        '--timeout=5', '-vconsole:off'] + action_args
+           '-vconsole:off'] + action_args

     output = subprocess.Popen(cmd, stdout=subprocess.PIPE).communicate()
     if len(output) == 0 or output[0] == None:
         output = ""
     output = subprocess.Popen(cmd, stdout=subprocess.PIPE).communicate()
     if len(output) == 0 or output[0] == None:
         output = ""

diff --git a/xenserver/usr_lib_xsconsole_plugins-base_XSFeatureVSwitch.py b/xenserver/usr_lib_xsconsole_plugins-base_XSFeatureVSwitch.py
index f62eaa8..fdbbc0e 100644 (file)
--- a/xenserver/usr_lib_xsconsole_plugins-base_XSFeatureVSwitch.py
+++ b/xenserver/usr_lib_xsconsole_plugins-base_XSFeatureVSwitch.py
@@ -1,5 +1,5 @@
+# Copyright (c) 2009,2010,2011,2012,2013 Nicira, Inc.

 # Copyright (c) 2007-2011 Citrix Systems Inc.
 # Copyright (c) 2007-2011 Citrix Systems Inc.

-# Copyright (c) 2009,2010,2011,2012 Nicira, Inc.

 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by


@@ -86,7 +86,7 @@ class VSwitchConfig:
     @staticmethod
     def Get(action):
         try:
     @staticmethod
     def Get(action):
         try:

-            arg = [vsctl, "--timeout=30", "-vconsole:off"] + action.split()
+            arg = [vsctl, "-vconsole:off"] + action.split()

             output = ShellPipe(arg).Stdout()
         except StandardError, e:
             XSLogError("config retrieval error: " + str(e))
             output = ShellPipe(arg).Stdout()
         except StandardError, e:
             XSLogError("config retrieval error: " + str(e))