[ovs-dev] [PATCH] ovs-vsctl: Add --if-exists option to many database commands.

Ben Pfaff blp at nicira.com
Tue Jan 15 21:24:05 UTC 2013


A few ovs-vsctl commands have accepted --if-exists options for some time,
to make it possible to execute them in cases where it doesn't really
matter if the records they touch exist.  This commit adds this option to
other commands.

This is intended for initial use with "ovs-vsctl set interface <iface>
ofport_request=<number>" commands in ovs-ctl for upgrades from OVS 1.9
to later versions.

CC: Gurucharan Shetty <gshetty at nicira.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 NEWS                     |    2 ++
 tests/ovs-vsctl.at       |   24 ++++++++++++++
 utilities/ovs-vsctl.8.in |   48 +++++++++++++++++++++-------
 utilities/ovs-vsctl.c    |   78 ++++++++++++++++++++++++++++++----------------
 4 files changed, 114 insertions(+), 38 deletions(-)

diff --git a/NEWS b/NEWS
index 3f06a7a..a7c8a0e 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,8 @@ post-v1.9.0
       separately on a per-port basis, so it should no longer be
       possible for a large number of new flows arriving on one port to
       prevent new flows from being processed on other ports.
+    - Many "ovs-vsctl" database commands now accept an --if-exists option.
+      Please refer to the ovs-vsctl manpage for details.
     - New "vlog/disable-rate-limit" and "vlog/enable-rate-limit" commands
       available through ovs-appctl allow control over logging rate limits.
     - The OpenFlow "dp_desc" may now be configured by setting the value of 
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 6a1cc35..326d5a4 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -666,6 +666,18 @@ AT_CHECK([RUN_OVS_VSCTL_TOGETHER([destroy b br0],
   [0], [stdout], [], [OVS_VSCTL_CLEANUP])
 AT_CHECK([RUN_OVS_VSCTL([list b])], 
   [0], [], [], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([--if-exists get b x datapath_id])],
+  [0], [], [], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([--if-exists list b x])],
+  [0], [], [], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([--if-exists set controller x connection_mode=standalone])],
+  [0], [], [], [OVS_VSCTL_CLEANUP])
+AT_CHECK(
+  [RUN_OVS_VSCTL([--if-exists remove netflow x targets '"1.2.3.4:567"'])],
+  [0], [], [], [OVS_VSCTL_CLEANUP])
+AT_CHECK(
+  [RUN_OVS_VSCTL([--if-exists clear netflow x targets])],
+  [0], [], [], [OVS_VSCTL_CLEANUP])
 OVS_VSCTL_CLEANUP
 AT_CLEANUP
 
@@ -713,6 +725,9 @@ AT_CHECK([RUN_OVS_VSCTL([list interx x])],
 AT_CHECK([RUN_OVS_VSCTL([list b x])], 
   [1], [], [ovs-vsctl: no row "x" in table Bridge
 ], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([get b x datapath_id])],
+  [1], [], [ovs-vsctl: no row "x" in table Bridge
+], [OVS_VSCTL_CLEANUP])
 AT_CHECK([RUN_OVS_VSCTL([get b br0 d])], 
   [1], [], [ovs-vsctl: Bridge contains more than one column whose name matches "d"
 ], [OVS_VSCTL_CLEANUP])
@@ -728,6 +743,9 @@ AT_CHECK([RUN_OVS_VSCTL([get b br0 datapath_id:y=z])],
 AT_CHECK([RUN_OVS_VSCTL([set b br0 'datapath_id:y>=z'])], 
   [1], [], [ovs-vsctl: datapath_id:y>=z: argument does not end in "=" followed by a value.
 ], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([set controller x connection_mode=standalone])],
+  [1], [], [ovs-vsctl: no row "x" in table Controller
+], [OVS_VSCTL_CLEANUP])
 AT_CHECK([RUN_OVS_VSCTL([wait-until b br0 datapath_id:y,z])], 
   [1], [], [ovs-vsctl: datapath_id:y,z: argument does not end in "=", "!=", "<", ">", "<=", ">=", "{=}", "{!=}", "{<}", "{>}", "{<=}", or "{>=}" followed by a value.
 ], [OVS_VSCTL_CLEANUP])
