Looks good. In the tests I think it would be a good idea to add a comment explaining why there are so many blank lines in the "database commands -- conditions" test. Someone is going to think it's a mistake and delete some.
Ethan On Wed, Nov 30, 2011 at 10:45, Ben Pfaff <b...@nicira.com> wrote: > Requested-by: Shih-Hao Li <s...@nicira.com> > --- > tests/ovs-vsctl.at | 136 > +++++++++++++++++++++++++++++++++++++++++++++- > utilities/ovs-vsctl.8.in | 55 +++++++++++++++++-- > utilities/ovs-vsctl.c | 138 > +++++++++++++++++++++++++++++++++++----------- > 3 files changed, 290 insertions(+), 39 deletions(-) > > diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at > index 73e2b52..8ade172 100644 > --- a/tests/ovs-vsctl.at > +++ b/tests/ovs-vsctl.at > @@ -692,7 +692,7 @@ 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([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. > + [1], [], [ovs-vsctl: datapath_id:y,z: argument does not end in "=", "!=", > "<", ">", "<=", ">=", "{=}", "{!=}", "{<}", "{>}", "{<=}", or "{>=}" followed > by a value. > ], [OVS_VSCTL_CLEANUP]) > AT_CHECK([RUN_OVS_VSCTL([get b br0 datapath_id::])], > [1], [], [ovs-vsctl: datapath_id::: trailing garbage ":" in argument > @@ -730,6 +730,140 @@ AT_CHECK([RUN_OVS_VSCTL([destroy b br2])], > OVS_VSCTL_CLEANUP > AT_CLEANUP > > +AT_SETUP([database commands -- conditions]) > +AT_KEYWORDS([ovs-vsctl]) > +trap 'kill `cat pid`' 0 > +OVS_VSCTL_SETUP > +AT_CHECK( > + [RUN_OVS_VSCTL_TOGETHER( > + [add-br br0], > + [add-br br1], [set bridge br1 flood_vlans=0 other-config:x='""'], > + [add-br br2], [set bridge br2 flood_vlans=1 other-config:x=y], > + [add-br br3], [set bridge br3 flood_vlans=0,1 other-config:x=z], > + [add-br br4], [set bridge br4 flood_vlans=2], > + [add-br br5], [set bridge br5 flood_vlans=0,2], > + [add-br br6], [set bridge br6 flood_vlans=1,2], > + [add-br br7], [set bridge br7 flood_vlans=0,1,2])], [0], [ > + > + > + > + > + > + > + > + > + > + > + > + > + > + > +]) > +m4_define([VSCTL_CHECK_FIND], > + [AT_CHECK([ovs-vsctl --bare --timeout=5 --no-wait -vreconnect:ANY:emer > --db=unix:socket -- --columns=name find bridge '$1' | sort | xargs echo], > [0], [$2 > +])]) > + > +# Arithmetic relational operators without keys. > +VSCTL_CHECK_FIND([flood_vlans=0], [br1]) > +VSCTL_CHECK_FIND([flood_vlans=1], [br2]) > +VSCTL_CHECK_FIND([flood_vlans=0,2], [br5]) > +VSCTL_CHECK_FIND([flood_vlans=0,1,2], [br7]) > +VSCTL_CHECK_FIND([flood_vlans=3], []) > + > +VSCTL_CHECK_FIND([flood_vlans!=0], [br0 br2 br3 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([flood_vlans!=1], [br0 br1 br3 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([flood_vlans!=0,2], [br0 br1 br2 br3 br4 br6 br7]) > +VSCTL_CHECK_FIND([flood_vlans!=0,1,2], [br0 br1 br2 br3 br4 br5 br6]) > +VSCTL_CHECK_FIND([flood_vlans!=3], [br0 br1 br2 br3 br4 br5 br6 br7]) > + > +VSCTL_CHECK_FIND([flood_vlans<2], [br0 br1 br2]) > +VSCTL_CHECK_FIND([flood_vlans<0,2], [br0 br1 br2 br3 br4]) > +VSCTL_CHECK_FIND([flood_vlans>1], [br3 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([flood_vlans>0,1], [br5 br6 br7]) > +VSCTL_CHECK_FIND([flood_vlans<=2], [br0 br1 br2 br4]) > +VSCTL_CHECK_FIND([flood_vlans<=0,2], [br0 br1 br2 br3 br4 br5]) > +VSCTL_CHECK_FIND([flood_vlans>=1], [br2 br3 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([flood_vlans>=0,1], [br3 br5 br6 br7]) > + > +# Set relational operators without keys. > +VSCTL_CHECK_FIND([flood_vlans{=}0], [br1]) > +VSCTL_CHECK_FIND([flood_vlans{=}1], [br2]) > +VSCTL_CHECK_FIND([flood_vlans{=}0,2], [br5]) > +VSCTL_CHECK_FIND([flood_vlans{=}0,1,2], [br7]) > +VSCTL_CHECK_FIND([flood_vlans{=}3], []) > + > +VSCTL_CHECK_FIND([flood_vlans{!=}0], [br0 br2 br3 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([flood_vlans{!=}1], [br0 br1 br3 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([flood_vlans{!=}0,2], [br0 br1 br2 br3 br4 br6 br7]) > +VSCTL_CHECK_FIND([flood_vlans{!=}0,1,2], [br0 br1 br2 br3 br4 br5 br6]) > +VSCTL_CHECK_FIND([flood_vlans{!=}3], [br0 br1 br2 br3 br4 br5 br6 br7]) > + > +VSCTL_CHECK_FIND([flood_vlans{<}[[]]], []) > +VSCTL_CHECK_FIND([flood_vlans{<=}[[]]], [br0]) > +VSCTL_CHECK_FIND([flood_vlans{<}0], [br0]) > +VSCTL_CHECK_FIND([flood_vlans{<=}0], [br0 br1]) > +VSCTL_CHECK_FIND([flood_vlans{<}1,2], [br0 br2 br4]) > +VSCTL_CHECK_FIND([flood_vlans{<=}1,2], [br0 br2 br4 br6]) > + > +VSCTL_CHECK_FIND([flood_vlans{>}[[]]], [br1 br2 br3 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([flood_vlans{>=}[[]]], [br0 br1 br2 br3 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([flood_vlans{>}0], [br3 br5 br7]) > +VSCTL_CHECK_FIND([flood_vlans{>=}0], [br1 br3 br5 br7]) > +VSCTL_CHECK_FIND([flood_vlans{>}0,2], [br7]) > +VSCTL_CHECK_FIND([flood_vlans{>=}1,2], [br6 br7]) > +VSCTL_CHECK_FIND([flood_vlans{>=}0,2], [br5 br7]) > + > +# Arithmetic relational operators with keys. > +VSCTL_CHECK_FIND([other-config:x=""], [br1]) > +VSCTL_CHECK_FIND([other-config:x=y], [br2]) > +VSCTL_CHECK_FIND([other-config:x=z], [br3]) > + > +VSCTL_CHECK_FIND([other-config:x!=""], [br2 br3]) > +VSCTL_CHECK_FIND([other-config:x!=y], [br1 br3]) > +VSCTL_CHECK_FIND([other-config:x!=z], [br1 br2]) > + > +VSCTL_CHECK_FIND([other-config:x>y], [br3]) > +VSCTL_CHECK_FIND([other-config:x>=y], [br2 br3]) > +VSCTL_CHECK_FIND([other-config:x<y], [br1]) > +VSCTL_CHECK_FIND([other-config:x<=y], [br1 br2]) > + > +# Set relational operators with keys. > +VSCTL_CHECK_FIND([other-config:x{=}[[]]], [br0 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([other-config:x{=}""], [br1]) > +VSCTL_CHECK_FIND([other-config:x{=}y], [br2]) > +VSCTL_CHECK_FIND([other-config:x{=}z], [br3]) > + > +VSCTL_CHECK_FIND([other-config:x{!=}[[]]], [br1 br2 br3]) > +VSCTL_CHECK_FIND([other-config:x{!=}""], [br0 br2 br3 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([other-config:x{!=}y], [br0 br1 br3 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([other-config:x{!=}z], [br0 br1 br2 br4 br5 br6 br7]) > + > +VSCTL_CHECK_FIND([other-config:x{<=}[[]]], [br0 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([other-config:x{<=}x], [br0 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([other-config:x{<=}""], [br0 br1 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([other-config:x{<=}y], [br0 br2 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([other-config:x{<=}z], [br0 br3 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([other-config:x{<=}x,y,z], [br0 br2 br3 br4 br5 br6 br7]) > + > +VSCTL_CHECK_FIND([other-config:x{<}[[]]], []) > +VSCTL_CHECK_FIND([other-config:x{<}x], [br0 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([other-config:x{<}""], [br0 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([other-config:x{<}y], [br0 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([other-config:x{<}z], [br0 br4 br5 br6 br7]) > + > +VSCTL_CHECK_FIND([other-config:x{>=}[[]]], [br0 br1 br2 br3 br4 br5 br6 br7]) > +VSCTL_CHECK_FIND([other-config:x{>=}x], []) > +VSCTL_CHECK_FIND([other-config:x{>=}""], [br1]) > +VSCTL_CHECK_FIND([other-config:x{>=}y], [br2]) > +VSCTL_CHECK_FIND([other-config:x{>=}z], [br3]) > + > +VSCTL_CHECK_FIND([other-config:x{>}[[]]], [br1 br2 br3]) > +VSCTL_CHECK_FIND([other-config:x{>}x], []) > +VSCTL_CHECK_FIND([other-config:x{>}""], []) > +VSCTL_CHECK_FIND([other-config:x{>}y], []) > +VSCTL_CHECK_FIND([other-config:x{>}z], []) > +AT_CLEANUP > + > AT_SETUP([database commands -- wait-until immediately true]) > AT_KEYWORDS([ovs-vsctl]) > OVS_VSCTL_SETUP > diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in > index 64255b5..a4a6766 100644 > --- a/utilities/ovs-vsctl.8.in > +++ b/utilities/ovs-vsctl.8.in > @@ -573,11 +573,56 @@ alphabetical order by column name. > .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 > \fIvalue\fR or, if \fIkey\fR is specified, whose \fIcolumn\fR contains > -a \fIkey\fR with the specified \fIvalue\fR. Any of the operators > -\fB!=\fR, \fB<\fR, \fB>\fR, \fB<=\fR, or \fB>=\fR may be substituted > -for \fB=\fR to test for inequality, less than, greater than, less than > -or equal to, or greater than or equal to, respectively. (Don't forget > -to escape \fB<\fR or \fB>\fR from interpretation by the shell.) > +a \fIkey\fR with the specified \fIvalue\fR. The following operators > +may be used where \fB=\fR is written in the syntax summary: > +.RS > +.IP "\fB= != < > <= >=\fR" > +Selects records in which \fIcolumn\fR[\fB:\fIkey\fR] equals, does not > +equal, is less than, is greater than, is less than or equal to, or is > +greater than or equal to \fIvalue\fR, respectively. > +.IP > +Consider \fIcolumn\fR[\fB:\fIkey\fR] and \fIvalue\fR as sets of > +elements. Identical sets are considered equal. Otherwise, if the > +sets have different numbers of elements, then the set with more > +elements is considered to be larger. Otherwise, consider a element > +from each set pairwise, in increasing order within each set. The > +first pair that differs determines the result. (For a column that > +contains key-value pairs, first all the keys are compared, and values > +are considered only if the two sets contain identical keys.) > +.IP "\fB{=} {!=}\fR" > +Test for set equality or inequality, respectively. > +.IP "\fB{<=}\fR" > +Selects records in which \fIcolumn\fR[\fB:\fIkey\fR] is a subset of > +\fIvalue\fR. For example, \fBflood-vlans{<=}1,2\fR selects records in > +which the \fBflood-vlans\fR column is the empty set or contains 1 or 2 > +or both. > +.IP "\fB{<}\fR" > +Selects records in which \fIcolumn\fR[\fB:\fIkey\fR] is a proper > +subset of \fIvalue\fR. For example, \fBflood-vlans{<}1,2\fR selects > +records in which the \fBflood-vlans\fR column is the empty set or > +contains 1 or 2 but not both. > +.IP "\fB{>=} {>}\fR" > +Same as \fB{<=}\fR and \fB{<}\fR, respectively, except that the > +relationship is reversed. For example, \fBflood-vlans{>=}1,2\fR > +selects records in which the \fBflood-vlans\fR column contains both 1 > +and 2. > +.RE > +.IP > +For arithmetic operators (\fB= != < > <= >=\fR), when \fIkey\fR is > +specified but a particular record's \fIcolumn\fR does not contain > +\fIkey\fR, the record is always omitted from the results. Thus, the > +condition \fBother-config:mtu!=1500\fR matches records that have a > +\fBmtu\fR key whose value is not 1500, but not those that lack an > +\fBmtu\fR key. > +.IP > +For the set operators, when \fIkey\fR is specified but a particular > +record's \fIcolumn\fR does not contain \fIkey\fR, the comparison is > +done against an empty set. Thus, the condition > +\fBother-config:mtu{!=}1500\fR matches records that have a \fBmtu\fR > +key whose value is not 1500 and those that lack an \fBmtu\fR key. > +.IP > +Don't forget to escape \fB<\fR or \fB>\fR from interpretation by the > +shell. > .IP > If \fB\-\-columns\fR is specified, only the requested columns are > listed, in the specified order. Otherwise all columns are listed, in > diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c > index 1f0f485..a2af2f6 100644 > --- a/utilities/ovs-vsctl.c > +++ b/utilities/ovs-vsctl.c > @@ -2619,10 +2619,9 @@ missing_operator_error(const char *arg, const char > **allowed_operators, > * - If 'valuep' is nonnull, an operator followed by a value string. The > * allowed operators are the 'n_allowed' string in 'allowed_operators', > * or just "=" if 'n_allowed' is 0. If 'operatorp' is nonnull, then > the > - * operator is stored into '*operatorp' (one of the pointers from > - * 'allowed_operators' is stored; nothing is malloc()'d). The value > is > - * stored as a malloc()'d string into '*valuep', or NULL if no value > is > - * present in 'arg'. > + * index of the operator within 'allowed_operators' is stored into > + * '*operatorp'. The value is stored as a malloc()'d string into > + * '*valuep', or NULL if no value is present in 'arg'. > * > * On success, returns NULL. On failure, returned a malloc()'d string error > * message and stores NULL into all of the nonnull output arguments. */ > @@ -2630,7 +2629,7 @@ static char * WARN_UNUSED_RESULT > parse_column_key_value(const char *arg, > const struct vsctl_table_class *table, > const struct ovsdb_idl_column **columnp, char **keyp, > - const char **operatorp, > + int *operatorp, > const char **allowed_operators, size_t n_allowed, > char **valuep) > { > @@ -2671,9 +2670,9 @@ parse_column_key_value(const char *arg, > > /* Parse value string. */ > if (valuep) { > - const char *best; > size_t best_len; > size_t i; > + int best; > > if (!allowed_operators) { > static const char *equals = "="; > @@ -2681,7 +2680,7 @@ parse_column_key_value(const char *arg, > n_allowed = 1; > } > > - best = NULL; > + best = -1; > best_len = 0; > for (i = 0; i < n_allowed; i++) { > const char *op = allowed_operators[i]; > @@ -2689,10 +2688,10 @@ parse_column_key_value(const char *arg, > > if (op_len > best_len && !strncmp(op, p, op_len) && p[op_len]) { > best_len = op_len; > - best = op; > + best = i; > } > } > - if (!best) { > + if (best < 0) { > error = missing_operator_error(arg, allowed_operators, n_allowed); > goto error; > } > @@ -2718,7 +2717,7 @@ error: > free(*valuep); > *valuep = NULL; > if (operatorp) { > - *operatorp = NULL; > + *operatorp = -1; > } > } > return error; > @@ -3401,22 +3400,86 @@ cmd_destroy(struct vsctl_context *ctx) > } > } > > +#define RELOPS \ > + RELOP(RELOP_EQ, "=") \ > + RELOP(RELOP_NE, "!=") \ > + RELOP(RELOP_LT, "<") \ > + RELOP(RELOP_GT, ">") \ > + RELOP(RELOP_LE, "<=") \ > + RELOP(RELOP_GE, ">=") \ > + RELOP(RELOP_SET_EQ, "{=}") \ > + RELOP(RELOP_SET_NE, "{!=}") \ > + RELOP(RELOP_SET_LT, "{<}") \ > + RELOP(RELOP_SET_GT, "{>}") \ > + RELOP(RELOP_SET_LE, "{<=}") \ > + RELOP(RELOP_SET_GE, "{>=}") > + > +enum relop { > +#define RELOP(ENUM, STRING) ENUM, > + RELOPS > +#undef RELOP > +}; > + > +static bool > +is_set_operator(enum relop op) > +{ > + return (op == RELOP_SET_EQ || op == RELOP_SET_NE || > + op == RELOP_SET_LT || op == RELOP_SET_GT || > + op == RELOP_SET_LE || op == RELOP_SET_GE); > +} > + > +static bool > +evaluate_relop(const struct ovsdb_datum *a, const struct ovsdb_datum *b, > + const struct ovsdb_type *type, enum relop op) > +{ > + switch (op) { > + case RELOP_EQ: > + case RELOP_SET_EQ: > + return ovsdb_datum_compare_3way(a, b, type) == 0; > + case RELOP_NE: > + case RELOP_SET_NE: > + return ovsdb_datum_compare_3way(a, b, type) != 0; > + case RELOP_LT: > + return ovsdb_datum_compare_3way(a, b, type) < 0; > + case RELOP_GT: > + return ovsdb_datum_compare_3way(a, b, type) > 0; > + case RELOP_LE: > + return ovsdb_datum_compare_3way(a, b, type) <= 0; > + case RELOP_GE: > + return ovsdb_datum_compare_3way(a, b, type) >= 0; > + > + case RELOP_SET_LT: > + return b->n > a->n && ovsdb_datum_includes_all(a, b, type); > + case RELOP_SET_GT: > + return a->n > b->n && ovsdb_datum_includes_all(b, a, type); > + case RELOP_SET_LE: > + return ovsdb_datum_includes_all(a, b, type); > + case RELOP_SET_GE: > + return ovsdb_datum_includes_all(b, a, type); > + > + default: > + NOT_REACHED(); > + } > +} > + > static bool > is_condition_satisfied(const struct vsctl_table_class *table, > const struct ovsdb_idl_row *row, const char *arg, > struct ovsdb_symbol_table *symtab) > { > static const char *operators[] = { > - "=", "!=", "<", ">", "<=", ">=" > +#define RELOP(ENUM, STRING) STRING, > + RELOPS > +#undef RELOP > }; > > const struct ovsdb_idl_column *column; > const struct ovsdb_datum *have_datum; > char *key_string, *value_string; > - const char *operator; > - unsigned int idx; > + struct ovsdb_type type; > + int operator; > + bool retval; > char *error; > - int cmp = 0; > > error = parse_column_key_value(arg, table, &column, &key_string, > &operator, operators, > ARRAY_SIZE(operators), > @@ -3426,9 +3489,14 @@ is_condition_satisfied(const struct vsctl_table_class > *table, > vsctl_fatal("%s: missing value", arg); > } > > + type = column->type; > + type.n_max = UINT_MAX; > + > have_datum = ovsdb_idl_read(row, column); > if (key_string) { > - union ovsdb_atom want_key, want_value; > + union ovsdb_atom want_key; > + struct ovsdb_datum b; > + unsigned int idx; > > if (column->type.value.type == OVSDB_TYPE_VOID) { > vsctl_fatal("cannot specify key to check for non-map column %s", > @@ -3437,41 +3505,45 @@ is_condition_satisfied(const struct vsctl_table_class > *table, > > die_if_error(ovsdb_atom_from_string(&want_key, &column->type.key, > key_string, symtab)); > - die_if_error(ovsdb_atom_from_string(&want_value, &column->type.value, > - value_string, symtab)); > + > + type.key = type.value; > + type.value.type = OVSDB_TYPE_VOID; > + die_if_error(ovsdb_datum_from_string(&b, &type, value_string, > symtab)); > > idx = ovsdb_datum_find_key(have_datum, > &want_key, column->type.key.type); > - if (idx != UINT_MAX) { > - cmp = ovsdb_atom_compare_3way(&have_datum->values[idx], > - &want_value, > - column->type.value.type); > + if (idx == UINT_MAX && !is_set_operator(operator)) { > + retval = false; > + } else { > + struct ovsdb_datum a; > + > + if (idx != UINT_MAX) { > + a.n = 1; > + a.keys = &have_datum->values[idx]; > + a.values = NULL; > + } else { > + a.n = 0; > + a.keys = NULL; > + a.values = NULL; > + } > + > + retval = evaluate_relop(&a, &b, &type, operator); > } > > ovsdb_atom_destroy(&want_key, column->type.key.type); > - ovsdb_atom_destroy(&want_value, column->type.value.type); > } else { > struct ovsdb_datum want_datum; > > die_if_error(ovsdb_datum_from_string(&want_datum, &column->type, > value_string, symtab)); > - idx = 0; > - cmp = ovsdb_datum_compare_3way(have_datum, &want_datum, > - &column->type); > + retval = evaluate_relop(have_datum, &want_datum, &type, operator); > ovsdb_datum_destroy(&want_datum, &column->type); > } > > free(key_string); > free(value_string); > > - return (idx == UINT_MAX ? false > - : !strcmp(operator, "=") ? cmp == 0 > - : !strcmp(operator, "!=") ? cmp != 0 > - : !strcmp(operator, "<") ? cmp < 0 > - : !strcmp(operator, ">") ? cmp > 0 > - : !strcmp(operator, "<=") ? cmp <= 0 > - : !strcmp(operator, ">=") ? cmp >= 0 > - : (abort(), 0)); > + return retval; > } > > static void > -- > 1.7.2.5 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev