[ovs-dev] [kernel-reload 3/8] ovs-vsctl: Add --columns options to "list" command.

Ben Pfaff blp at nicira.com
Tue Feb 8 18:29:04 UTC 2011


On Mon, Feb 07, 2011 at 06:13:11PM -0800, Ethan Jackson wrote:
> > @@ -533,6 +533,12 @@ sflow ? ? ? ? ? ? ? : []
> > ?<0>
> > ?]], [ignore], [test ! -e pid || kill `cat pid`])
> > ?AT_CHECK(
> > + ?[RUN_OVS_VSCTL([--columns=fail_mode,name,datapath_type list b])],
> > + ?[0],
> > + ?[[fail_mode ? ? ? ? ? : []
> > +name ? ? ? ? ? ? ? ?: "br0"
> > +datapath_type ? ? ? : ""
> > +]], [ignore], [test ! -e pid || kill `cat pid`])
> > ? [RUN_OVS_VSCTL(
> > ? ? [set bridge br0 \
> > ? ? ? 'other_config:datapath_id="0123456789ab"' \
> 
> This new test fails for me with the following output:
> 862: database commands -- positive checks
> /home/root/ovs/tests/testsuite.dir/at-groups/862/test-source: line
> 248: syntax error near unexpected token `newline'
> /home/root/ovs/tests/testsuite.dir/at-groups/862/test-source: line
> 248: `  RUN_OVS_VSCTL('
> testsuite: WARNING: unable to parse test group: 862
> testsuite: WARNING: A failure happened in a test group before any test could be
> testsuite: WARNING: run. This means that test suite is improperly
> designed.  Please
> testsuite: WARNING: report this failure to <ovs-bugs at openvswitch.org>.

Oops.  Fixed, thanks.  (I swear I tested this.)

> > +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.
> 
> Otherwise needs a comma following it.  I'm not sure if listed does or
> not.  I'm not known for my incredible grammatical skill so feel free
> to ignore me on this one if I'm wrong.

I added a comma, thanks.

> > +.IP
> > +The UUIDs shown for rows created in the same \fBovs\-vsctl\fR
> > +invocation will be wrong.
> > +.
> > +.IP
> > ?The UUIDs shown for rows created in the same \fBovs\-vsctl\fR
> > ?invocation will be wrong.
> 
> This phrase is repeated twice.

Oops, I removed one.

> > @@ -2587,9 +2662,11 @@ cmd_list(struct vsctl_context *ctx)
> > ? ? ? ? ? ? if (!first) {
> > ? ? ? ? ? ? ? ? ds_put_char(out, '\n');
> > ? ? ? ? ? ? }
> > - ? ? ? ? ? ?list_record(table, row, out);
> > + ? ? ? ? ? ?list_record(row, columns, n_columns, out);
> > ? ? ? ? }
> > ? ? }
> > + ? ?free(columns);
> > +}
> > ?}
> 
> Extra "}"

I swear I tested this, really.

> Other than the above comments, this seems fine.

Thanks, I applied the following incremental to fix those problems.  Full
revised patch after that just in case you want to see it.

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index 7224cba..a6f0dbf 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -565,6 +565,7 @@ AT_CHECK(
 name                : "br0"
 datapath_type       : ""
 ]], [ignore], [test ! -e pid || kill `cat pid`])
