Thank you, I will push this soon.
On Mon, Nov 28, 2011 at 01:02:42PM -0800, Ethan Jackson wrote:
> Looks good to me.
>
> Ethan
>
> On Wed, Nov 16, 2011 at 14:39, Ben Pfaff <b...@nicira.com> wrote:
> > Multiple-clause conditions in OVSDB operations with "where" clauses are
> > supposed to be conjunctions, that is, the condition is true only if every
> > clause is true. ?In fact, the implementation only checked a single clause
> > (not necessarily the first one) and ignored the rest. ?This fixes the
> > problem and adds test coverage for multiple-clause conditions.
> >
> > Reported-by: Shih-Hao Li <s...@nicira.com>
> > ---
> > ?ovsdb/condition.c ? ? ? ?| ? 93
> > +++++++++++++++++++++++++---------------------
> > ?tests/ovsdb-condition.at | ? 52 ++++++++++++++++++--------
> > ?2 files changed, 87 insertions(+), 58 deletions(-)
> >
> > diff --git a/ovsdb/condition.c b/ovsdb/condition.c
> > index 59f742c..c57e419 100644
> > --- a/ovsdb/condition.c
> > +++ b/ovsdb/condition.c
> > @@ -1,4 +1,4 @@
> > -/* Copyright (c) 2009, 2010, 2011 Nicira Networks
> > +/* Copyright (c) 2009, 2010, 2011 Nicira Networks
> > ?*
> > ?* Licensed under the Apache License, Version 2.0 (the "License");
> > ?* you may not use this file except in compliance with the License.
> > @@ -217,6 +217,54 @@ ovsdb_condition_to_json(const struct ovsdb_condition
> > *cnd)
> > ? ? return json_array_create(clauses, cnd->n_clauses);
> > ?}
> >
> > +static bool
> > +ovsdb_clause_evaluate(const struct ovsdb_row *row,
> > + ? ? ? ? ? ? ? ? ? ? ?const struct ovsdb_clause *c)
> > +{
> > + ? ?const struct ovsdb_datum *field = &row->fields[c->column->index];
> > + ? ?const struct ovsdb_datum *arg = &c->arg;
> > + ? ?const struct ovsdb_type *type = &c->column->type;
> > +
> > + ? ?if (ovsdb_type_is_scalar(type)) {
> > + ? ? ? ?int cmp = ovsdb_atom_compare_3way(&field->keys[0], &arg->keys[0],
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?type->key.type);
> > + ? ? ? ?switch (c->function) {
> > + ? ? ? ?case OVSDB_F_LT:
> > + ? ? ? ? ? ?return cmp < 0;
> > + ? ? ? ?case OVSDB_F_LE:
> > + ? ? ? ? ? ?return cmp <= 0;
> > + ? ? ? ?case OVSDB_F_EQ:
> > + ? ? ? ?case OVSDB_F_INCLUDES:
> > + ? ? ? ? ? ?return cmp == 0;
> > + ? ? ? ?case OVSDB_F_NE:
> > + ? ? ? ?case OVSDB_F_EXCLUDES:
> > + ? ? ? ? ? ?return cmp != 0;
> > + ? ? ? ?case OVSDB_F_GE:
> > + ? ? ? ? ? ?return cmp >= 0;
> > + ? ? ? ?case OVSDB_F_GT:
> > + ? ? ? ? ? ?return cmp > 0;
> > + ? ? ? ?}
> > + ? ?} else {
> > + ? ? ? ?switch (c->function) {
> > + ? ? ? ?case OVSDB_F_EQ:
> > + ? ? ? ? ? ?return ovsdb_datum_equals(field, arg, type);
> > + ? ? ? ?case OVSDB_F_NE:
> > + ? ? ? ? ? ?return !ovsdb_datum_equals(field, arg, type);
> > + ? ? ? ?case OVSDB_F_INCLUDES:
> > + ? ? ? ? ? ?return ovsdb_datum_includes_all(arg, field, type);
> > + ? ? ? ?case OVSDB_F_EXCLUDES:
> > + ? ? ? ? ? ?return ovsdb_datum_excludes_all(arg, field, type);
> > + ? ? ? ?case OVSDB_F_LT:
> > + ? ? ? ?case OVSDB_F_LE:
> > + ? ? ? ?case OVSDB_F_GE:
> > + ? ? ? ?case OVSDB_F_GT:
> > + ? ? ? ? ? ?NOT_REACHED();
> > + ? ? ? ?}
> > + ? ?}
> > +
> > + ? ?NOT_REACHED();
> > +}
> > +
> > ?bool
> > ?ovsdb_condition_evaluate(const struct ovsdb_row *row,
> > ? ? ? ? ? ? ? ? ? ? ? ? ?const struct ovsdb_condition *cnd)
> > @@ -224,48 +272,9 @@ ovsdb_condition_evaluate(const struct ovsdb_row *row,
> > ? ? size_t i;
> >
> > ? ? for (i = 0; i < cnd->n_clauses; i++) {
> > - ? ? ? ?const struct ovsdb_clause *c = &cnd->clauses[i];
> > - ? ? ? ?const struct ovsdb_datum *field = &row->fields[c->column->index];
> > - ? ? ? ?const struct ovsdb_datum *arg = &cnd->clauses[i].arg;
> > - ? ? ? ?const struct ovsdb_type *type = &c->column->type;
> > -
> > - ? ? ? ?if (ovsdb_type_is_scalar(type)) {
> > - ? ? ? ? ? ?int cmp = ovsdb_atom_compare_3way(&field->keys[0],
> > &arg->keys[0],
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?type->key.type);
> > - ? ? ? ? ? ?switch (c->function) {
> > - ? ? ? ? ? ?case OVSDB_F_LT:
> > - ? ? ? ? ? ? ? ?return cmp < 0;
> > - ? ? ? ? ? ?case OVSDB_F_LE:
> > - ? ? ? ? ? ? ? ?return cmp <= 0;
> > - ? ? ? ? ? ?case OVSDB_F_EQ:
> > - ? ? ? ? ? ?case OVSDB_F_INCLUDES:
> > - ? ? ? ? ? ? ? ?return cmp == 0;
> > - ? ? ? ? ? ?case OVSDB_F_NE:
> > - ? ? ? ? ? ?case OVSDB_F_EXCLUDES:
> > - ? ? ? ? ? ? ? ?return cmp != 0;
> > - ? ? ? ? ? ?case OVSDB_F_GE:
> > - ? ? ? ? ? ? ? ?return cmp >= 0;
> > - ? ? ? ? ? ?case OVSDB_F_GT:
> > - ? ? ? ? ? ? ? ?return cmp > 0;
> > - ? ? ? ? ? ?}
> > - ? ? ? ?} else {
> > - ? ? ? ? ? ?switch (c->function) {
> > - ? ? ? ? ? ?case OVSDB_F_EQ:
> > - ? ? ? ? ? ? ? ?return ovsdb_datum_equals(field, arg, type);
> > - ? ? ? ? ? ?case OVSDB_F_NE:
> > - ? ? ? ? ? ? ? ?return !ovsdb_datum_equals(field, arg, type);
> > - ? ? ? ? ? ?case OVSDB_F_INCLUDES:
> > - ? ? ? ? ? ? ? ?return ovsdb_datum_includes_all(arg, field, type);
> > - ? ? ? ? ? ?case OVSDB_F_EXCLUDES:
> > - ? ? ? ? ? ? ? ?return ovsdb_datum_excludes_all(arg, field, type);
> > - ? ? ? ? ? ?case OVSDB_F_LT:
> > - ? ? ? ? ? ?case OVSDB_F_LE:
> > - ? ? ? ? ? ?case OVSDB_F_GE:
> > - ? ? ? ? ? ?case OVSDB_F_GT:
> > - ? ? ? ? ? ? ? ?NOT_REACHED();
> > - ? ? ? ? ? ?}
> > + ? ? ? ?if (!ovsdb_clause_evaluate(row, &cnd->clauses[i])) {
> > + ? ? ? ? ? ?return false;
> > ? ? ? ? }
> > - ? ? ? ?NOT_REACHED();
> > ? ? }
> >
> > ? ? return true;
> > diff --git a/tests/ovsdb-condition.at b/tests/ovsdb-condition.at
> > index 80961cc..d3d7d83 100644
> > --- a/tests/ovsdb-condition.at
> > +++ b/tests/ovsdb-condition.at
> > @@ -203,7 +203,8 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on
> > integers],
> > ? ? ? [["i", ">=", 1]],
> > ? ? ? [["i", ">", 1]],
> > ? ? ? [["i", "includes", 1]],
> > - ? ? ?[["i", "excludes", 1]]]' \
> > + ? ? ?[["i", "excludes", 1]],
> > + ? ? ?[["i", ">", 0], ["i", "<", 2]]]' \
> > ? ? '[{"i": 0},
> > ? ? ? {"i": 1},
> > ? ? ? {"i": 2}']]],
> > @@ -214,7 +215,8 @@ condition ?3: T-T
> > ?condition ?4: -TT
> > ?condition ?5: --T
> > ?condition ?6: -T-
> > -condition ?7: T-T], [condition])
> > +condition ?7: T-T
> > +condition ?8: -T-], [condition])
> >
> > ?OVSDB_CHECK_POSITIVE([evaluating conditions on reals],
> > ? [[evaluate-conditions \
> > @@ -226,7 +228,8 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on reals],
> > ? ? ? [["r", ">=", 5.0]],
> > ? ? ? [["r", ">", 5.0]],
> > ? ? ? [["r", "includes", 5.0]],
> > - ? ? ?[["r", "excludes", 5.0]]]' \
> > + ? ? ?[["r", "excludes", 5.0]],
> > + ? ? ?[["r", "!=", 0], ["r", "!=", 5.1]]]' \
> > ? ? '[{"r": 0},
> > ? ? ? {"r": 5.0},
> > ? ? ? {"r": 5.1}']]],
> > @@ -237,7 +240,8 @@ condition ?3: T-T
> > ?condition ?4: -TT
> > ?condition ?5: --T
> > ?condition ?6: -T-
> > -condition ?7: T-T], [condition])
> > +condition ?7: T-T
> > +condition ?8: -T-], [condition])
> >
> > ?OVSDB_CHECK_POSITIVE([evaluating conditions on booleans],
> > ? [[evaluate-conditions \
> > @@ -249,7 +253,8 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on
> > booleans],
> > ? ? ? [["b", "==", false]],
> > ? ? ? [["b", "!=", false]],
> > ? ? ? [["b", "includes", false]],
> > - ? ? ?[["b", "excludes", false]]]' \
> > + ? ? ?[["b", "excludes", false]],
> > + ? ? ?[["b", "==", true], ["b", "==", false]]]' \
> > ? ? '[{"b": true},
> > ? ? ? {"b": false}']]],
> > ? [condition ?0: T-
> > @@ -259,7 +264,8 @@ condition ?3: -T
> > ?condition ?4: -T
> > ?condition ?5: T-
> > ?condition ?6: -T
> > -condition ?7: T-], [condition])
> > +condition ?7: T-
> > +condition ?8: --], [condition])
> >
> > ?OVSDB_CHECK_POSITIVE([evaluating conditions on strings],
> > ? [[evaluate-conditions \
> > @@ -271,7 +277,8 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on strings],
> > ? ? ? [["s", "==", "foo"]],
> > ? ? ? [["s", "!=", "foo"]],
> > ? ? ? [["s", "includes", "foo"]],
> > - ? ? ?[["s", "excludes", "foo"]]]' \
> > + ? ? ?[["s", "excludes", "foo"]],
> > + ? ? ?[["s", "!=", "foo"], ["s", "!=", ""]]]' \
> > ? ? '[{"s": ""},
> > ? ? ? {"s": "foo"},
> > ? ? ? {"s": "xxx"}']]],
> > @@ -282,7 +289,8 @@ condition ?3: -TT
> > ?condition ?4: -T-
> > ?condition ?5: T-T
> > ?condition ?6: -T-
> > -condition ?7: T-T], [condition])
> > +condition ?7: T-T
> > +condition ?8: --T], [condition])
> >
> > ?OVSDB_CHECK_POSITIVE([evaluating conditions on UUIDs],
> > ? [[evaluate-conditions \
> > @@ -294,7 +302,9 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on UUIDs],
> > ? ? ? [["u", "==", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]]],
> > ? ? ? [["u", "!=", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]]],
> > ? ? ? [["u", "includes", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]]],
> > - ? ? ?[["u", "excludes", ["uuid",
> > "06151f9d-62d6-4f59-8504-e9765107faa9"]]]]' \
> > + ? ? ?[["u", "excludes", ["uuid",
> > "06151f9d-62d6-4f59-8504-e9765107faa9"]]],
> > + ? ? ?[["u", "!=", ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]],
> > + ? ? ? ["u", "!=", ["uuid", "cb160ed6-92a6-4503-a6aa-a09a09e01f0d"]]]]' \
> > ? ? '[{"u": ["uuid", "8a1dbdb8-416f-4ce9-affa-3332691714b6"]},
> > ? ? ? {"u": ["uuid", "06151f9d-62d6-4f59-8504-e9765107faa9"]},
> > ? ? ? {"u": ["uuid", "00000000-0000-0000-0000-000000000000"]}']]],
> > @@ -305,7 +315,8 @@ condition ?3: -TT
> > ?condition ?4: -T-
> > ?condition ?5: T-T
> > ?condition ?6: -T-
> > -condition ?7: T-T], [condition])
> > +condition ?7: T-T
> > +condition ?8: T-T], [condition])
> >
> > ?OVSDB_CHECK_POSITIVE([evaluating conditions on sets],
> > ? [[evaluate-conditions \
> > @@ -341,7 +352,9 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on sets],
> > ? ? ? [["i", "excludes", ["set", [2]]]],
> > ? ? ? [["i", "excludes", ["set", [2, 0]]]],
> > ? ? ? [["i", "excludes", ["set", [2, 1]]]],
> > - ? ? ?[["i", "excludes", ["set", [2, 1, 0]]]]]' \
> > + ? ? ?[["i", "excludes", ["set", [2, 1, 0]]]],
> > + ? ? ?[["i", "includes", ["set", [0]]],
> > + ? ? ? ["i", "includes", ["set", [1]]]]]' \
> > ? ? '[{"i": ["set", []]},
> > ? ? ? {"i": ["set", [0]]},
> > ? ? ? {"i": ["set", [1]]},
> > @@ -382,7 +395,8 @@ condition 27: T---T ---
> > ?condition 28: TTTT- ---
> > ?condition 29: T-T-- ---
> > ?condition 30: TT--- ---
> > -condition 31: T---- ---], [condition])
> > +condition 31: T---- ---
> > +condition 32: ---T- --T], [condition])
> >
> > ?# This is the same as the "set" test except that it adds values,
> > ?# all of which always match.
> > @@ -423,7 +437,9 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on maps
> > (1)],
> > ? ? ? [["i", "excludes", ["map", [[2, true]]]]],
> > ? ? ? [["i", "excludes", ["map", [[2, true], [0, true]]]]],
> > ? ? ? [["i", "excludes", ["map", [[2, true], [1, false]]]]],
> > - ? ? ?[["i", "excludes", ["map", [[2, true], [1, false], [0, true]]]]]]' \
> > + ? ? ?[["i", "excludes", ["map", [[2, true], [1, false], [0, true]]]]],
> > + ? ? ?[["i", "includes", ["map", [[0, true]]]],
> > + ? ? ? ["i", "includes", ["map", [[1, false]]]]]]' \
> > ? ? '[{"i": ["map", []]},
> > ? ? ? {"i": ["map", [[0, true]]]},
> > ? ? ? {"i": ["map", [[1, false]]]},
> > @@ -464,7 +480,8 @@ condition 27: T---T ---
> > ?condition 28: TTTT- ---
> > ?condition 29: T-T-- ---
> > ?condition 30: TT--- ---
> > -condition 31: T---- ---], [condition])
> > +condition 31: T---- ---
> > +condition 32: ---T- --T], [condition])
> >
> > ?# This is the same as the "set" test except that it adds values,
> > ?# and those values don't always match.
> > @@ -505,7 +522,9 @@ OVSDB_CHECK_POSITIVE([evaluating conditions on maps
> > (2)],
> > ? ? ? [["i", "excludes", ["map", [[2, true]]]]],
> > ? ? ? [["i", "excludes", ["map", [[2, true], [0, true]]]]],
> > ? ? ? [["i", "excludes", ["map", [[2, true], [1, false]]]]],
> > - ? ? ?[["i", "excludes", ["map", [[2, true], [1, false], [0, true]]]]]]' \
> > + ? ? ?[["i", "excludes", ["map", [[2, true], [1, false], [0, true]]]]],
> > + ? ? ?[["i", "includes", ["map", [[0, true]]]],
> > + ? ? ? ["i", "includes", ["map", [[1, false]]]]]]' \
> > ? ? '[{"i": ["map", []]},
> > ? ? ? {"i": ["map", [[0, true]]]},
> > ? ? ? {"i": ["map", [[0, false]]]},
> > @@ -555,4 +574,5 @@ condition 27: T-T-T --TT- --T--
> > ?condition 28: TTTTT TT-T- T----
> > ?condition 29: T-TTT ---T- -----
> > ?condition 30: TTT-T -T-T- T----
> > -condition 31: T-T-T ---T- -----], [condition])
> > +condition 31: T-T-T ---T- -----
> > +condition 32: ----- T---- ---T-], [condition])
> > --
> > 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
