ovs-vsctl: Fix bugs.

The full ovs-vsctl test suite now passes.

tests/ovs-vsctl.at

@@ -130,7 +130,9 @@ m4_define([CHECK_IFACES],
      [RUN_OVS_VSCTL([list-ifaces $1])],
      [0],
      [m4_foreach([iface], m4_cdr($@), [iface
-])])
+])],
+     [],
+     [OVS_VSCTL_CLEANUP])
    AT_CHECK([RUN_OVS_VSCTL([iface-to-br $1])], [1], [],
             [ovs-vsctl: no interface named $1
 ],
@@ -269,31 +271,23 @@ CHECK_PORTS([a])
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
-m4_define([ovs_vsctl_disabled_tests], [
 dnl ----------------------------------------------------------------------
 AT_BANNER([ovs-vsctl unit tests -- fake bridges])
 
-m4_define([SIMPLE_FAKE_CONF], [dnl
-bridge.xenbr0.port=eth0
-bridge.xenbr0.port=eth0.9
-bridge.xenbr0.port=xapi1
-bridge.xenbr0.port=xenbr0
-iface.xapi1.fake-bridge=true
-iface.xapi1.internal=true
-vlan.eth0.9.tag=9
-vlan.xapi1.tag=9
-])
+m4_define([OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF],
+  [AT_CHECK(
+     [RUN_OVS_VSCTL(
+        [add-br xenbr0],
+        [add-port xenbr0 eth0],
+        [add-br xapi1 xenbr0 9],
+        [add-port xapi1 eth0.9])],
+     [0], [], [], [OVS_VSCTL_CLEANUP])])
 
 AT_SETUP([simple fake bridge])
 AT_KEYWORDS([ovs-vsctl fake-bridge])
 OVS_VSCTL_SETUP
-AT_CHECK([RUN_OVS_VSCTL(
-  [add-br xenbr0],
-  [add-port xenbr0 eth0],
-  [add-br xapi1 xenbr0 9],
-  [add-port xapi1 eth0.9])])
-AT_CHECK([cat conf], [0], [SIMPLE_FAKE_CONF])
-CHECK_BRIDGES([xenbr0, xenbr0, 0], [xapi1, xenbr0, 9])
+OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF
+CHECK_BRIDGES([xapi1, xenbr0, 9], [xenbr0, xenbr0, 0])
 CHECK_PORTS([xenbr0], [eth0])
 CHECK_IFACES([xenbr0], [eth0])
 CHECK_PORTS([xapi1], [eth0.9])
@@ -304,12 +298,8 @@ AT_CLEANUP
 AT_SETUP([simple fake bridge + del-br fake bridge])
 AT_KEYWORDS([ovs-vsctl fake-bridge])
 OVS_VSCTL_SETUP
-AT_DATA([conf], [SIMPLE_FAKE_CONF])
-AT_CHECK([RUN_OVS_VSCTL([del-br xapi1])])
-AT_CHECK([cat conf], [0], [dnl
-bridge.xenbr0.port=eth0
-bridge.xenbr0.port=xenbr0
-])
+OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF
+AT_CHECK([RUN_OVS_VSCTL([del-br xapi1])], [0], [], [], [OVS_VSCTL_CLEANUP])
 CHECK_BRIDGES([xenbr0, xenbr0, 0])
 CHECK_PORTS([xenbr0], [eth0])
 CHECK_IFACES([xenbr0], [eth0])
@@ -319,35 +309,25 @@ AT_CLEANUP
 AT_SETUP([simple fake bridge + del-br real bridge])
 AT_KEYWORDS([ovs-vsctl fake-bridge])
 OVS_VSCTL_SETUP
-AT_DATA([conf], [SIMPLE_FAKE_CONF])
-AT_CHECK([RUN_OVS_VSCTL([del-br xenbr0])])
-AT_CHECK([cat conf], [0], [])
+OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF
+AT_CHECK([RUN_OVS_VSCTL([del-br xenbr0])], [0], [], [], [OVS_VSCTL_CLEANUP])
 CHECK_BRIDGES
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
-m4_define([BOND_FAKE_CONF], [dnl
-bonding.bond0.slave=eth0
-bonding.bond0.slave=eth1
-bridge.xapi1.port=bond0
-bridge.xapi1.port=bond0.11
-bridge.xapi1.port=xapi1
-bridge.xapi1.port=xapi2
-iface.xapi2.fake-bridge=true
-iface.xapi2.internal=true
-vlan.bond0.11.tag=11
-vlan.xapi2.tag=11
-])
+m4_define([OVS_VSCTL_SETUP_BOND_FAKE_CONF],
+  [AT_CHECK(
+     [RUN_OVS_VSCTL(
+        [add-br xapi1],
+        [add-bond xapi1 bond0 eth0 eth1],
+        [add-br xapi2 xapi1 11],
+        [add-port xapi2 bond0.11])],
+     [0], [], [], [OVS_VSCTL_CLEANUP])])
 
 AT_SETUP([fake bridge on bond])
 AT_KEYWORDS([ovs-vsctl fake-bridge])
 OVS_VSCTL_SETUP
-AT_CHECK([RUN_OVS_VSCTL(
-  [add-br xapi1],
-  [add-bond xapi1 bond0 eth0 eth1],
-  [add-br xapi2 xapi1 11],
-  [add-port xapi2 bond0.11])])
-AT_CHECK([cat conf], [0], [BOND_FAKE_CONF])
+OVS_VSCTL_SETUP_BOND_FAKE_CONF
 CHECK_BRIDGES([xapi1, xapi1, 0], [xapi2, xapi1, 11])
 CHECK_PORTS([xapi1], [bond0])
 CHECK_IFACES([xapi1], [eth0], [eth1])
@@ -359,9 +339,9 @@ AT_CLEANUP
 AT_SETUP([fake bridge on bond + del-br fake bridge])
 AT_KEYWORDS([ovs-vsctl fake-bridge])
 OVS_VSCTL_SETUP
-AT_DATA([conf], [BOND_FAKE_CONF])
+OVS_VSCTL_SETUP_BOND_FAKE_CONF
 AT_CHECK([RUN_OVS_VSCTL([--oneline del-br xapi2])], [0], [
-])
+], [], [OVS_VSCTL_CLEANUP])
 CHECK_BRIDGES([xapi1, xapi1, 0])
 CHECK_PORTS([xapi1], [bond0])
 CHECK_IFACES([xapi1], [eth0], [eth1])
@@ -370,9 +350,9 @@ AT_CLEANUP
 
 AT_SETUP([fake bridge on bond + del-br real bridge])
 AT_KEYWORDS([ovs-vsctl fake-bridge])
-AT_DATA([conf], [BOND_FAKE_CONF])
+OVS_VSCTL_SETUP
+OVS_VSCTL_SETUP_BOND_FAKE_CONF
 AT_CHECK([RUN_OVS_VSCTL([del-br xapi1])])
 CHECK_BRIDGES
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
-])

