On Fri, Jan 22, 2016 at 12:24 AM, Liran Schour <lir...@il.ibm.com> wrote:
> Andy Zhou <az...@ovn.org> wrote on 21/01/2016 11:32:54 PM:
>
> > On Thu, Jan 21, 2016 at 4:41 AM, Liran Schour <lir...@il.ibm.com> wrote:
> > Andy Zhou <az...@ovn.org> wrote on 21/01/2016 11:27:42 AM:
> > > On Sat, Jan 16, 2016 at 12:16 AM, Liran Schour <lir...@il.ibm.com>
> wrote:
> >
> > > <condition> now is a 3-element json array or a boolean value, see
> > > ovsdb-server(1) man page. This functions will be used for
> > > conditional monitoring sessions.
> > >
> > > Signed-off-by: Liran Schour <lir...@il.ibm.com>
> > > ---
> > > ovsdb/condition.c | 270 ++++++++++++++++++++++++++++++++++++
> > > +++++++++--
> > > ovsdb/condition.h | 28 ++++-
> > > ovsdb/query.c | 4 +-
> > > tests/ovsdb-condition.at | 35 +++++-
> > > tests/test-ovsdb.c | 29 ++++-
> > > 5 files changed, 348 insertions(+), 18 deletions(-)
> >
> > > From the last review:
> > > > Should any columns within schema be allowed in condition? or
> only
> > > > the ones being monitored?
> > > > From protocol viewpoint, it would make sense to allow any
> column.
> > > >
> > >
> > > Yes, any columns should be allowed in condition.
> > >
> > > I did not find the implementation to match this. The values used to
> > > conditional comparison is coming
> > > from the monitored table rows, this is required to be part of the
> > > monitored columns? Also tested and verified
> > > that monitor_cond does not work if where clause contains columns not
> > > being monitored.
> > >
> > > It would be nice to clearly define which columns are allowed in the
> > > where clause.
> > >
> > > I suppose if the implementation actually allows any column, than
> > > there may not be a need for
> > > 'columns_index_map' introduced from the last patch.
> > >
> >
>
> > You are right. We have 2 options to fix it:
> > 1. Save in ovsdb_monitor_row all the columns in the db and mark the
> > columns needs to be monitored. Then we will not need the
> > 'columns_index_map' also.
> > 2. If a none monitored column is included in a condition then add
> > this columns to mt->columns and mark it as only for condition
> > evaluation. In this case we do need the 'columns_index_map'.
> > I prefer option 1 as being simpler and save indexing mapping issue.
> > What do you think?
> >
> > If we go with option 1, It may be more straight forward to require
> > columns in condition also be in monitored.
> >
> I think that from protocol standpoint we should allow columns that are not
> being monitored in condition. But still option 1 is valid.
>
> > The end result of Option 2 seems better from protocol standpoint.
> > Implementation wise, I'd like to propose that we add condition to
> > dbmon. Multiple JSONRPCs can share a dbmon
> > if both monitored columns and conditions are the same. Would you
> > please consider this option?
> >
> I have a problem with it. Conditions changes are very dynamic. If we will
> include condition in dbmon at the end we will get to a single dbmon per
> session (and we will need to handle dbmon spliting due to condition
> change).
>
In case conditions are mostly the same, for example, during the start up
phase, there
seems to be no downside to having condition as part of dbmon. Right?
What's the down down side of a single dbmon per session in case every
condition
is different?
I was thinking we can handling condition changes the same as creating a new
monitor
internally.
The plus of this approach is that we don't need to keep track of monitored
columns vs
non-monitored columns. The condition will be evaluated when dbmon receives
any change.
Current cashing logic should just work as is.
> Now we should ask what is the reason to share dbmon between sessions with
> different sets of condition clauses if we can not really use the json
> cache in that case?
> As I see it, we can not use json cache but it will allow us to reduce the
> complexity of: (1) inserting change to the db (2) prepare the update
> notification.
> This optimization is not included in this patch series. I intend to submit
> it based on this patch series.
>
O.K. you have thought about it.
>
> I think that as long as we allow any columns to be in condition. Option 1
> and 2 are OK.
> Option 2 will reduce the size of ovsdb_monitor_row since it will include
> only the columns that are being monitored or included in condition. I
> think that I will go with option 2. What do you think?
>
I don't like having unexpected columns showing up in update messages. For
this reason,
I'd prefer option 2 as well. The implementation as described may lead of
some complicated data structure and code -- but I don't know for sure. It
is your call.
>
> > > The commit message is slightly misleading since ovsdb-server(1)
> > > changes are not contained within this patch,
> > > but in a later patch.
> > >
> >
> > Right will remove this from message.
>
> > > diff --git a/ovsdb/condition.c b/ovsdb/condition.c
> > > index 4baf1bb..3e1cc19 100644
> > > --- a/ovsdb/condition.c
> > > +++ b/ovsdb/condition.c
> > > @@ -19,6 +19,7 @@
> > >
> > > #include <limits.h>
> > >
> > > +#include "bitmap.h"
> > > #include "column.h"
> > > #include "json.h"
> > > #include "ovsdb-error.h"
> > > @@ -64,6 +65,17 @@ ovsdb_clause_from_json(const struct
> > ovsdb_table_schema *ts,
> > > const char *column_name;
> > > struct ovsdb_type type;
> > >
> > > + if (json->type == JSON_TRUE || json->type == JSON_FALSE) {
> > > + function_name = (json->type == JSON_TRUE) ? "true" : "false";
> > > + error = ovsdb_function_from_string(function_name,
> > &clause->function);
> > > +
> > > + /* column and arg fields are not being used with boolean
> function
> > > + * use dummy values */
> > > + clause->column = ovsdb_table_schema_get_column(ts, "_uuid");
> > > + ovsdb_datum_init_default(&clause->arg,
> &clause->column->type);
> > > + return error;
> > > + }
> > > +
> > >
> > > It seems we can simply drop any FALSE if there are more than one
> > > clauses, because
> > > of the or conditions. On the other hand, the presence to True can
> > > be reduced to an empty
> > > condition..
> > >
> > But you still need to remember that you have FALSE clause. For
> > example client does the following: condition_add([false,[x],y[])
> > then does condition_remove([x],[y]) the outcome of these 2
> > operations should be FALSE.
> > With the TRUE clause you still need to remember all other clauses in
> > cas that client will remove the TRUE clause.
>
> > I think we should reject conditions with redundant clauses. [false,
> > [x]], [true, [x]] and [ [x], [x]] should all be rejected, while
> > single [false] is not redundant.
> >
> > Further, we should also reject monitor_cond_update messages that
> > leads to a redundant clauses. Since those conditions are usually
> > machine generated, I'd think they are reasonable.
> > What do you think?
> >
> Sounds good to me. Will implement this in the next patch version.
>
> > We can then define a truth table for handling single term add and
> removal:
> >
> > T + T = T (or reject)
> > T - T = Empty
> > T - F = T (or reject)
> > T + F = T
> >
> > F + T = T
> > F - F = Empty
> > F + F = F (or reject)
> > F - T = F (or reject)
> >
> > {T, F} +/- Empty = {T, F}
> > Empty - * = Empty
> > Empty + {T, F} = {T, F}
> >
> > Empty evaluates to true
> >
> Right.
>
> > > To handle condition changes, you may still want to finish the JSON
> > > conversions, or define the protocol
> > > in such way that the modification of a TRUE condition the same as
> > > modifying an empty condition.
> > >
> > As I see it, TRUE is not equal to empty condition. For example:
> > condition[TRUE + [x]] != condition[EMPTY + [x]]
> > True + [X] will evaluate to true, same as empty.
> >
> > > If JSON_TRUE and JSON_FALSE has been removed, this function can just
> > > expect a JSON_ARRAY.
> > >
> > After the examples above do you still think it is relevant?
> > If we agree that condition should not contain redundant clauses,
> > then either JSON_TRUE or JSON_FALSE has to be the only clause.
> >
> Right.
>
> >
> > > if (json->type != JSON_ARRAY
> > > || json->u.array.n != 3
> > > || json->u.array.elems[0]->type != JSON_STRING
> > > @@ -109,7 +121,8 @@ ovsdb_clause_from_json(const struct
> > > ovsdb_table_schema *ts,
> > > return error;
> > > }
> > > break;
> > > -
> > > + case OVSDB_F_TRUE:
> > > + case OVSDB_F_FALSE:
> > > case OVSDB_F_EQ:
> > > case OVSDB_F_NE:
> > > break;
> > > @@ -164,6 +177,19 @@ compare_clauses_3way(const void *a_, const void
> *b_)
> > > }
> > > }
> > >
> > > +static int
> > > +compare_clauses_3way_with_data(const void *a_, const void *b_)
> > > +{
> > > + const struct ovsdb_clause *a = a_;
> > > + const struct ovsdb_clause *b = b_;
> > > + int res;
> > > +
> > > + res = compare_clauses_3way(a, b);
> > > + return res ? res : ovsdb_datum_compare_3way(&a->arg,
> > > + &b->arg,
> > > + &a->column->type);
> > > + }
> > > +
> > > struct ovsdb_error *
> > > ovsdb_condition_from_json(const struct ovsdb_table_schema *ts,
> > > const struct json *json,
> > >
> > > Should we make sure there is not duplicated clauses here?
> > >
> > >
> > > @@ -173,7 +199,7 @@ ovsdb_condition_from_json(const struct
> > > ovsdb_table_schema *ts,
> > > const struct json_array *array = json_array(json);
> > > size_t i;
> > >
> > > - cnd->clauses = xmalloc(array->n * sizeof *cnd->clauses);
> > > + cnd->clauses = xzalloc(array->n * sizeof *cnd->clauses);
> > > cnd->n_clauses = 0;
> > > for (i = 0; i < array->n; i++) {
> > > struct ovsdb_error *error;
> > > @@ -198,10 +224,16 @@ ovsdb_condition_from_json(const struct
> > > ovsdb_table_schema *ts,
> > > static struct json *
> > > ovsdb_clause_to_json(const struct ovsdb_clause *clause)
> > > {
> > > - return json_array_create_3(
> > > - json_string_create(clause->column->name),
> > > -
> json_string_create(ovsdb_function_to_string(clause->function)),
> > > - ovsdb_datum_to_json(&clause->arg, &clause->column->type));
> > > + if (clause->function != OVSDB_F_TRUE &&
> > > + clause->function != OVSDB_F_FALSE) {
> > > + return json_array_create_3(
> > > + json_string_create(clause->column->name),
> > > + json_string_create(ovsdb_function_to_string
> > > (clause->function)),
> > > + ovsdb_datum_to_json(&clause->arg,
> &clause->column->type));
> > > + }
> > > +
> > > + return json_boolean_create(clause->function == OVSDB_F_TRUE ?
> > > + true : false);
> > > }
> > >
> > > struct json *
> > > @@ -218,13 +250,22 @@ ovsdb_condition_to_json(const struct
> > > ovsdb_condition *cnd)
> > > }
> > >
> > > static bool
> > > -ovsdb_clause_evaluate(const struct ovsdb_row *row,
> > > - const struct ovsdb_clause *c)
> > > +ovsdb_clause_evaluate(const struct ovsdb_datum *fields,
> > > + const struct ovsdb_clause *c,
> > > + const unsigned int columns_index_map[])
> > > {
> > > - const struct ovsdb_datum *field = &row->fields[c->column->index];
> > > + if (c->function == OVSDB_F_TRUE || c->function == OVSDB_F_FALSE)
> {
> > > + return c->function == OVSDB_F_TRUE ? true : false;
> > > + }
> > > +
> > > + const struct ovsdb_datum *field;
> > > const struct ovsdb_datum *arg = &c->arg;
> > > const struct ovsdb_type *type = &c->column->type;
> > >
> > > + field = !columns_index_map ?
> > > + &fields[c->column->index] :
> > > + &fields[columns_index_map[c->column->index]];
> > > +
> > > if (ovsdb_type_is_optional_scalar(type) && field->n == 0) {
> > > switch (c->function) {
> > > case OVSDB_F_LT:
> > > @@ -237,6 +278,9 @@ ovsdb_clause_evaluate(const struct ovsdb_row *row,
> > > case OVSDB_F_NE:
> > > case OVSDB_F_EXCLUDES:
> > > return true;
> > > + case OVSDB_F_TRUE:
> > > + case OVSDB_F_FALSE:
> > > + OVS_NOT_REACHED();
> > > }
> > > } else if (ovsdb_type_is_scalar(type)
> > > || ovsdb_type_is_optional_scalar(type)) {
> > > @@ -257,6 +301,9 @@ ovsdb_clause_evaluate(const struct ovsdb_row *row,
> > > return cmp >= 0;
> > > case OVSDB_F_GT:
> > > return cmp > 0;
> > > + case OVSDB_F_TRUE:
> > > + case OVSDB_F_FALSE:
> > > + OVS_NOT_REACHED();
> > > }
> > > } else {
> > > switch (c->function) {
> > > @@ -272,6 +319,8 @@ ovsdb_clause_evaluate(const struct ovsdb_row *row,
> > > case OVSDB_F_LE:
> > > case OVSDB_F_GE:
> > > case OVSDB_F_GT:
> > > + case OVSDB_F_TRUE:
> > > + case OVSDB_F_FALSE:
> > > OVS_NOT_REACHED();
> > > }
> > > }
> > > @@ -279,14 +328,42 @@ ovsdb_clause_evaluate(const struct ovsdb_row
> *row,
> > > OVS_NOT_REACHED();
> > > }
> > >
> > > +static int
> > > +ovsdb_clause_exists(const struct ovsdb_condition *cnd,
> > > + const struct ovsdb_clause *clause)
> > > +{
> > > + size_t i;
> > > +
> > > + for (i=0; i < cnd->n_clauses; i++) {
> > > + if(!compare_clauses_3way_with_data(&cnd->clauses[i], clause))
> {
> > > + return i;
> > > + }
> > > + }
> > > +
> > > + return -1;
> > > +}
> > > +
> > > +static void
> > > +ovsdb_clone_clause(struct ovsdb_clause *new, struct ovsdb_clause
> *old)
> > > +{
> > > + new->function = old->function;
> > > + new->column = old->column;
> > > + ovsdb_datum_clone(&new->arg,
> > > + &old->arg,
> > > + &old->column->type);
> > > +}
> > > +
> > > bool
> > > ovsdb_condition_evaluate(const struct ovsdb_row *row,
> > > - const struct ovsdb_condition *cnd)
> > > + const struct ovsdb_condition *cnd,
> > > + const unsigned int columns_index_map[])
> > > {
> > > size_t i;
> > >
> > > for (i = 0; i < cnd->n_clauses; i++) {
> > > - if (!ovsdb_clause_evaluate(row, &cnd->clauses[i])) {
> > > + if (!ovsdb_clause_evaluate(row->fields,
> > > + &cnd->clauses[i],
> > > + columns_index_map)) {
> > > return false;
> > > }
> > > }
> > > @@ -294,6 +371,28 @@ ovsdb_condition_evaluate(const struct ovsdb_row
> *row,
> > > return true;
> > > }
> > >
> > > +bool
> > > +ovsdb_condition_evaluate_or_datum(const struct ovsdb_datum
> *row_datum,
> > > + const struct ovsdb_condition *cnd,
> > > + const unsigned int
> columns_index_map[])
> > > The name of this function is not clear about its intention. The
> > > implementation also
> > > duplicate in many parts with ovsdb_condition_evaluate() above.
> > > +{
> > > + size_t i;
> > > +
> > > + if (cnd->n_clauses == 0) {
> > > + return true;
> > > + }
> > > +
> > > + for (i = 0; i < cnd->n_clauses; i++) {
> > > + if (ovsdb_clause_evaluate(row_datum,
> > > + &cnd->clauses[i],
> > > + columns_index_map)) {
> > > + return true;
> > > + }
> > > + }
> > > +
> > > + return false;
> > > +}
> > > +
> > > void
> > > ovsdb_condition_destroy(struct ovsdb_condition *cnd)
> > > {
> > > @@ -303,4 +402,153 @@ ovsdb_condition_destroy(struct ovsdb_condition
> *cnd)
> > > ovsdb_clause_free(&cnd->clauses[i]);
> > > }
> > > free(cnd->clauses);
> > > Should cnd->clauses be set to NULL?
> > > + cnd->n_clauses = 0;
> > > +}
> > > +
> > > +void ovsdb_condition_init(struct ovsdb_condition *cnd)
> > > +{
> > > + cnd->clauses = NULL;
> > > + cnd->n_clauses = 0;
> > > +}
> > > +
> > > +bool ovsdb_condition_empty(const struct ovsdb_condition *cnd)
> > > +{
> > > + return cnd->n_clauses == 0;
> > > +}
> > > +
> > > +int ovsdb_condition_cmp(const struct ovsdb_condition *a,
> > > + const struct ovsdb_condition *b)
> > > +{
> > > + size_t i;
> > > + int res;
> > > +
> > > + if (a->n_clauses != b->n_clauses) {
> > > + return a->n_clauses - b->n_clauses;
> > > + }
> > > +
> > > + /* We assume clauses are sorted */
> > > + for (i = 0; i < a->n_clauses; i++) {
> > > + res = (compare_clauses_3way_with_data(&a->clauses[i],
> > > &b->clauses[i]));
> > > + if (res != 0) {
> > > + return res;
> > > + }
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +void
> > > +ovsdb_condition_clone(struct ovsdb_condition *to,
> > > + const struct ovsdb_condition *from)
> > > +{
> > > + size_t i;
> > > +
> > > + to->clauses = xzalloc(from->n_clauses * sizeof *to->clauses);
> > > +
> > > + for (i = 0; i < from->n_clauses; i++) {
> > > + ovsdb_clone_clause(&to->clauses[i], &from->clauses[i]);
> > > + }
> > > + to->n_clauses = from->n_clauses;
> > > +}
> > > +
> > >
> > > With the way it is implemented, the order of when "add" and
> > > "remove' are applied
> > > is important. May be it should be documented.
> > >
> > Agree. Add is being applied before of remove.
> >
> > > Another possibility is to check and make sure clauses in "add" and
> > > "remove" are orthogonal.
> > > Reject the change if they are not.
> > >
> > To simplify client usage I think we should allow client to add a
> > clause and then remove it in the same change operation. What do you
> think?
> >
> > Is there a use case for this? I can't think of a case that allowing
> > this can be useful.
> > On the other hand, not allowing this removes the need to document
> > and reasoning about the ordering or add and removal.
> >
> Agreed. Will add it to the next patch series.
>
> >
> >
> > > +void
> > > +ovsdb_condition_add(struct ovsdb_condition *to,
> > > + const struct ovsdb_condition *add)
> > > +{
> > > + size_t i, count = 0;
> > > + struct ovsdb_clause *clauses;
> > > + unsigned long int *clause_map = xzalloc(bitmap_n_bytes
> > (add->n_clauses));
> > > + int index = to->n_clauses;;
> > > +
> > > + for (i = 0; i < add->n_clauses; i++) {
> > > + if (ovsdb_clause_exists(to, &add->clauses[i]) == -1) {
> > > + bitmap_set1(clause_map, i);
> > > + count++;
> > > + }
> > > + }
> > > +
> > > + if (!count) {
> > > + free(clause_map);
> > > + return;
> > > + }
> > > + clauses = xzalloc((to->n_clauses + count) * sizeof *clauses);
> > > +
> > > + for (i = 0; i < to->n_clauses; i++) {
> > > + ovsdb_clone_clause(&clauses[i], &to->clauses[i]);
> > > + ovsdb_clause_free(&to->clauses[i]);
> > > + }
> > > +
> > > + for (i = 0; i < add->n_clauses; i++) {
> > > + if (bitmap_is_set(clause_map, i)) {
> > > + ovsdb_clone_clause(&clauses[index++], &add->clauses[i]);
> > > + }
> > > + }
> >
> > >
> > > Should we keep the clauses sorted?
> > >
> > > If yes, It may be more efficient to blindly add the new clauses,
> > > using xreallc()
> > > sort them, then only check for duplicates with adjacent clauses.
> > >
>
> > Will fix this.
> >
> >
> > > +
> > > + free(to->clauses);
> > > + free(clause_map);
> > > + to->clauses = clauses;
> > > + to->n_clauses += count;
> > > +}
> > > +
> > > +void
> > > +ovsdb_condition_remove(struct ovsdb_condition *from,
> > > + const struct ovsdb_condition *remove)
> > > +{
> > > + size_t i, count = 0;
> > > + int j;
> > > + struct ovsdb_clause *clauses;
> > > + unsigned long int *clause_map = xmalloc(bitmap_n_bytes
> > (from->n_clauses));
> > > + int index = 0;
> > > +
> > > + if (remove->n_clauses == 0) {
> > > + free(clause_map);
> > > + return;
> > > + }
> > > +
> > > + for (i = 0; i < from->n_clauses; i++) {
> > > + bitmap_set1(clause_map, i);
> > > + }
> > > +
> > > + for (i = 0; i < remove->n_clauses; i++) {
> > > + j = ovsdb_clause_exists(from, &remove->clauses[i]);
> > > + if (j >= 0) {
> > > + bitmap_set0(clause_map, j);
> > > + count++;
> > > + }
> > > + }
> > > +
> > > + if (count == 0) {
> > > + free(clause_map);
> > > + return;
> > > + }
> > > +
> > > + clauses = xzalloc((from->n_clauses - count) * sizeof *clauses);
> > > + for (i = 0; i < from->n_clauses; i++) {
> > > + if (bitmap_is_set(clause_map, i)) {
> > > + ovsdb_clone_clause(&clauses[index++], &from->clauses[i]);
> > > + }
> > > + }
> > > +
> > > Should clauses still be sorted after removal? Since we make sure
> > all removed
> > > clauses exist in current condition at message parsing time
> > > (ovsdb_monitor_table_condition_change()),
> > > sort then remove should also work.
> > >
>
> > Will fix this.
> >
> >
> > > + free(from->clauses);
> > > + free(clause_map);
> > > + from->clauses = clauses;
> > > + from->n_clauses -= count;
> > > +}
> > > +
> > > +/* Returns if a + b includes c */
> > > +bool
> > > +ovsdb_conditions_includes(const struct ovsdb_condition *a,
> > > + const struct ovsdb_condition *b,
> > > + const struct ovsdb_condition *c)
> > > +{
> > > + size_t i;
> > > +
> > > + for (i = 0; i < c->n_clauses; i++) {
> > > + if(ovsdb_clause_exists(a, &c->clauses[i]) == -1 &&
> > > + ovsdb_clause_exists(b, &c->clauses[i]) == -1) {
> > > + return false;
> > > + }
> > > + }
> > > +
> > > + return true;
> > > }
> > > diff --git a/ovsdb/condition.h b/ovsdb/condition.h
> > > index 620757f..4d223d4 100644
> > > --- a/ovsdb/condition.h
> > > +++ b/ovsdb/condition.h
> > > @@ -27,6 +27,8 @@ struct ovsdb_row;
> > > /* These list is ordered in ascending order of the fraction of
> > > tables row that
> > > * they are (heuristically) expected to leave in query results. */
> > > #define OVSDB_FUNCTIONS \
> > > + OVSDB_FUNCTION(OVSDB_F_FALSE, "false") \
> > > + OVSDB_FUNCTION(OVSDB_F_TRUE, "true") \
> > > OVSDB_FUNCTION(OVSDB_F_EQ, "==") \
> > > OVSDB_FUNCTION(OVSDB_F_INCLUDES, "includes") \
> > > OVSDB_FUNCTION(OVSDB_F_LE, "<=") \
> > > @@ -60,6 +62,8 @@ struct ovsdb_condition {
> > >
> > > #define OVSDB_CONDITION_INITIALIZER { NULL, 0 }
> > >
> > > +void ovsdb_condition_init(struct ovsdb_condition *);
> > > +bool ovsdb_condition_empty(const struct ovsdb_condition *);
> > > struct ovsdb_error *ovsdb_condition_from_json(
> > > const struct ovsdb_table_schema *,
> > > const struct json *, struct ovsdb_symbol_table *,
> > > @@ -67,6 +71,28 @@ struct ovsdb_error *ovsdb_condition_from_json(
> > > struct json *ovsdb_condition_to_json(const struct ovsdb_condition *);
> > > void ovsdb_condition_destroy(struct ovsdb_condition *);
> > > bool ovsdb_condition_evaluate(const struct ovsdb_row *,
> > > - const struct ovsdb_condition *);
> > > + const struct ovsdb_condition *,
> > > + const unsigned int
> columns_index_map[]);
> > > +bool ovsdb_condition_evaluate_or_datum(const struct ovsdb_datum *,
> > > + const struct ovsdb_condition
> *,
> > > + const unsigned int
> > > columns_index_map[]);
> > > +int ovsdb_condition_cmp(const struct ovsdb_condition *a,
> > > + const struct ovsdb_condition *b);
> > > +
> > > +void ovsdb_condition_clone(struct ovsdb_condition *to,
> > > + const struct ovsdb_condition *from);
> > > +
> > > +void
> > > +ovsdb_condition_add(struct ovsdb_condition *to,
> > > + const struct ovsdb_condition *add);
> > > +
> > > +void
> > > +ovsdb_condition_remove(struct ovsdb_condition *from,
> > > + const struct ovsdb_condition *remove);
> > > +
> > > +bool
> > > +ovsdb_conditions_includes(const struct ovsdb_condition *a,
> > > + const struct ovsdb_condition *b,
> > > + const struct ovsdb_condition *c);
> > >
> > > #endif /* ovsdb/condition.h */
> > > diff --git a/ovsdb/query.c b/ovsdb/query.c
> > > index e288020..5d3f9b1 100644
> > > --- a/ovsdb/query.c
> > > +++ b/ovsdb/query.c
> > > @@ -34,7 +34,7 @@ ovsdb_query(struct ovsdb_table *table, const
> > > struct ovsdb_condition *cnd,
> > > const struct ovsdb_row *row;
> > >
> > > row = ovsdb_table_get_row(table, &cnd->clauses
> > [0].arg.keys[0].uuid);
> > > - if (row && row->table == table && ovsdb_condition_evaluate
> > > (row, cnd)) {
> > > + if (row && row->table == table && ovsdb_condition_evaluate
> > > (row, cnd, NULL)) {
> > > output_row(row, aux);
> > > }
> > > } else {
> > > @@ -42,7 +42,7 @@ ovsdb_query(struct ovsdb_table *table, const
> > > struct ovsdb_condition *cnd,
> > > const struct ovsdb_row *row, *next;
> > >
> > > HMAP_FOR_EACH_SAFE (row, next, hmap_node, &table->rows) {
> > > - if (ovsdb_condition_evaluate(row, cnd) && !output_row
> > > (row, aux)) {
> > > + if (ovsdb_condition_evaluate(row, cnd, NULL) && !
> > > output_row(row, aux)) {
> > > break;
> > > }
> > > }
> > > diff --git a/tests/ovsdb-condition.at b/tests/ovsdb-condition.at
> > > index ab54b1c..285c3d6 100644
> > > --- a/tests/ovsdb-condition.at
> > > +++ b/tests/ovsdb-condition.at
> > > @@ -181,8 +181,21 @@ OVSDB_CHECK_POSITIVE([condition sorting],
> > > ["i", "<", 4],
> > > ["i", ">", 6],
> > > ["i", ">=", 5],
> > > - ["_uuid", "==", ["uuid", "d50e85c6-8ae7-4b16-
> > b69e-4395928bd9be"]]]']],
> > > - [[[["_uuid","==",["uuid","d50e85c6-8ae7-4b16-
> > > b69e-4395928bd9be"]],["i","==",1],["i","includes",2],["i","<=",3],
> > >
> ["i","<",4],["i",">=",5],["i",">",6],["i","excludes",7],["i","!=",8]]]])
> > > + ["_uuid", "==", ["uuid",
> "d50e85c6-8ae7-4b16-b69e-4395928bd9be"]],
> > > + true]']],
> > > + [[[true,["_uuid","==",["uuid","d50e85c6-8ae7-4b16-
> > > b69e-4395928bd9be"]],["i","==",1],["i","includes",2],["i","<=",3],
> > >
> ["i","<",4],["i",">=",5],["i",">",6],["i","excludes",7],["i","!=",8]]]])
> > > +
> > > +OVSDB_CHECK_POSITIVE([boolean condition],
> > > + [[parse-conditions \
> > > + '{"columns": {"name": {"type": "string"}}}' \
> > > + '[true]']],
> > > + [[[true]]])
> > > +
> > > +OVSDB_CHECK_POSITIVE([boolean condition],
> > > + [[parse-conditions \
> > > + '{"columns": {"name": {"type": "string"}}}' \
> > > + '[false]']],
> > > + [[[false]]])
> > >
> > > OVSDB_CHECK_POSITIVE([evaluating null condition],
> > > [[evaluate-conditions \
> > > @@ -657,3 +670,21 @@ condition 5: --T-
> > > condition 6: -T--
> > > condition 7: T-TT
> > > condition 8: -T-T], [condition])
> > > +
> > > +OVSDB_CHECK_POSITIVE([evaluating false boolean condition],
> > > + [[evaluate-conditions \
> > > + '{"columns": {"i": {"type": "integer"}}}' \
> > > + '[[false,["i","==",0]]]' \
> > > + '[{"i": 0},
> > > + {"i": 1},
> > > + {"i": 2}']]],
> > > + [condition 0: ---])
> > > +
> > > +OVSDB_CHECK_POSITIVE([evaluating true boolean condition],
> > > + [[evaluate-conditions-or \
> > > + '{"columns": {"i": {"type": "integer"}}}' \
> > > + '[[true,["i","==",0]]]' \
> > > + '[{"i": 0},
> > > + {"i": 1},
> > > + {"i": 2}']]],
> > > + [condition 0: TTT])
> > > diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
> > > index 15f41b0..e924532 100644
> > > --- a/tests/test-ovsdb.c
> > > +++ b/tests/test-ovsdb.c
> > > @@ -851,8 +851,11 @@ do_parse_conditions(struct ovs_cmdl_context *ctx)
> > > exit(exit_code);
> > > }
> > >
> > > +#define OVSDB_CONDITION_AND 0
> > > +#define OVSDB_CONDITION_OR 1
> > > +
> > > static void
> > > -do_evaluate_conditions(struct ovs_cmdl_context *ctx)
> > > +do_evaluate_condition__(struct ovs_cmdl_context *ctx, int mode)
> > > {
> > > struct ovsdb_table_schema *ts;
> > > struct ovsdb_table *table;
> > > @@ -900,7 +903,16 @@ do_evaluate_conditions(struct ovs_cmdl_context
> *ctx)
> > > for (i = 0; i < n_conditions; i++) {
> > > printf("condition %2"PRIuSIZE":", i);
> > > for (j = 0; j < n_rows; j++) {
> > > - bool result = ovsdb_condition_evaluate(rows[j],
> > &conditions[i]);
> > > + bool result;
> > > + if (mode == OVSDB_CONDITION_AND) {
> > > + result = ovsdb_condition_evaluate(rows[j],
> > > + &conditions[i],
> > > + NULL);
> > > + } else {
> > > + result =
> ovsdb_condition_evaluate_or_datum(rows[j]->fields,
> > > +
> &conditions[i],
> > > + NULL);
> > > + }
> > > if (j % 5 == 0) {
> > > putchar(' ');
> > > }
> > > @@ -921,6 +933,18 @@ do_evaluate_conditions(struct ovs_cmdl_context
> *ctx)
> > > }
> > >
> > > static void
> > > +do_evaluate_conditions(struct ovs_cmdl_context *ctx)
> > > +{
> > > + do_evaluate_condition__(ctx, OVSDB_CONDITION_AND);
> > > +}
> > > +
> > > +static void
> > > +do_evaluate_conditions_or(struct ovs_cmdl_context *ctx)
> > > +{
> > > + do_evaluate_condition__(ctx, OVSDB_CONDITION_OR);
> > > +}
> > > +
> > > +static void
> > > do_parse_mutations(struct ovs_cmdl_context *ctx)
> > > {
> > > struct ovsdb_table_schema *ts;
> > > @@ -2191,6 +2215,7 @@ static struct ovs_cmdl_command all_commands[] =
> {
> > > { "compare-rows", NULL, 2, INT_MAX, do_compare_rows },
> > > { "parse-conditions", NULL, 2, INT_MAX, do_parse_conditions },
> > > { "evaluate-conditions", NULL, 3, 3, do_evaluate_conditions },
> > > + { "evaluate-conditions-or", NULL, 3, 3, do_evaluate_conditions_or
> },
> > > { "parse-mutations", NULL, 2, INT_MAX, do_parse_mutations },
> > > { "execute-mutations", NULL, 3, 3, do_execute_mutations },
> > > { "query", NULL, 3, 3, do_query },
> > > --
> > > 2.1.4
> > >
> > >
> > > _______________________________________________
> > > dev mailing list
> > > dev@openvswitch.org
> > > http://openvswitch.org/mailman/listinfo/dev
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
