[ovs-dev] [PATCH 3/3] ovn-nbctl: Add sanity checking for lswitch-add.
Ben Pfaff
blp at ovn.org
Fri May 6 20:10:39 UTC 2016
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 at ovn.org>
---
ovn/utilities/ovn-nbctl.8.xml | 81 +++++++++++++++++++++++++++++++++----------
ovn/utilities/ovn-nbctl.c | 34 +++++++++++++++---
tests/ovn-nbctl.at | 21 ++++++++++-
3 files changed, 113 insertions(+), 23 deletions(-)
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index f6ef803..66263cd 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -23,15 +23,39 @@
<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> [<var>lswitch</var>]</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>lswitch-del</code> <var>lswitch</var></dt>
+ <dt>[<code>--if-exists</code>] <code>lswitch-del</code> <var>lswitch</var></dt>
<dd>
- Deletes <var>lswitch</var>.
+ Deletes <var>lswitch</var>. It is an error if <var>lswitch</var> does
+ not exist, unless <code>--if-exists</code> is specified.
</dd>
<dt><code>lswitch-list</code></dt>
@@ -70,25 +94,46 @@
<h1>Logical Port Commands</h1>
<dl>
- <dt><code>lport-add</code> <var>lswitch</var> <var>lport</var></dt>
+ <dt>[<code>--may-exist</code>] <code>lport-add</code> <var>lswitch</var> <var>lport</var></dt>
<dd>
- Creates on <var>lswitch</var> a new logical port named
- <var>lport</var>.
+ <p>
+ Creates on <var>lswitch</var> a new logical port named
+ <var>lport</var>.
+ </p>
+
+ <p>
+ It is an error if a logical port named <var>lport</var> already
+ exists, unless <code>--may-exist</code> is specified. Regardless of
+ <code>--may-exist</code>, it is an error if the existing port is in
+ some logical switch other than <var>lswitch</var> or if it has a
+ parent port.
+ </p>
</dd>
- <dt><code>lport-add</code> <var>lswitch</var> <var>lport</var> <var>parent</var> <var>tag</var></dt>
+ <dt>[<code>--may-exist</code>] <code>lport-add</code> <var>lswitch</var> <var>lport</var> <var>parent</var> <var>tag</var></dt>
<dd>
- Creates on <var>lswitch</var> a logical port named <var>lport</var>
- that is a child of <var>parent</var> that is identifed with VLAN ID
- <var>tag</var>. This is useful in cases such as virtualized
- container environments where Open vSwitch does not have a direct
- connection to the container's port and it must be shared with
- the virtual machine's port.
+ <p>
+ Creates on <var>lswitch</var> a logical port named <var>lport</var>
+ that is a child of <var>parent</var> that is identifed with VLAN ID
+ <var>tag</var>. This is useful in cases such as virtualized
+ container environments where Open vSwitch does not have a direct
+ connection to the container's port and it must be shared with
+ the virtual machine's port.
+ </p>
+
+ <p>
+ It is an error if a logical port named <var>lport</var> already
+ exists, unless <code>--may-exist</code> is specified. Regardless of
+ <code>--may-exist</code>, it is an error if the existing port is not
+ in <var>lswitch</var> or if it does not have the specified
+ <var>parent</var> and <var>tag</var>.
+ </p>
</dd>
- <dt><code>lport-del</code> <var>lport</var></dt>
+ <dt>[<code>--if-exists</code>] <code>lport-del</code> <var>lport</var></dt>
<dd>
- Deletes <var>lport</var>.
+ Deletes <var>lport</var>. It is an error if <var>lport</var> does
+ not exist, unless <code>--if-exists</code> is specified.
</dd>
<dt><code>lport-list</code> <var>lswitch</var></dt>
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 6dcb873..5bdf757 100644
--- 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);
}
}
@@ -538,7 +565,6 @@ nbctl_lport_add(struct ctl_context *ctx)
const struct nbrec_logical_switch *lswitch;
lswitch = lswitch_by_name_or_uuid(ctx, ctx->argv[1], true);
-
const char *parent_name;
int64_t tag;
if (ctx->argc == 3) {
@@ -1311,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
--- 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
--
2.1.3
More information about the dev
mailing list