utilities/ovs-vsctl.8.in

@@ -15,20 +15,20 @@ ovs\-vsctl \- utility for querying and configuring \fBovs\-vswitchd\fR
 [\fB\-\-\fR \fIcommand \fR[\fIargs\fR\&...]]
 .
 .SH DESCRIPTION
-The \fBovs\-vsctl\fR program configures \fBovs\-vswitchd\fR(8), mainly
-by providing a high\-level interface to editing its configuration file
-\fBovs\-vswitchd.conf\fR(5).  This program is mainly intended for use
-when \fBovs\-vswitchd\fR is running, but it can also be used when
-\fBovs\-vswitchd\fR is not running.  In the latter case configuration
-changes will only take effect when \fBovs\-vswitchd\fR is started.
+The \fBovs\-vsctl\fR program configures \fBovs\-vswitchd\fR(8) by
+providing a high\-level interface to editing its configuration
+database.  This program is mainly intended for use when
+\fBovs\-vswitchd\fR is running.  If it is used when
+\fBovs\-vswitchd\fR is not running, then \fB\-\-no\-wait\fR should be
+specified and configuration changes will only take effect when
+\fBovs\-vswitchd\fR is started.
 .PP
-By default, each time \fBovs\-vsctl\fR runs, it examines and,
-depending on the requested command or commands, possibly applies
-changes to an
-\fBovs\-vswitchd.conf\fR file.  Then, if it applied any changes and if
-\fBovs\-vswitchd\fR is running, it tells \fBovs\-vswitchd\fR to reload
-the modified configuration file and waits for the reload to complete
-before exiting.
+By default, each time \fBovs\-vsctl\fR runs, it connects to an
+\fBovsdb\-server\fR process that maintains an Open vSwitch
+configuration database.  Using this connection, it queries and
+possibly applies changes to the database, depending on the supplied
+commands.  Then, if it applied any changes, it waits until
+\fBovs\-vswitchd\fR has finished reconfiguring itself before it exits.
 .
 .SS "Linux VLAN Bridging Compatibility"
 The \fBovs\-vsctl\fR program supports the model of a bridge
