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 <[email protected]>.
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 <[email protected]>
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
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
