ovs-vsctl: Issue warning for likely erroneous "get" commands.

[openvswitch] / utilities / ovs-vsctl.c

diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 2baab003acc9a40b7fa2e1bbb6f7001bb60bc923..6c2fbece788fe222c7a70c6404b656af131fc858 100644 (file)
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -26,6 +26,7 @@
 #include <stdarg.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>
 
 #include "command-line.h"
 #include "compiler.h"
@@ -38,6 +39,7 @@
 #include "process.h"
 #include "stream.h"
 #include "stream-ssl.h"
+#include "sset.h"
 #include "svec.h"
 #include "vswitchd/vswitch-idl.h"
 #include "table.h"
@@ -757,7 +759,7 @@ static void
 get_info(struct vsctl_context *ctx, struct vsctl_info *info)
 {
     const struct ovsrec_open_vswitch *ovs = ctx->ovs;
-    struct shash bridges, ports;
+    struct sset bridges, ports;
     size_t i;
 
     info->ctx = ctx;
@@ -765,14 +767,14 @@ get_info(struct vsctl_context *ctx, struct vsctl_info *info)
     shash_init(&info->ports);
     shash_init(&info->ifaces);
 
-    shash_init(&bridges);
-    shash_init(&ports);
+    sset_init(&bridges);
+    sset_init(&ports);
     for (i = 0; i < ovs->n_bridges; i++) {
         struct ovsrec_bridge *br_cfg = ovs->bridges[i];
         struct vsctl_bridge *br;
         size_t j;
 
-        if (!shash_add_once(&bridges, br_cfg->name, NULL)) {
+        if (!sset_add(&bridges, br_cfg->name)) {
             VLOG_WARN("%s: database contains duplicate bridge name",
                       br_cfg->name);
             continue;
@@ -785,29 +787,29 @@ get_info(struct vsctl_context *ctx, struct vsctl_info *info)
         for (j = 0; j < br_cfg->n_ports; j++) {
             struct ovsrec_port *port_cfg = br_cfg->ports[j];
 
-            if (!shash_add_once(&ports, port_cfg->name, NULL)) {
+            if (!sset_add(&ports, port_cfg->name)) {
                 VLOG_WARN("%s: database contains duplicate port name",
                           port_cfg->name);
                 continue;
             }
 
             if (port_is_fake_bridge(port_cfg)
-                && shash_add_once(&bridges, port_cfg->name, NULL)) {
+                && sset_add(&bridges, port_cfg->name)) {
                 add_bridge(info, NULL, port_cfg->name, br, *port_cfg->tag);
             }
         }
     }
-    shash_destroy(&bridges);
-    shash_destroy(&ports);
+    sset_destroy(&bridges);
+    sset_destroy(&ports);
 
-    shash_init(&bridges);
-    shash_init(&ports);
+    sset_init(&bridges);
+    sset_init(&ports);
     for (i = 0; i < ovs->n_bridges; i++) {
         struct ovsrec_bridge *br_cfg = ovs->bridges[i];
         struct vsctl_bridge *br;
         size_t j;
 
-        if (!shash_add_once(&bridges, br_cfg->name, NULL)) {
+        if (!sset_add(&bridges, br_cfg->name)) {
             continue;
         }
         br = shash_find_data(&info->bridges, br_cfg->name);
@@ -816,12 +818,12 @@ get_info(struct vsctl_context *ctx, struct vsctl_info *info)
             struct vsctl_port *port;
             size_t k;
 
-            if (!shash_add_once(&ports, port_cfg->name, NULL)) {
+            if (!sset_add(&ports, port_cfg->name)) {
                 continue;
             }
 
             if (port_is_fake_bridge(port_cfg)
-                && !shash_add_once(&bridges, port_cfg->name, NULL)) {
+                && !sset_add(&bridges, port_cfg->name)) {
                 continue;
             }
 
@@ -855,8 +857,8 @@ get_info(struct vsctl_context *ctx, struct vsctl_info *info)
             }
         }
     }
-    shash_destroy(&bridges);
-    shash_destroy(&ports);
+    sset_destroy(&bridges);
+    sset_destroy(&ports);
 }
 
 static void
@@ -2502,9 +2504,6 @@ parse_column_key_value(const char *arg,
         }
         *valuep = xstrdup(p + best_len);
     } else {
-        if (valuep) {
-            *valuep = NULL;
-        }
         if (*p != '\0') {
             error = xasprintf("%s: trailing garbage \"%s\" in argument",
                               arg, p);
@@ -2549,10 +2548,20 @@ pre_parse_column_key_value(struct vsctl_context *ctx,
 static void
 pre_cmd_get(struct vsctl_context *ctx)
 {
+    const char *id = shash_find_data(&ctx->options, "--id");
     const char *table_name = ctx->argv[1];
     const struct vsctl_table_class *table;
     int i;
 
+    /* Using "get" without --id or a column name could possibly make sense.
+     * Maybe, for example, a ovs-vsctl run wants to assert that a row exists.
+     * But it is unlikely that an interactive user would want to do that, so
+     * issue a warning if we're running on a terminal. */
+    if (!id && ctx->argc <= 3 && isatty(STDOUT_FILENO)) {
+        VLOG_WARN("\"get\" command without row arguments or \"--id\" is "
+                  "possibly erroneous");
+    }
+
     table = pre_get_table(ctx, table_name);
     for (i = 3; i < ctx->argc; i++) {
         if (!strcasecmp(ctx->argv[i], "_uuid")
@@ -2802,11 +2811,9 @@ cmd_list(struct vsctl_context *ctx)
         }
     } else {
         const struct ovsdb_idl_row *row;
-        bool first;
 
-        for (row = ovsdb_idl_first_row(ctx->idl, table->class), first = true;
-             row != NULL;
-             row = ovsdb_idl_next_row(row), first = false) {
+        for (row = ovsdb_idl_first_row(ctx->idl, table->class); row != NULL;
+             row = ovsdb_idl_next_row(row)) {
             list_record(row, columns, n_columns, out);
         }
     }