ovn-nbctl: Add sanity checking for lswitch-add.

I don't think anyone really wants the painful behavior of creating multiple
logical switches with the same name to be the default.  This commit retains
the possibility of doing that in case someone really wants it, but refuses
by default for sanity.

Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Justin Pettit <jpettit@ovn.org>

diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index 9228301..8375ab7 100644 (file)
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -23,10 +23,33 @@
     <h1>Logical Switch Commands</h1>
 
     <dl>
-      <dt><code>lswitch-add</code> [<var>lswitch</var>]</dt> <dd> Creates a new logical switch named <var>lswitch</var>.  If
-        <var>lswitch</var> is not provided, the switch will not have a
-        name so other commands must refer to this switch by its UUID.
-        Initially the switch will have no ports.
+      <dt><code>lswitch-add</code></dt>
+      <dd>
+        <p>
+          Creates a new, unnamed logical switch, which initially has no ports.
+          The switch does not have a name, other commands must refer to this
+          switch by its UUID.
+        </p>
+      </dd>
+
+      <dt>[<code>--may-exist</code> | <code>--add-duplicate</code>] <code>lswitch-add</code> <var>lswitch</var></dt> 
+      <dd>
+        <p>
+          Creates a new logical switch named <var>lswitch</var>, which
+          initially has no ports.
+        </p>
+
+        <p>
+          The OVN northbound database schema does not require logical switch
+          names to be unique, but the whole point to the names is to provide an
+          easy way for humans to refer to the switches, making duplicate names
+          unhelpful.  Thus, without any options, this command regards it as an
+          error if <var>lswitch</var> is a duplicate name.  With
+          <code>--may-exist</code>, adding a duplicate name succeeds but does
+          not create a new logical switch.  With <code>--add-duplicate</code>,
+          the command really creates a new logical switch with a duplicate
+          name.  It is an error to specify both options.
+        </p>
       </dd>
 
       <dt>[<code>--if-exists</code>] <code>lswitch-del</code> <var>lswitch</var></dt>

diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 5a5c66c..5bdf757 100644 (file)
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -431,10 +431,37 @@ nbctl_show(struct ctl_context *ctx)
 static void
 nbctl_lswitch_add(struct ctl_context *ctx)
 {
+    const char *lswitch_name = ctx->argc == 2 ? ctx->argv[1] : NULL;
+
+    bool may_exist = shash_find(&ctx->options, "--may-exist") != NULL;
+    bool add_duplicate = shash_find(&ctx->options, "--add-duplicate") != NULL;
+    if (may_exist && add_duplicate) {
+        ctl_fatal("--may-exist and --add-duplicate may not be used together");
+    }
+
+    if (lswitch_name) {
+        if (!add_duplicate) {
+            const struct nbrec_logical_switch *lswitch;
+            NBREC_LOGICAL_SWITCH_FOR_EACH (lswitch, ctx->idl) {
+                if (!strcmp(lswitch->name, lswitch_name)) {
+                    if (may_exist) {
+                        return;
+                    }
+                    ctl_fatal("%s: an lswitch with this name already exists",
+                              lswitch_name);
+                }
+            }
+        }
+    } else if (may_exist) {
+        ctl_fatal("--may-exist requires specifying a name");
+    } else if (add_duplicate) {
+        ctl_fatal("--add-duplicate requires specifying a name");
+    }
+
     struct nbrec_logical_switch *lswitch;
     lswitch = nbrec_logical_switch_insert(ctx->txn);
-    if (ctx->argc == 2) {
-        nbrec_logical_switch_set_name(lswitch, ctx->argv[1]);
+    if (lswitch_name) {
+        nbrec_logical_switch_set_name(lswitch, lswitch_name);
     }
 }
 
@@ -1310,7 +1337,7 @@ static const struct ctl_command_syntax nbctl_commands[] = {
 
     /* lswitch commands. */
     { "lswitch-add", 0, 1, "[LSWITCH]", NULL, nbctl_lswitch_add,
-      NULL, "", RW },
+      NULL, "--may-exist,--add-duplicate", RW },
     { "lswitch-del", 1, 1, "LSWITCH", NULL, nbctl_lswitch_del,
       NULL, "--if-exists", RW },
     { "lswitch-list", 0, 0, "", NULL, nbctl_lswitch_list, NULL, "", RO },

diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index a8ed41e..47ec1ef 100644 (file)
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -53,7 +53,17 @@ AT_CHECK([ovn-nbctl lswitch-add ls0])
 AT_CHECK([ovn-nbctl show ls0 | ${PERL} $srcdir/uuidfilt.pl], [0],
   [    lswitch <0> (ls0)
 ])
-AT_CHECK([ovn-nbctl lswitch-add ls0])
+AT_CHECK([ovn-nbctl lswitch-add ls0], [1], [],
+  [ovn-nbctl: ls0: an lswitch with this name already exists
+])
+AT_CHECK([ovn-nbctl --may-exist lswitch-add ls0])
+AT_CHECK([ovn-nbctl show ls0 | ${PERL} $srcdir/uuidfilt.pl], [0],
+  [    lswitch <0> (ls0)
+])
+AT_CHECK([ovn-nbctl --add-duplicate lswitch-add ls0])
+AT_CHECK([ovn-nbctl --may-exist --add-duplicate lswitch-add ls0], [1], [],
+  [ovn-nbctl: --may-exist and --add-duplicate may not be used together
+])
 AT_CHECK([ovn-nbctl lswitch-del ls0], [1], [],
   [ovn-nbctl: Multiple logical switches named 'ls0'.  Use a UUID.
 ])
@@ -63,6 +73,15 @@ AT_CHECK([ovn-nbctl lswitch-del ls2], [1], [],
 ])
 AT_CHECK([ovn-nbctl --if-exists lswitch-del ls2])
 
+AT_CHECK([ovn-nbctl lswitch-add])
+AT_CHECK([ovn-nbctl lswitch-add])
+AT_CHECK([ovn-nbctl --add-duplicate lswitch-add], [1], [],
+  [ovn-nbctl: --add-duplicate requires specifying a name
+])
+AT_CHECK([ovn-nbctl --may-exist lswitch-add], [1], [],
+  [ovn-nbctl: --may-exist requires specifying a name
+])
+
 OVN_NBCTL_TEST_STOP
 AT_CLEANUP