@@ -54,46 +54,34 @@ they are members.
 The following options affect the general outline of \fBovs\-vsctl\fR's
 activities:
 .
-.IP "\fB\-c \fIfile\fR"
-.IQ "\fB\-\-config=\fIfile\fR"
-Sets the configuration file that \fBovs\-vsctl\fR reads and possibly
-modifies.  The default is \fB@localstatedir@/ovs\-vswitchd.conf\fR.
+.IP "\fB\-\-db=\fIserver\fR"
+Sets \fIserver\fR as the database server that \fBovs\-vsctl\fR
+contacts to query or modify configuration.  The default is
+\fBunix:@RUNDIR@/ovsdb\-server\fR.  \fIserver\fR must take one of the
+following forms:
+.RS
+.IP "\fBtcp:\fIip\fB:\fIport\fR"
+Connect to the given TCP \fIport\fR on \fIip\fR.
+.IP "\fBunix:\fIfile\fR"
+Connect to the Unix domain server socket named \fIfile\fR.
+.RE
+.IP "\fB\-\-no\-wait\fR"
+Prevents \fBovs\-vsctl\fR from waiting for \fBovs\-vswitchd\fR to
+reconfigure itself according to the the modified database.  This
+option should be used if \fBovs\-vswitchd\fR is not running;
+otherwise, \fBovs-vsctl\fR will not exit until \fBovs-vswitchd\fR
+starts.
 .IP
-If \fIfile\fR is specified as \fB\-\fR, then \fBovs\-vsctl\fR reads
-the configuration file from standard input and, for commands that
-modify the configuration, writes the new one to standard output.  This
-is useful for testing but it should not be used in production because
-it bypasses the Open vSwitch configuration file locking protocol.
-.
-.IP "\fB\-t \fItarget\fR"
-.IQ "\fB\-\-target=\fItarget\fR"
-Configures how \fBovs\-vsctl\fR contacts \fBovs\-vswitchd\fR to
-instruct it to reload its configuration file.
-.IP
-If \fItarget\fR begins with \fB/\fR it must name a Unix domain socket
-on which \fBovs\-vswitchd\fR is listening for control channel
-connections.  By default, \fBovs\-vswitchd\fR listens on a Unix domain
-socket named \fB@RUNDIR@/ovs\-vswitchd.\fIpid\fB.ctl\fR, where
-\fIpid\fR is \fBovs\-vswitchd\fR's process ID.
-.IP
-Otherwise, \fBovs\-appctl\fR looks for a pidfile, that is, a file
-whose contents are the process ID of a running process as a decimal
-number, named \fB@RUNDIR@/\fItarget\fB.pid\fR.  (The \fB\-\-pidfile\fR
-option makes an Open vSwitch daemon create a pidfile.)
-\fBovs\-appctl\fR reads the pidfile, then looks for a Unix socket
-named \fB@RUNDIR@/\fItarget\fB.\fIpid\fB.ctl\fR, where \fIpid\fR is
-replaced by the process ID read from \fItarget\fR, and uses that file
-as if it had been specified directly as the target.
-.IP
-The default target is \fBovs\-vswitchd\fR.
-.IP "\fB\-\-no\-reload\fR"
-Prevents \fBovs\-vsctl\fR from telling \fBovs\-vswitchd\fR to reload
-its configuration file.
+This option has no effect if the commands specified do not change the
+database.
 .
 .IP "\fB\-\-no\-syslog\fR"
 By default, \fBovs\-vsctl\fR logs its arguments and the details of any
 changes that it makes to the system log.  This option disables this
 logging.
