Skip to content

Commit

Permalink
ovs-vsctl: Add --if-exists option to many database commands.
Browse files Browse the repository at this point in the history 
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.

This makes a very small incompatible change to the ovs-vsctl "get"
command: previously with --if-exists, only keys in map columns were
allowed to be missing; now with --if-eixsts, also records are allowed
to be missing.

CC: Gurucharan Shetty <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
  • Loading branch information
@blp
blp committed Jan 16, 2013
1 parent 329e346 commit e99d41f
Show file tree
Hide file tree
Showing 4 changed files with 114 additions and 38 deletions.
2 changes: 2 additions & 0 deletions NEWS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
24 changes: 24 additions & 0 deletions tests/ovs-vsctl.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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])
Expand All @@ -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])
Expand Down Expand Up @@ -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])
Expand Down
48 changes: 36 additions & 12 deletions utilities/ovs-vsctl.8.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand Down
78 changes: 52 additions & 26 deletions utilities/ovs-vsctl.c
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down Expand Up @@ -2943,16 +2936,24 @@ 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;
const struct ovsdb_idl_row *row;
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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -3302,14 +3308,19 @@ 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;
const struct ovsdb_idl_row *row;
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);
}
Expand All @@ -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];
Expand All @@ -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);
Expand Down Expand Up @@ -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];
Expand All @@ -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);
Expand Down Expand Up @@ -3457,14 +3476,19 @@ 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;
const struct ovsdb_idl_row *row;
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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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},
Expand Down

0 comments on commit e99d41f

Please sign in to comment.