ovs-vsctl: Add --may-exist option for add-port, add-bond commands.
authorBen Pfaff <blp@nicira.com>
Tue, 9 Feb 2010 20:13:46 +0000 (12:13 -0800)
committerBen Pfaff <blp@nicira.com>
Tue, 9 Feb 2010 20:13:52 +0000 (12:13 -0800)
This is useful in interface-reconfigure, in an upcoming commit.

tests/ovs-vsctl.at
utilities/ovs-vsctl.8.in
utilities/ovs-vsctl.c

index e6e209d..ee2905b 100644 (file)
@@ -244,6 +244,12 @@ OVS_VSCTL_SETUP
 AT_CHECK([RUN_OVS_VSCTL(
    [add-br a], 
    [add-bond a bond0 a1 a2 a3])], [0], [], [], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([--may-exist add-bond a bond0 a3 a1 a2])], [0], [], [],
+  [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([--may-exist add-bond a bond0 a2 a1])], [1], [], 
+  [ovs-vsctl: "--may-exist add-bond a bond0 a2 a1" but bond0 actually has interface(s) a1, a2, a3
+],
+  [OVS_VSCTL_CLEANUP])
 CHECK_BRIDGES([a, a, 0])
 CHECK_PORTS([a], [bond0])
 CHECK_IFACES([a], [a1], [a2], [a3])
@@ -257,8 +263,14 @@ AT_CHECK([RUN_OVS_VSCTL(
   [add-br a], 
   [add-br b], 
   [add-port a a1],
-  [add-port b b1],
+  [--may-exist add-port b b1],
   [del-port a a1])], [0], [], [], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([--may-exist add-port b b1])], [0], [], [],
+  [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([--may-exist add-port a b1])], [1], [], 
+  [ovs-vsctl: "--may-exist add-port a b1" but b1 is actually attached to bridge b
+],
+  [OVS_VSCTL_CLEANUP])
 CHECK_BRIDGES([a, a, 0], [b, b, 0])
 CHECK_PORTS([a])
 CHECK_IFACES([a])
@@ -347,6 +359,7 @@ m4_define([OVS_VSCTL_SETUP_SIMPLE_FAKE_CONF],
         [add-br xenbr0],
         [--may-exist add-br xenbr0],
         [add-port xenbr0 eth0],
+        [--may-exist add-port xenbr0 eth0],
         [add-br xapi1 xenbr0 9],
         [--may-exist add-br xapi1 xenbr0 9],
         [add-port xapi1 eth0.9])],
index 723a3af..f588581 100644 (file)
@@ -216,9 +216,13 @@ commands treat a bonded port as a single entity.
 Lists all of the ports within \fIbridge\fR on standard output, one per
 line.  The local port \fIbridge\fR is not included in the list.
 .
-.IP "\fBadd\-port \fIbridge port\fR"
+.IP "[\fB\-\-may\-exist\fR] \fBadd\-port \fIbridge port\fR"
 Creates on \fIbridge\fR a new port named \fIport\fR from the network
 device of the same name.
+.IP
+Without \fB\-\-may\-exist\fR, attempting to create a port that exists
+is an error.  With \fB\-\-may\-exist\fR, \fIport\fR may already exist
+(but it must be on \fIbridge\fR and not be a bonded port).
 .
 .IP "[\fB\-\-fake\-iface\fR] \fBadd\-bond \fIbridge port iface\fR\&..."
 Creates on \fIbridge\fR a new port named \fIport\fR that bonds
@@ -228,6 +232,11 @@ interfaces must be named.
 With \fB\-\-fake\-iface\fR, a fake interface with the name \fIport\fR is
 created.  This should only be used for compatibility with legacy
 software that requires it.
+.IP
+Without \fB\-\-may\-exist\fR, attempting to create a port that exists
+is an error.  With \fB\-\-may\-exist\fR, \fIport\fR may already exist
+(but it must be on \fIbridge\fR and bond together exactly the
+specified interface).
 .
 .IP "[\fB\-\-if\-exists\fR] \fBdel\-port \fR[\fIbridge\fR] \fIport\fR"
 Deletes \fIport\fR.  If \fIbridge\fR is omitted, \fIport\fR is removed
index 98b7cea..9baca42 100644 (file)
@@ -492,6 +492,30 @@ struct vsctl_info {
     struct ovsrec_controller *ctrl;
 };
 