+.IP
+This option is equivalent to \fB\-\-verbose=vvsctl:syslog:warn\fR.
+.
 .IP "\fB\-\-oneline\fR"
 Modifies the output format so that the output for each command is printed
 on a single line.  New-line characters that would otherwise separate
@@ -101,6 +89,8 @@ lines are printed as \fB\\n\fR, and any instances of \fB\\\fR that
 would otherwise appear in the output are doubled.
 Prints a blank line for each command that has no output.
 .
+.so lib/vlog.man
+.
 .SH COMMANDS
 The commands implemented by \fBovs\-vsctl\fR are described in the
 sections below.
@@ -203,5 +193,5 @@ The \fIbridge\fR argument to \fBbr\-exists\fR specified the name of a
 bridge that does not exist.
 .SH "SEE ALSO"
 .
-.BR ovs\-vswitchd.conf (5),
+.BR ovsdb\-server (1),
 .BR ovs\-vswitchd (8).

utilities/ovs-vsctl.c

@@ -31,6 +31,7 @@
 #include "dynamic-string.h"
 #include "ovsdb-idl.h"
 #include "poll-loop.h"
+#include "svec.h"
 #include "vswitchd/vswitch-idl.h"
 #include "timeval.h"
 #include "util.h"
@@ -48,6 +49,7 @@ static char *default_db(void);
 static void usage(void) NO_RETURN;
 static void parse_options(int argc, char *argv[]);
 
+static void check_vsctl_command(int argc, char *argv[]);
 static void do_vsctl(int argc, char *argv[], struct ovsdb_idl *idl);
 
 int
