On Tue, Jan 15, 2013 at 1:24 PM, Ben Pfaff <[email protected]> wrote: > 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.
There is a whitespace error. Otherwise looks good. Thanks. I will re-spin my patches. ~Guru > > CC: Gurucharan Shetty <[email protected]> > Signed-off-by: Ben Pfaff <[email protected]> > --- > 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 > _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