+AT_CHECK(
   [RUN_OVS_VSCTL(
     [set bridge br0 \
       'other_config:datapath_id="0123456789ab"' \
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 8ccb5a3..79a1486 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -548,11 +548,8 @@ 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
+listed, in the specified order.  Otherwise, all columns are listed, in
 alphabetical order by column name.
-.IP
-The UUIDs shown for rows created in the same \fBovs\-vsctl\fR
-invocation will be wrong.
 .
 .IP
 The UUIDs shown for rows created in the same \fBovs\-vsctl\fR
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 6e5cda2..18bf63b 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -2789,7 +2789,6 @@ cmd_list(struct vsctl_context *ctx)
     }
     free(columns);
 }
-}
 
 static void
 pre_cmd_set(struct vsctl_context *ctx)


--8<--------------------------cut here-------------------------->8--

From: Ben Pfaff <blp at nicira.com>
Date: Tue, 8 Feb 2011 10:27:35 -0800
Subject: [PATCH] ovs-vsctl: Add --columns options to "list" command.

This allows the user to list just selected columns from a table,
for example just the "name" column.

This will become more useful as additional formatting options
are added in upcoming commits.
---
 tests/ovs-vsctl.at       |    7 +++
 utilities/ovs-vsctl.8.in |    9 +++-
 utilities/ovs-vsctl.c    |  114 ++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 109 insertions(+), 21 deletions(-)

diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index bb49450..a6f0dbf 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -559,6 +559,13 @@ sflow               : []
 <0>
 ]], [ignore], [test ! -e pid || kill `cat pid`])
 AT_CHECK(
+  [RUN_OVS_VSCTL([--columns=fail_mode,name,datapath_type list b])],
+  [0],
+  [[fail_mode           : []
+name                : "br0"
+datapath_type       : ""
+]], [ignore], [test ! -e pid || kill `cat pid`])
+AT_CHECK(
   [RUN_OVS_VSCTL(
     [set bridge br0 \
       'other_config:datapath_id="0123456789ab"' \
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 5e3ab39..79a1486 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -543,10 +543,15 @@ as \fB{}\fR, and curly braces may be optionally enclose non-empty maps
 as well.
 .
 .ST "Database Command Syntax"
-.IP "\fBlist \fItable \fR[\fIrecord\fR]..."
-List the values of all columns of each specified \fIrecord\fR.  If no
+.IP "[\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
 The UUIDs shown for rows created in the same \fBovs\-vsctl\fR
 invocation will be wrong.
 .
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index d70cb79..18bf63b 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -2651,53 +2651,128 @@ cmd_get(struct vsctl_context *ctx)
 }
 
 static void
+parse_column_names(const char *column_names,
+                   const struct vsctl_table_class *table,
+                   const struct ovsdb_idl_column ***columnsp,
+                   size_t *n_columnsp)
+{
+    const struct ovsdb_idl_column **columns;
+    size_t n_columns;
+
+    if (!column_names) {
+        size_t i;
+
+        n_columns = table->class->n_columns + 1;
+        columns = xmalloc(n_columns * sizeof *columns);
+        columns[0] = NULL;
+        for (i = 0; i < table->class->n_columns; i++) {
+            columns[i + 1] = &table->class->columns[i];
+        }
+    } else {
+        char *s = xstrdup(column_names);
+        size_t allocated_columns;
+        char *save_ptr = NULL;
+        char *column_name;
+
+        columns = NULL;
+        allocated_columns = n_columns = 0;
+        for (column_name = strtok_r(s, ", ", &save_ptr); column_name;
+             column_name = strtok_r(NULL, ", ", &save_ptr)) {
+            const struct ovsdb_idl_column *column;
+
+            if (!strcasecmp(column_name, "_uuid")) {
+                column = NULL;
+            } else {
+                die_if_error(get_column(table, column_name, &column));
+            }
+            if (n_columns >= allocated_columns) {
+                columns = x2nrealloc(columns, &allocated_columns,
+                                     sizeof *columns);
+            }
+            columns[n_columns++] = column;
+        }
+        free(s);
+
+        if (!n_columns) {
+            vsctl_fatal("must specify at least one column name");
+        }
+    }
+    *columnsp = columns;
+    *n_columnsp = n_columns;
+}
+
+
+static void
+pre_list_columns(struct vsctl_context *ctx,
+                 const struct vsctl_table_class *table,
+                 const char *column_names)
+{
+    const struct ovsdb_idl_column **columns;
+    size_t n_columns;
+    size_t i;
+
+    parse_column_names(column_names, table, &columns, &n_columns);
+    for (i = 0; i < n_columns; i++) {
+        if (columns[i]) {
+            ovsdb_idl_add_column(ctx->idl, columns[i]);
+        }
+    }
+    free(columns);
+}
+
+static void
 pre_cmd_list(struct vsctl_context *ctx)
 {
+    const char *column_names = shash_find_data(&ctx->options, "--columns");
     const char *table_name = ctx->argv[1];
     const struct vsctl_table_class *table;
-    size_t i;
 
     table = pre_get_table(ctx, table_name);
-    for (i = 0; i < table->class->n_columns; i++) {
-        ovsdb_idl_add_column(ctx->idl, &table->class->columns[i]);
-    }
+    pre_list_columns(ctx, table, column_names);
 }
 
 static void
-list_record(const struct vsctl_table_class *table,
-            const struct ovsdb_idl_row *row, struct ds *out)
+list_record(const struct ovsdb_idl_row *row,
+            const struct ovsdb_idl_column **columns, size_t n_columns,
+            struct ds *out)
 {
     size_t i;
 
-    ds_put_format(out, "%-20s: "UUID_FMT"\n", "_uuid",
-                  UUID_ARGS(&row->uuid));
-    for (i = 0; i < table->class->n_columns; i++) {
-        const struct ovsdb_idl_column *column = &table->class->columns[i];
-        const struct ovsdb_datum *datum;
-
-        datum = ovsdb_idl_read(row, column);
+    for (i = 0; i < n_columns; i++) {
+        const struct ovsdb_idl_column *column = columns[i];
 
-        ds_put_format(out, "%-20s: ", column->name);
-        ovsdb_datum_to_string(datum, &column->type, out);
-        ds_put_char(out, '\n');
+        if (!column) {
+            ds_put_format(out, "%-20s: "UUID_FMT"\n", "_uuid",
+                          UUID_ARGS(&row->uuid));
+        } else {
+            const struct ovsdb_datum *datum = ovsdb_idl_read(row, column);
+            ds_put_format(out, "%-20s: ", column->name);
+            ovsdb_datum_to_string(datum, &column->type, out);
+            ds_put_char(out, '\n');
+        }
     }
 }
 
 static void
 cmd_list(struct vsctl_context *ctx)
 {
+    const char *column_names = shash_find_data(&ctx->options, "--columns");
+    const struct ovsdb_idl_column **columns;
     const char *table_name = ctx->argv[1];
     const struct vsctl_table_class *table;
     struct ds *out = &ctx->output;
+    size_t n_columns;
     int i;
 
     table = get_table(table_name);
+    parse_column_names(column_names, table, &columns, &n_columns);
     if (ctx->argc > 2) {
         for (i = 2; i < ctx->argc; i++) {
             if (i > 2) {
                 ds_put_char(out, '\n');
             }
-            list_record(table, must_get_row(ctx, table, ctx->argv[i]), out);
+            list_record(must_get_row(ctx, table, ctx->argv[i]),
+                        columns, n_columns, out);
         }
     } else {
         const struct ovsdb_idl_row *row;
@@ -2709,9 +2784,10 @@ cmd_list(struct vsctl_context *ctx)
             if (!first) {
                 ds_put_char(out, '\n');
             }
-            list_record(table, row, out);
+            list_record(row, columns, n_columns, out);
         }
     }
+    free(columns);
 }
 
 static void
@@ -3417,7 +3493,7 @@ static const struct vsctl_command_syntax all_commands[] = {
 
     /* Parameter commands. */
     {"get", 2, INT_MAX, pre_cmd_get, cmd_get, NULL, "--if-exists,--id=", RO},
-    {"list", 1, INT_MAX, pre_cmd_list, cmd_list, NULL, "", RO},
+    {"list", 1, INT_MAX, pre_cmd_list, cmd_list, 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},
-- 
1.7.1





More information about the dev mailing list