@@ -55,14 +57,43 @@ main(int argc, char *argv[])
 {
     struct ovsdb_idl *idl;
     unsigned int seqno;
+    struct ds args;
+    int start, n_commands;
     int trials;
+    int i;
 
     set_program_name(argv[0]);
     signal(SIGPIPE, SIG_IGN);
     time_init();
     vlog_init();
+    vlog_set_levels(VLM_ANY_MODULE, VLF_CONSOLE, VLL_WARN);
+    vlog_set_levels(VLM_reconnect, VLF_ANY_FACILITY, VLL_WARN);
     parse_options(argc, argv);
 
+    /* Log our arguments.  This is often valuable for debugging systems. */
+    ds_init(&args);
+    for (i = 1; i < argc; i++) {
+        ds_put_format(&args, " %s", argv[i]);
+    }
+    VLOG_INFO("Called as%s", ds_cstr(&args));
+    ds_destroy(&args);
+
+    /* Do basic command syntax checking. */
+    n_commands = 0;
+    for (start = i = optind; i <= argc; i++) {
+        if (i == argc || !strcmp(argv[i], "--")) {
+            if (i > start) {
+                check_vsctl_command(i - start, &argv[start]);
+                n_commands++;
+            }
+            start = i + 1;
+        }
+    }
+    if (!n_commands) {
+        ovs_fatal(0, "missing command name (use --help for help)");
+    }
+
+    /* Now execut the commands. */
     idl = ovsdb_idl_create(db, &ovsrec_idl_class);
     seqno = ovsdb_idl_get_seqno(idl);
     trials = 0;
@@ -90,23 +121,22 @@ parse_options(int argc, char *argv[])
     enum {
         OPT_DB = UCHAR_MAX + 1,
         OPT_ONELINE,
+        OPT_NO_SYSLOG
     };
     static struct option long_options[] = {
         {"db", required_argument, 0, OPT_DB},
+        {"no-syslog", no_argument, 0, OPT_NO_SYSLOG},
         {"oneline", no_argument, 0, OPT_ONELINE},
         {"verbose", optional_argument, 0, 'v'},
         {"help", no_argument, 0, 'h'},
         {"version", no_argument, 0, 'V'},
         {0, 0, 0, 0},
     };
-    char *short_options;
 
-    short_options = xasprintf ("+%s",
-                               long_options_to_short_options(long_options));
     for (;;) {
         int c;
 
-        c = getopt_long(argc, argv, short_options, long_options, NULL);
+        c = getopt_long(argc, argv, "+v::hV", long_options, NULL);
         if (c == -1) {
             break;
         }
@@ -120,6 +150,10 @@ parse_options(int argc, char *argv[])
             oneline = true;
             break;
 
+        case OPT_NO_SYSLOG:
+            vlog_set_levels(VLM_vsctl, VLF_SYSLOG, VLL_WARN);
+            break;
+
         case 'h':
             usage();
 
@@ -138,7 +172,6 @@ parse_options(int argc, char *argv[])
             abort();
         }
     }
-    free(short_options);
 
     if (!db) {
         db = default_db();
@@ -205,7 +238,7 @@ default_db(void)
 {
     static char *def;
     if (!def) {
-        def = xasprintf("%s/ovsdb-server", ovs_rundir);
+        def = xasprintf("unix:%s/ovsdb-server", ovs_rundir);
     }
     return def;
 }
@@ -339,7 +372,7 @@ get_info(const struct ovsrec_open_vswitch *ovs, struct vsctl_info *info)
             }
 
             if (port_is_fake_bridge(port_cfg)
-                && shash_add_once(&bridges, br_cfg->name, NULL)) {
+                && shash_add_once(&bridges, port_cfg->name, NULL)) {
                 add_bridge(info, NULL, port_cfg->name, br, *port_cfg->tag);
             }
         }
@@ -368,7 +401,7 @@ get_info(const struct ovsrec_open_vswitch *ovs, struct vsctl_info *info)
             }
 
             if (port_is_fake_bridge(port_cfg)
-                && !shash_add_once(&bridges, br_cfg->name, NULL)) {
+                && !shash_add_once(&bridges, port_cfg->name, NULL)) {
                 continue;
             }
 
@@ -398,6 +431,7 @@ get_info(const struct ovsrec_open_vswitch *ovs, struct vsctl_info *info)
                 iface = xmalloc(sizeof *iface);
                 iface->iface_cfg = iface_cfg;
                 iface->port = port;
+                shash_add(&info->ifaces, iface_cfg->name, iface);
             }
         }
     }