@@ -758,6 +776,12 @@ AT_CHECK([RUN_OVS_VSCTL([add b br1 datapath_id x y])],
 AT_CHECK([RUN_OVS_VSCTL([remove netflow `cat netflow-uuid` targets '"1.2.3.4:567"'])], 
   [1], [], [ovs-vsctl: "remove" operation would put 0 values in column targets of table NetFlow but the minimum number is 1
 ], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([remove netflow x targets '"1.2.3.4:567"'])], 
+  [1], [], [ovs-vsctl: no row "x" in table NetFlow
+], [OVS_VSCTL_CLEANUP])
+AT_CHECK([RUN_OVS_VSCTL([clear netflow x targets])],
+  [1], [], [ovs-vsctl: no row "x" in table NetFlow
+], [OVS_VSCTL_CLEANUP])
 AT_CHECK([RUN_OVS_VSCTL([clear netflow `cat netflow-uuid` targets])], 
   [1], [], [ovs-vsctl: "clear" operation cannot be applied to column targets of table NetFlow, which is not allowed to be empty
 ], [OVS_VSCTL_CLEANUP])
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 30baafd..cf6cf88 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -567,13 +567,19 @@ well (but use quotes to prevent the shell from expanding
 other-config=1=y\fR, which may not have the desired effect).
 .
 .ST "Database Command Syntax"
-.IP "[\fB\-\-columns=\fIcolumn\fR[\fB,\fIcolumn\fR]...] \fBlist \fItable \fR[\fIrecord\fR]..."
+.
+.IP "[\fB\-\-if\-exists\fR] [\fB\-\-columns=\fIcolumn\fR[\fB,\fIcolumn\fR]...] \fBlist \fItable \fR[\fIrecord\fR]..."
 Lists the data in each specified \fIrecord\fR.  If no
 records are specified, lists all the records in \fItable\fR.
 .IP
 If \fB\-\-columns\fR is specified, only the requested columns are
 listed, in the specified order.  Otherwise, all columns are listed, in
 alphabetical order by column name.
+.IP
+Without \fB\-\-if-exists\fR, it is an error if any specified
+\fIrecord\fR does not exist.  With \fB\-\-if-exists\fR, the command
+ignores any \fIrecord\fR that does not exist, without producing any
+output.
 .
 .IP "[\fB\-\-columns=\fIcolumn\fR[\fB,\fIcolumn\fR]...] \fBfind \fItable \fR[\fIcolumn\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR]..."
 Lists the data in each record in \fItable\fR whose \fIcolumn\fR equals
@@ -636,16 +642,16 @@ alphabetical order by column name.
 The UUIDs shown for rows created in the same \fBovs\-vsctl\fR
 invocation will be wrong.
 .
-.IP "[\fB\-\-id=@\fIname\fR] [\fB\-\-if\-exists\fR] \fBget \fItable record \fR[\fIcolumn\fR[\fB:\fIkey\fR]]..."
+.IP "[\fB\-\-if\-exists\fR] [\fB\-\-id=@\fIname\fR] \fBget \fItable record \fR[\fIcolumn\fR[\fB:\fIkey\fR]]..."
 Prints the value of each specified \fIcolumn\fR in the given
 \fIrecord\fR in \fItable\fR.  For map columns, a \fIkey\fR may
 optionally be specified, in which case the value associated with
 \fIkey\fR in the column is printed, instead of the entire map.
 .IP
-For a map column, without \fB\-\-if\-exists\fR it is an error if
-\fIkey\fR does not exist; with it, a blank line is printed.  If
-\fIcolumn\fR is not a map column or if \fIkey\fR is not specified,
-\fB\-\-if\-exists\fR has no effect.
+Without \fB\-\-if\-exists\fR, it is an error if \fIrecord\fR does not
+exist or \fIkey\fR is specified, if \fIkey\fR does not exist in
+\fIrecord\fR.  With \fB\-\-if\-exists\fR, a missing \fIrecord\fR
+yields no output and a missing \fIkey\fR prints a blank line.
 .IP
 If \fB@\fIname\fR is specified, then the UUID for \fIrecord\fR may be
 referred to by that name later in the same \fBovs\-vsctl\fR
@@ -655,24 +661,34 @@ Both \fB\-\-id\fR and the \fIcolumn\fR arguments are optional, but
 usually at least one or the other should be specified.  If both are
 omitted, then \fBget\fR has no effect except to verify that
 \fIrecord\fR exists in \fItable\fR.
+.IP
+\fB\-\-id\fR and \fB\-\-if\-exists\fR cannot be used together.
 .
