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

Ethan Jackson ethan at nicira.com
Tue Feb 8 02:13:11 UTC 2011


Thanks for writing this up, it looks like it will be really useful
when scripting ovs-vsctl and what not.


> @@ -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>.


> +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.

> +.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.


> @@ -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 "}"

Other than the above comments, this seems fine.




More information about the dev mailing list