@@ -455,7 +489,7 @@ static struct vsctl_iface *
 find_iface(struct vsctl_info *info, const char *name)
 {
     struct vsctl_iface *iface = shash_find_data(&info->ifaces, name);
-    if (!iface) {
+    if (!iface || !strcmp(name, iface->port->bridge->name)) {
         ovs_fatal(0, "no interface named %s", name);
     }
     return iface;
@@ -471,7 +505,6 @@ bridge_insert_port(struct ovsrec_bridge *br, struct ovsrec_port *port)
     for (i = 0; i < br->n_ports; i++) {
         ports[i] = br->ports[i];
     }
-    printf("bridge has %zu ports, adding 1\n", br->n_ports);
     ports[br->n_ports] = port;
     ovsrec_bridge_set_ports(br, ports, br->n_ports + 1);
     free(ports);
@@ -556,7 +589,7 @@ cmd_add_br(int argc, char *argv[], const struct ovsrec_open_vswitch *ovs,
         ovs_insert_bridge(ovs, br);
     } else if (argc == 3) {
         ovs_fatal(0, "'%s' comamnd takes exactly 1 or 3 arguments", argv[0]);
-    } else if (argc >= 4) {
+    } else if (argc == 4) {
         const char *parent_name = argv[2];
         int vlan = atoi(argv[3]);
         struct ovsrec_bridge *br;
@@ -587,6 +620,8 @@ cmd_add_br(int argc, char *argv[], const struct ovsrec_open_vswitch *ovs,
         ovsrec_port_set_interfaces(port, &iface, 1);
         ovsrec_port_set_fake_bridge(port, true);
         ovsrec_port_set_tag(port, &tag, 1);
+
+        bridge_insert_port(br, port);
     } else {
         NOT_REACHED();
     }
@@ -624,7 +659,8 @@ cmd_del_br(int argc UNUSED, char *argv[],
     bridge = find_bridge(&info, argv[1]);
     SHASH_FOR_EACH (node, &info.ports) {
         struct vsctl_port *port = node->data;
-        if (port->bridge == bridge) {
+        if (port->bridge == bridge
+            || !strcmp(port->port_cfg->name, bridge->name)) {
             del_port(&info, port);
         }
     }
@@ -635,18 +671,36 @@ cmd_del_br(int argc UNUSED, char *argv[],
     free_info(&info);
 }
 
+static void
+output_sorted(struct svec *svec, struct ds *output)
+{
+    const char *name;
+    size_t i;
+
+    svec_sort(svec);
+    SVEC_FOR_EACH (i, name, svec) {
+        ds_put_format(output, "%s\n", name);
+    }
+}
+
 static void
 cmd_list_br(int argc UNUSED, char *argv[] UNUSED,
             const struct ovsrec_open_vswitch *ovs, struct ds *output)
 {
     struct shash_node *node;
     struct vsctl_info info;
+    struct svec bridges;
 
     get_info(ovs, &info);
+
+    svec_init(&bridges);
     SHASH_FOR_EACH (node, &info.bridges) {
         struct vsctl_bridge *br = node->data;
-        ds_put_format(output, "%s\n", br->name);
+        svec_add(&bridges, br->name);
     }
+    output_sorted(&bridges, output);
+    svec_destroy(&bridges);
+
     free_info(&info);
 }
 
@@ -670,16 +724,22 @@ cmd_list_ports(int argc UNUSED, char *argv[],
     struct vsctl_bridge *br;
     struct shash_node *node;
     struct vsctl_info info;
+    struct svec ports;
 
     get_info(ovs, &info);
     br = find_bridge(&info, argv[1]);
+
+    svec_init(&ports);
     SHASH_FOR_EACH (node, &info.ports) {
         struct vsctl_port *port = node->data;
 
         if (strcmp(port->port_cfg->name, br->name) && br == port->bridge) {
-            ds_put_format(output, "%s\n", port->port_cfg->name);
+            svec_add(&ports, port->port_cfg->name);
         }
     }
+    output_sorted(&ports, output);
+    svec_destroy(&ports);
+
     free_info(&info);
 }
 
@@ -811,16 +871,23 @@ cmd_list_ifaces(int argc UNUSED, char *argv[],
     struct vsctl_bridge *br;
     struct shash_node *node;
     struct vsctl_info info;
+    struct svec ifaces;
 
     get_info(ovs, &info);
     br = find_bridge(&info, argv[1]);
+
+    svec_init(&ifaces);
     SHASH_FOR_EACH (node, &info.ifaces) {
         struct vsctl_iface *iface = node->data;
 
-        if (br == iface->port->bridge) {
-            ds_put_format(output, "%s\n", iface->iface_cfg->name);
+        if (strcmp(iface->iface_cfg->name, br->name)
+            && br == iface->port->bridge) {
+            svec_add(&ifaces, iface->iface_cfg->name);
         }
     }
+    output_sorted(&ifaces, output);
+    svec_destroy(&ifaces);
+
     free_info(&info);
 }
 
@@ -837,12 +904,15 @@ cmd_iface_to_br(int argc UNUSED, char *argv[],
     free_info(&info);
 }
 
+typedef void vsctl_handler_func(int argc, char *argv[],
+                                const struct ovsrec_open_vswitch *,
+                                struct ds *output);
+
 struct vsctl_command {
     const char *name;
     int min_args;
     int max_args;
-    void (*handler)(int argc, char *argv[],
-                    const struct ovsrec_open_vswitch *ovs, struct ds *output);
+    vsctl_handler_func *handler;
 };
 
 static void run_vsctl_command(int argc, char *argv[],
@@ -870,8 +940,8 @@ do_vsctl(int argc, char *argv[], struct ovsdb_idl *idl)
     txn = ovsdb_idl_txn_create(idl);
     output = xmalloc(argc * sizeof *output);
     n_output = 0;
-    for (start = i = 0; i < argc; i++) {
-        if (!strcmp(argv[i], "--")) {
+    for (start = i = 0; i <= argc; i++) {
+        if (i == argc || !strcmp(argv[i], "--")) {
             if (i > start) {
                 ds_init(&output[n_output]);
                 run_vsctl_command(i - start, &argv[start], ovs,
@@ -880,10 +950,6 @@ do_vsctl(int argc, char *argv[], struct ovsdb_idl *idl)
             start = i + 1;
         }
     }
-    if (i > start) {
-        ds_init(&output[n_output]);
-        run_vsctl_command(i - start, &argv[start], ovs, &output[n_output++]);
-    }
 
     while ((status = ovsdb_idl_txn_commit(txn)) == TXN_INCOMPLETE) {
         ovsdb_idl_run(idl);
@@ -945,9 +1011,8 @@ do_vsctl(int argc, char *argv[], struct ovsdb_idl *idl)
     exit(EXIT_SUCCESS);
 }
 
-static void
-run_vsctl_command(int argc, char *argv[],
-                  const struct ovsrec_open_vswitch *ovs, struct ds *output)
+static vsctl_handler_func *
+get_vsctl_handler(int argc, char *argv[])
 {
     static const struct vsctl_command all_commands[] = {
         {"add-br", 1, 3, cmd_add_br},
@@ -968,7 +1033,7 @@ run_vsctl_command(int argc, char *argv[],
     const struct vsctl_command *p;
 
     assert(argc > 0);
-    for (p = all_commands; p->name != NULL; p++) {
+    for (p = all_commands; p < &all_commands[ARRAY_SIZE(all_commands)]; p++) {
         if (!strcmp(p->name, argv[0])) {
             int n_arg = argc - 1;
             if (n_arg < p->min_args) {
@@ -978,11 +1043,23 @@ run_vsctl_command(int argc, char *argv[],
                 ovs_fatal(0, "'%s' command takes at most %d arguments",
                           p->name, p->max_args);
             } else {
-                p->handler(argc, argv, ovs, output);
-                return;
+                return p->handler;
             }
         }
     }
 
     ovs_fatal(0, "unknown command '%s'; use --help for help", argv[0]);
 }
+
+static void
+check_vsctl_command(int argc, char *argv[])
+{
+    get_vsctl_handler(argc, argv);
+}
+
+static void
+run_vsctl_command(int argc, char *argv[],
+                  const struct ovsrec_open_vswitch *ovs, struct ds *output)
+{
+    get_vsctl_handler(argc, argv)(argc, argv, ovs, output);
+}