-.IP "\fBset \fItable record column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..."
+.IP "[\fB\-\-if\-exists\fR] \fBset \fItable record column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..."
 Sets the value of each specified \fIcolumn\fR in the given
 \fIrecord\fR in \fItable\fR to \fIvalue\fR.  For map columns, a
 \fIkey\fR may optionally be specified, in which case the value
 associated with \fIkey\fR in that column is changed (or added, if none
 exists), instead of the entire map.
+.IP
+Without \fB\-\-if-exists\fR, it is an error if \fIrecord\fR does not
+exist.  With \fB\-\-if-exists\fR, this command does nothing if
+\fIrecord\fR does not exist.
 .
-.IP "\fBadd \fItable record column \fR[\fIkey\fB=\fR]\fIvalue\fR..."
+.IP "[\fB\-\-if\-exists\fR] \fBadd \fItable record column \fR[\fIkey\fB=\fR]\fIvalue\fR..."
 Adds the specified value or key-value pair to \fIcolumn\fR in
 \fIrecord\fR in \fItable\fR.  If \fIcolumn\fR is a map, then \fIkey\fR
 is required, otherwise it is prohibited.  If \fIkey\fR already exists
 in a map column, then the current \fIvalue\fR is not replaced (use the
 \fBset\fR command to replace an existing value).
+.IP
+Without \fB\-\-if-exists\fR, it is an error if \fIrecord\fR does not
+exist.  With \fB\-\-if-exists\fR, this command does nothing if
+\fIrecord\fR does not exist.
 .
-.IP "\fBremove \fItable record column \fR\fIvalue\fR..."
-.IQ "\fBremove \fItable record column \fR\fIkey\fR..."
-.IQ "\fBremove \fItable record column \fR\fIkey\fB=\fR\fIvalue\fR..."
+.IP "[\fB\-\-if\-exists\fR] \fBremove \fItable record column \fR\fIvalue\fR..."
+.IQ "[\fB\-\-if\-exists\fR] \fBremove \fItable record column \fR\fIkey\fR..."
+.IQ "[\fB\-\-if\-exists\fR] \fBremove \fItable record column \fR\fIkey\fB=\fR\fIvalue\fR..."
 Removes the specified values or key-value pairs from \fIcolumn\fR in
 \fIrecord\fR in \fItable\fR.  The first form applies to columns that
 are not maps: each specified \fIvalue\fR is removed from the column.
@@ -683,11 +699,19 @@ pair is removed only if both key and value match.
 .IP
 It is not an error if the column does not contain the specified key or
 value or pair.
+.IP
+Without \fB\-\-if-exists\fR, it is an error if \fIrecord\fR does not
+exist.  With \fB\-\-if-exists\fR, this command does nothing if
+\fIrecord\fR does not exist.
 .
-.IP "\fBclear\fR \fItable record column\fR..."
+.IP "[\fB\-\-if\-exists\fR] \fBclear\fR \fItable record column\fR..."
 Sets each \fIcolumn\fR in \fIrecord\fR in \fItable\fR to the empty set
 or empty map, as appropriate.  This command applies only to columns
 that are allowed to be empty.
+.IP
+Without \fB\-\-if-exists\fR, it is an error if \fIrecord\fR does not
+exist.  With \fB\-\-if-exists\fR, this command does nothing if
+\fIrecord\fR does not exist.
 .
 .IP "[\fB\-\-id=@\fIname\fR] \fBcreate\fR \fItable column\fR[\fB:\fIkey\fR]\fB=\fIvalue\fR..."
 Creates a new record in \fItable\fR and sets the initial values of
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index bccb2c9..bbaaa1b 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -2646,7 +2646,8 @@ get_row_by_id(struct vsctl_context *ctx, const struct vsctl_table_class *table,
 
 static const struct ovsdb_idl_row *
 get_row (struct vsctl_context *ctx,
-         const struct vsctl_table_class *table, const char *record_id)
+         const struct vsctl_table_class *table, const char *record_id,
+         bool must_exist)
 {
     const struct ovsdb_idl_row *row;
     struct uuid uuid;
@@ -2663,15 +2664,7 @@ get_row (struct vsctl_context *ctx,
             }
         }
     }
