[ovs-dev,2/3] ovn-sbctl: Use environment var OVN_SB_DB to find the database by default.
diff mbox

Message ID 1444251436-26967-2-git-send-email-blp@nicira.com
State Accepted
Headers show

Commit Message

Ben Pfaff Oct. 7, 2015, 8:57 p.m. UTC
This makes it possible to use ovn-sbctl without always typing the --db
option (outside of trivial single-machine OVN deployments).

Also modifies the testsuite to use this.

Signed-off-by: Ben Pfaff <blp@nicira.com>
---
 ovn/utilities/ovn-sbctl.8.in | 11 +++++++----
 ovn/utilities/ovn-sbctl.c    | 16 +++++++++++++++-
 tests/ofproto-macros.at      | 13 ++++++++-----
 tests/ovn.at                 |  2 +-
 utilities/ovs-sim.in         |  5 +++--
 5 files changed, 34 insertions(+), 13 deletions(-)

Comments

Justin Pettit Oct. 7, 2015, 9:11 p.m. UTC | #1
> On Oct 7, 2015, at 1:57 PM, Ben Pfaff <blp@nicira.com> wrote:
> 
> # ovn_init_db DATABASE
> #
> -# Creates and initializes the given DATABASE (one of "ovn-sb" or "ovn-nb")
> -# and starts its ovsdb-server instance.
> +# Creates and initializes the given DATABASE (one of "ovn-sb" or "ovn-nb"),
> +# starts its ovsdb-server instance, and sets the appropriate environment
> +# variable so that ovn-sbctl or ovn-nbctl uses the database by default.

Do you think it's worth mentioning what the appropriate environment variable is?

Acked-by: Justin Pettit <jpettit@nicira.com>

--Justin
Ben Pfaff Oct. 7, 2015, 9:29 p.m. UTC | #2
On Wed, Oct 07, 2015 at 02:11:04PM -0700, Justin Pettit wrote:
> 
> > On Oct 7, 2015, at 1:57 PM, Ben Pfaff <blp@nicira.com> wrote:
> > 
> > # ovn_init_db DATABASE
> > #
> > -# Creates and initializes the given DATABASE (one of "ovn-sb" or "ovn-nb")
> > -# and starts its ovsdb-server instance.
> > +# Creates and initializes the given DATABASE (one of "ovn-sb" or "ovn-nb"),
> > +# starts its ovsdb-server instance, and sets the appropriate environment
> > +# variable so that ovn-sbctl or ovn-nbctl uses the database by default.
> 
> Do you think it's worth mentioning what the appropriate environment
> variable is?

That's appropriate.

I added a parenthetical (OVN_SB_DB or OVN_NB_DB).

> Acked-by: Justin Pettit <jpettit@nicira.com>

Thanks, I'll apply this soon.

Patch
diff mbox

diff --git a/ovn/utilities/ovn-sbctl.8.in b/ovn/utilities/ovn-sbctl.8.in
index 7e68d68..9c3706d 100644
--- a/ovn/utilities/ovn-sbctl.8.in
+++ b/ovn/utilities/ovn-sbctl.8.in
@@ -56,10 +56,13 @@  command line has options, then those options must be separated from
 the global options by \fB\-\-\fR.
 .
 .IP "\fB\-\-db=\fIserver\fR"
-Sets \fIserver\fR as the database server that \fBovn\-sbctl\fR
-contacts to query or modify configuration.  The default is
-\fBunix:@RUNDIR@/db.sock\fR.  \fIserver\fR must take one of the
-following forms:
+The OVSDB database remote to contact.  If the \fBOVN_SB_DB\fR
+environment variable is set, its value is used as the default.
+Otherwise, the default is \fBunix:@RUNDIR@/db.sock\fR, but this
+default is unlikely to be useful outside of single-machine OVN test
+environments.
+.IP
+\fIserver\fR must take one of the following forms:
 .RS
 .so ovsdb/remote-active.man
 .so ovsdb/remote-passive.man
diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c
index f7cb156..29aaf47 100644
--- a/ovn/utilities/ovn-sbctl.c
+++ b/ovn/utilities/ovn-sbctl.c
@@ -77,6 +77,7 @@  OVS_NO_RETURN static void sbctl_exit(int status);
 static void sbctl_cmd_init(void);
 OVS_NO_RETURN static void usage(void);
 static void parse_options(int argc, char *argv[], struct shash *local_options);