+static char *
+vsctl_context_to_string(const struct vsctl_context *ctx)
+{
+    const struct shash_node *node;
+    struct svec words;
+    char *s;
+    int i;
+
+    svec_init(&words);
+    SHASH_FOR_EACH (node, &ctx->options) {
+        svec_add(&words, node->name);
+    }
+    for (i = 0; i < ctx->argc; i++) {
+        svec_add(&words, ctx->argv[i]);
+    }
+    svec_terminate(&words);
+
+    s = process_escape_args(words.names);
+
+    svec_destroy(&words);
+
+    return s;
+}
+
 static struct vsctl_bridge *
 add_bridge(struct vsctl_info *b,
            struct ovsrec_bridge *br_cfg, const char *name,
@@ -1148,7 +1172,8 @@ cmd_list_ports(struct vsctl_context *ctx)
 
 static void
 add_port(struct vsctl_context *ctx,
-         const char *br_name, const char *port_name, bool fake_iface,
+         const char *br_name, const char *port_name,
+         bool may_exist, bool fake_iface,
          char *iface_names[], int n_ifaces)
 {
     struct vsctl_info info;
@@ -1158,9 +1183,53 @@ add_port(struct vsctl_context *ctx,
     size_t i;
 
     get_info(ctx->ovs, &info);
+    if (may_exist) {
+        struct vsctl_port *port;
+
+        port = find_port(&info, port_name, false);
+        if (port) {
+            struct svec want_names, have_names;
+            size_t i;
+
+            svec_init(&want_names);
+            for (i = 0; i < n_ifaces; i++) {
+                svec_add(&want_names, iface_names[i]);
+            }
+            svec_sort(&want_names);
+
+            svec_init(&have_names);
+            for (i = 0; i < port->port_cfg->n_interfaces; i++) {
+                svec_add(&have_names, port->port_cfg->interfaces[i]->name);
+            }
+            svec_sort(&have_names);
+
+            if (strcmp(port->bridge->name, br_name)) {
+                char *command = vsctl_context_to_string(ctx);
+                vsctl_fatal("\"%s\" but %s is actually attached to bridge %s",
+                            command, port_name, port->bridge->name);
+            }
+
+            if (!svec_equal(&want_names, &have_names)) {
+                char *have_names_string = svec_join(&have_names, ", ", "");
+                char *command = vsctl_context_to_string(ctx);
+
+                vsctl_fatal("\"%s\" but %s actually has interface(s) %s",
+                            command, port_name, have_names_string);
+            }
+
+            svec_destroy(&want_names);
+            svec_destroy(&have_names);
+
+            return;
+        }
+    }
     check_conflicts(&info, port_name,
                     xasprintf("cannot create a port named %s", port_name));
-    /* XXX need to check for conflicts on interfaces too */
+    for (i = 0; i < n_ifaces; i++) {
+        check_conflicts(&info, iface_names[i],
+                        xasprintf("cannot create an interface named %s",
+                                  iface_names[i]));
+    }
     bridge = find_bridge(&info, br_name, true);
 
     ifaces = xmalloc(n_ifaces * sizeof *ifaces);
@@ -1189,15 +1258,19 @@ add_port(struct vsctl_context *ctx,
 static void
 cmd_add_port(struct vsctl_context *ctx)
 {
-    add_port(ctx, ctx->argv[1], ctx->argv[2], false, &ctx->argv[2], 1);
+    bool may_exist = shash_find(&ctx->options, "--may-exist") != 0;
+
+    add_port(ctx, ctx->argv[1], ctx->argv[2], may_exist, false,
+             &ctx->argv[2], 1);
 }
 
 static void
 cmd_add_bond(struct vsctl_context *ctx)
 {
+    bool may_exist = shash_find(&ctx->options, "--may-exist") != 0;
     bool fake_iface = shash_find(&ctx->options, "--fake-iface");
 
-    add_port(ctx, ctx->argv[1], ctx->argv[2], fake_iface,
+    add_port(ctx, ctx->argv[1], ctx->argv[2], may_exist, fake_iface,
              &ctx->argv[3], ctx->argc - 3);
 }
 
@@ -2410,8 +2483,8 @@ static const struct vsctl_command_syntax all_commands[] = {
 
     /* Port commands. */
     {"list-ports", 1, 1, cmd_list_ports, NULL, ""},
-    {"add-port", 2, 2, cmd_add_port, NULL, ""},
-    {"add-bond", 4, INT_MAX, cmd_add_bond, NULL, "--fake-iface"},
+    {"add-port", 2, 2, cmd_add_port, NULL, "--may-exist"},
+    {"add-bond", 4, INT_MAX, cmd_add_bond, NULL, "--may-exist,--fake-iface"},
     {"del-port", 1, 2, cmd_del_port, NULL, "--if-exists"},
     {"port-to-br", 1, 1, cmd_port_to_br, NULL, ""},