-    return row;
-}
-
-static const struct ovsdb_idl_row *
-must_get_row(struct vsctl_context *ctx,
-             const struct vsctl_table_class *table, const char *record_id)
-{
-    const struct ovsdb_idl_row *row = get_row(ctx, table, record_id);
-    if (!row) {
+    if (must_exist && !row) {
         vsctl_fatal("no row \"%s\" in table %s",
                     record_id, table->class->name);
     }
@@ -2943,7 +2936,7 @@ static void
 cmd_get(struct vsctl_context *ctx)
 {
     const char *id = shash_find_data(&ctx->options, "--id");
-    bool if_exists = shash_find(&ctx->options, "--if-exists");
+    bool must_exist = !shash_find(&ctx->options, "--if-exists");
     const char *table_name = ctx->argv[1];
     const char *record_id = ctx->argv[2];
     const struct vsctl_table_class *table;
@@ -2951,8 +2944,16 @@ cmd_get(struct vsctl_context *ctx)
     struct ds *out = &ctx->output;
     int i;
 
+    if (id && !must_exist) {
+        vsctl_fatal("--if-exists and --id may not be specified together");
+    }
+
     table = get_table(table_name);
-    row = must_get_row(ctx, table, record_id);
+    row = get_row(ctx, table, record_id, must_exist);
+    if (!row) {
+        return;
+    }
+
     if (id) {
         struct ovsdb_symbol *symbol;
         bool new;
@@ -3004,7 +3005,7 @@ cmd_get(struct vsctl_context *ctx)
             idx = ovsdb_datum_find_key(datum, &key,
                                        column->type.key.type);
             if (idx == UINT_MAX) {
-                if (!if_exists) {
+                if (must_exist) {
                     vsctl_fatal("no key \"%s\" in %s record \"%s\" column %s",
                                 key_string, table->class->name, record_id,
                                 column->name);
@@ -3130,6 +3131,10 @@ list_record(const struct ovsdb_idl_row *row,
 {
     size_t i;
 
+    if (!row) {
+        return;
+    }
+
     table_add_row(out);
     for (i = 0; i < n_columns; i++) {
         const struct ovsdb_idl_column *column = columns[i];
@@ -3160,6 +3165,7 @@ static void
 cmd_list(struct vsctl_context *ctx)
 {
     const char *column_names = shash_find_data(&ctx->options, "--columns");
+    bool must_exist = !shash_find(&ctx->options, "--if-exists");
     const struct ovsdb_idl_column **columns;
     const char *table_name = ctx->argv[1];
     const struct vsctl_table_class *table;
@@ -3172,7 +3178,7 @@ cmd_list(struct vsctl_context *ctx)
     out = ctx->table = list_make_table(columns, n_columns);
     if (ctx->argc > 2) {
         for (i = 2; i < ctx->argc; i++) {
-            list_record(must_get_row(ctx, table, ctx->argv[i]),
+            list_record(get_row(ctx, table, ctx->argv[i], must_exist),
                         columns, n_columns, out);
         }
     } else {
@@ -3302,6 +3308,7 @@ set_column(const struct vsctl_table_class *table,
 static void
 cmd_set(struct vsctl_context *ctx)
 {
+    bool must_exist = !shash_find(&ctx->options, "--if-exists");
     const char *table_name = ctx->argv[1];
     const char *record_id = ctx->argv[2];
     const struct vsctl_table_class *table;
@@ -3309,7 +3316,11 @@ cmd_set(struct vsctl_context *ctx)
     int i;
 
     table = get_table(table_name);
-    row = must_get_row(ctx, table, record_id);
+    row = get_row(ctx, table, record_id, must_exist);
+    if (!row) {
+        return;
+    }
+
     for (i = 3; i < ctx->argc; i++) {
         set_column(table, row, ctx->argv[i], ctx->symtab);
     }
@@ -3333,6 +3344,7 @@ pre_cmd_add(struct vsctl_context *ctx)
 static void
 cmd_add(struct vsctl_context *ctx)
 {
+    bool must_exist = !shash_find(&ctx->options, "--if-exists");
     const char *table_name = ctx->argv[1];
     const char *record_id = ctx->argv[2];
     const char *column_name = ctx->argv[3];
@@ -3344,8 +3356,11 @@ cmd_add(struct vsctl_context *ctx)
     int i;
 
     table = get_table(table_name);
-    row = must_get_row(ctx, table, record_id);
     die_if_error(get_column(table, column_name, &column));
+    row = get_row(ctx, table, record_id, must_exist);
+    if (!row) {
+        return;
+    }
 
     type = &column->type;
     ovsdb_datum_clone(&old, ovsdb_idl_read(row, column), &column->type);
@@ -3390,6 +3405,7 @@ pre_cmd_remove(struct vsctl_context *ctx)
 static void
 cmd_remove(struct vsctl_context *ctx)
 {
+    bool must_exist = !shash_find(&ctx->options, "--if-exists");
     const char *table_name = ctx->argv[1];
     const char *record_id = ctx->argv[2];
     const char *column_name = ctx->argv[3];
@@ -3401,8 +3417,11 @@ cmd_remove(struct vsctl_context *ctx)
     int i;
 
     table = get_table(table_name);
-    row = must_get_row(ctx, table, record_id);
     die_if_error(get_column(table, column_name, &column));
+    row = get_row(ctx, table, record_id, must_exist);
+    if (!row) {
+        return;
+    }
 
     type = &column->type;
     ovsdb_datum_clone(&old, ovsdb_idl_read(row, column), &column->type);
@@ -3457,6 +3476,7 @@ pre_cmd_clear(struct vsctl_context *ctx)
 static void
 cmd_clear(struct vsctl_context *ctx)
 {
+    bool must_exist = !shash_find(&ctx->options, "--if-exists");
     const char *table_name = ctx->argv[1];
     const char *record_id = ctx->argv[2];
     const struct vsctl_table_class *table;
@@ -3464,7 +3484,11 @@ cmd_clear(struct vsctl_context *ctx)
     int i;
 
     table = get_table(table_name);
-    row = must_get_row(ctx, table, record_id);
+    row = get_row(ctx, table, record_id, must_exist);
+    if (!row) {
+        return;
+    }
+
     for (i = 3; i < ctx->argc; i++) {
         const struct ovsdb_idl_column *column;
         const struct ovsdb_type *type;
@@ -3597,7 +3621,7 @@ cmd_destroy(struct vsctl_context *ctx)
         for (i = 2; i < ctx->argc; i++) {
             const struct ovsdb_idl_row *row;
 
-            row = (must_exist ? must_get_row : get_row)(ctx, table, ctx->argv[i]);
+            row = get_row(ctx, table, ctx->argv[i], must_exist);
             if (row) {
                 ovsdb_idl_txn_delete(row);
             }
@@ -3778,7 +3802,7 @@ cmd_wait_until(struct vsctl_context *ctx)
 
     table = get_table(table_name);
 
-    row = get_row(ctx, table, record_id);
+    row = get_row(ctx, table, record_id, false);
     if (!row) {
         ctx->try_again = true;
         return;
@@ -4126,12 +4150,14 @@ static const struct vsctl_command_syntax all_commands[] = {
     /* Database commands. */
     {"comment", 0, INT_MAX, NULL, NULL, NULL, "", RO},
     {"get", 2, INT_MAX, pre_cmd_get, cmd_get, NULL, "--if-exists,--id=", RO},
-    {"list", 1, INT_MAX, pre_cmd_list, cmd_list, NULL, "--columns=", RO},
+    {"list", 1, INT_MAX, pre_cmd_list, cmd_list, NULL,
+     "--if-exists,--columns=", RO},
     {"find", 1, INT_MAX, pre_cmd_find, cmd_find, NULL, "--columns=", RO},
-    {"set", 3, INT_MAX, pre_cmd_set, cmd_set, NULL, "", RW},
-    {"add", 4, INT_MAX, pre_cmd_add, cmd_add, NULL, "", RW},
-    {"remove", 4, INT_MAX, pre_cmd_remove, cmd_remove, NULL, "", RW},
-    {"clear", 3, INT_MAX, pre_cmd_clear, cmd_clear, NULL, "", RW},
+    {"set", 3, INT_MAX, pre_cmd_set, cmd_set, NULL, "--if-exists", RW},
+    {"add", 4, INT_MAX, pre_cmd_add, cmd_add, NULL, "--if-exists", RW},
+    {"remove", 4, INT_MAX, pre_cmd_remove, cmd_remove, NULL, "--if-exists",
+     RW},
+    {"clear", 3, INT_MAX, pre_cmd_clear, cmd_clear, NULL, "--if-exists", RW},
     {"create", 2, INT_MAX, pre_create, cmd_create, post_create, "--id=", RW},
     {"destroy", 1, INT_MAX, pre_cmd_destroy, cmd_destroy, NULL,
      "--if-exists,--all", RW},
-- 
1.7.10.4




More information about the dev mailing list