+static const char *sbctl_default_db(void);
 static void run_prerequisites(struct ctl_command[], size_t n_commands,
                               struct ovsdb_idl *);
 static void do_sbctl(const char *args, struct ctl_command *, size_t n,
@@ -147,6 +148,19 @@  main(int argc, char *argv[])
     }
 }
 
+static const char *
+sbctl_default_db(void)
+{
+    static char *def;
+    if (!def) {
+        def = getenv("OVN_SB_DB");
+        if (!def) {
+            def = ctl_default_db();
+        }
+    }
+    return def;
+}
+
 static void
 parse_options(int argc, char *argv[], struct shash *local_options)
 {
@@ -270,7 +284,7 @@  parse_options(int argc, char *argv[], struct shash *local_options)
     free(short_options);
 
     if (!db) {
-        db = ctl_default_db();
+        db = sbctl_default_db();
     }
 
     for (i = n_global_long_options; options[i].name; i++) {
diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
index a5aee3a..6ca4ab4 100644
--- a/tests/ofproto-macros.at
+++ b/tests/ofproto-macros.at
@@ -110,8 +110,9 @@  as() {
 
 # ovn_init_db DATABASE
 #
-# Creates and initializes the given DATABASE (one of "ovn-sb" or "ovn-nb")
-# and starts its ovsdb-server instance.
+# Creates and initializes the given DATABASE (one of "ovn-sb" or "ovn-nb"),
+# starts its ovsdb-server instance, and sets the appropriate environment
+# variable so that ovn-sbctl or ovn-nbctl uses the database by default.
 #
 # Usually invoked from ovn_start.
 ovn_init_db () {
@@ -121,18 +122,20 @@  ovn_init_db () {
     : > "$d"/.$1.db.~lock~
     as $1 ovsdb-tool create "$d"/$1.db "$abs_top_srcdir"/ovn/$1.ovsschema
     as $1 start_daemon ovsdb-server --remote=punix:"$d"/$1.sock "$d"/$1.db
+    local var=`echo $1_db | tr a-z- A-Z_`
+    AS_VAR_SET([$var], [unix:$ovs_base/$1/$1.sock]); export $var
 }
 
 # ovn_start
 #
 # Creates and initializes ovn-sb and ovn-nb databases and starts their
-# ovsdb-server instance, and starts ovn-northd running against them.
+# ovsdb-server instance, sets appropriate environment variables so that
+# ovn-sbctl and ovn-nbctl use them by default, and starts ovn-northd running
+# against them.
 ovn_start () {
     ovn_init_db ovn-sb
     ovn_init_db ovn-nb
 
-    OVN_NB_DB=unix:$ovs_base/ovn-nb/ovn-nb.sock; export OVN_NB_DB
-
     echo "starting ovn-northd"
     mkdir "$ovs_base"/northd
     as northd start_daemon ovn-northd \
diff --git a/tests/ovn.at b/tests/ovn.at
index 9bf8022..1eb6d0b 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -541,7 +541,7 @@  ovn_populate_arp
 # Allow some time for ovn-northd and ovn-controller to catch up.
 # XXX This should be more systematic.
 sleep 1
-ovn-sbctl --db=unix:`pwd`/ovn-sb/ovn-sb.sock dump-flows -- list multicast_group
+ovn-sbctl dump-flows -- list multicast_group
 
 # test_packet INPORT DST SRC ETHTYPE OUTPORT...
 #
diff --git a/utilities/ovs-sim.in b/utilities/ovs-sim.in
index 2d9d66d..1702f0e 100755
--- a/utilities/ovs-sim.in
+++ b/utilities/ovs-sim.in
@@ -261,11 +261,12 @@  EOF
     done
 
     OVN_NB_DB=unix:$sim_base/ovn-nb/ovn-nb.sock; export OVN_NB_DB
+    OVN_SB_DB=unix:$sim_base/ovn-sb/ovn-sb.sock; export OVN_SB_DB
 
     mkdir "$sim_base"/northd
     as northd ovn-northd $daemon_opts \
-	       --ovnnb-db=unix:"$sim_base"/ovn-nb/ovn-nb.sock \
-	       --ovnsb-db=unix:"$sim_base"/ovn-sb/ovn-sb.sock
+	       --ovnnb-db="$OVN_NB_DB" \
+	       --ovnsb-db="$OVN_SB_DB"
 }
 export -f ovn_start