diff --git a/lib/ovsdb-data.h b/lib/ovsdb-data.h index ae2672e25..9fce380cf 100644 --- a/lib/ovsdb-data.h +++ b/lib/ovsdb-data.h @@ -127,6 +127,7 @@ struct ovsdb_datum { union ovsdb_atom *keys; /* Each of the ovsdb_type's 'key_type'. */ union ovsdb_atom *values; /* Each of the ovsdb_type's 'value_type'. */ }; +#define OVSDB_DATUM_INITIALIZER { 0, NULL, NULL } /* Basics. */ void ovsdb_datum_init_empty(struct ovsdb_datum *); diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h index 1f5a49eaa..8cfbb95aa 100644 --- a/lib/ovsdb-idl-provider.h +++ b/lib/ovsdb-idl-provider.h @@ -102,10 +102,6 @@ struct ovsdb_idl_table_class { void (*row_init)(struct ovsdb_idl_row *); }; -struct ovsdb_idl_condition { - struct ovs_list clauses; -}; - struct ovsdb_idl_table { const struct ovsdb_idl_table_class *class; unsigned char *modes; /* OVSDB_IDL_* bitmasks, indexed by column. */ diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 3974eb669..1be1c6805 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -24,6 +24,7 @@ #include "bitmap.h" #include "coverage.h" +#include "hash.h" #include "openvswitch/dynamic-string.h" #include "fatal-signal.h" #include "openvswitch/json.h" @@ -219,7 +220,6 @@ ovsdb_idl_table_from_class(const struct ovsdb_idl *, const struct ovsdb_idl_table_class *); static bool ovsdb_idl_track_is_set(struct ovsdb_idl_table *table); static void ovsdb_idl_send_cond_change(struct ovsdb_idl *idl); -static void ovsdb_idl_condition_init(struct ovsdb_idl_condition *cnd); /* Creates and returns a connection to database 'remote', which should be in a * form acceptable to jsonrpc_session_open(). The connection will maintain an @@ -279,6 +279,7 @@ ovsdb_idl_create(const char *remote, const struct ovsdb_idl_class *class, = table->change_seqno[OVSDB_IDL_CHANGE_DELETE] = 0; table->idl = idl; ovsdb_idl_condition_init(&table->condition); + ovsdb_idl_condition_add_clause_true(&table->condition); table->cond_changed = false; } @@ -837,147 +838,210 @@ ovsdb_idl_add_table(struct ovsdb_idl *idl, OVS_NOT_REACHED(); } +/* A single clause within an ovsdb_idl_condition. */ struct ovsdb_idl_clause { - struct ovs_list node; - enum ovsdb_function function; - const struct ovsdb_idl_column *column; - struct ovsdb_datum arg; + struct hmap_node hmap_node; /* In struct ovsdb_idl_condition. */ + enum ovsdb_function function; /* Never OVSDB_F_TRUE or OVSDB_F_FALSE. */ + const struct ovsdb_idl_column *column; /* Must be nonnull. */ + struct ovsdb_datum arg; /* Has ovsdb_type ->column->type. */ }; +static uint32_t +ovsdb_idl_clause_hash(const struct ovsdb_idl_clause *clause) +{ + uint32_t hash = hash_pointer(clause->column, clause->function); + return ovsdb_datum_hash(&clause->arg, &clause->column->type, hash); +} + +static int +ovsdb_idl_clause_equals(const struct ovsdb_idl_clause *a, + const struct ovsdb_idl_clause *b) +{ + return (a->function == b->function + && a->column == b->column + && ovsdb_datum_equals(&a->arg, &b->arg, &a->column->type)); +} + static struct json * ovsdb_idl_clause_to_json(const struct ovsdb_idl_clause *clause) { - if (clause->function != OVSDB_F_TRUE && - clause->function != OVSDB_F_FALSE) { - const char *function = ovsdb_function_to_string(clause->function); - - return json_array_create_3(json_string_create(clause->column->name), - json_string_create(function), - ovsdb_datum_to_json(&clause->arg, - &clause->column->type)); - } - - return json_boolean_create(clause->function == OVSDB_F_TRUE ? - true : false); + const char *function = ovsdb_function_to_string(clause->function); + return json_array_create_3(json_string_create(clause->column->name), + json_string_create(function), + ovsdb_datum_to_json(&clause->arg, + &clause->column->type)); } static void -ovsdb_idl_clause_free(struct ovsdb_idl_clause *clause) +ovsdb_idl_clause_destroy(struct ovsdb_idl_clause *clause) { - if (clause->function != OVSDB_F_TRUE && - clause->function != OVSDB_F_FALSE) { + if (clause) { ovsdb_datum_destroy(&clause->arg, &clause->column->type); + free(clause); } - - ovs_list_remove(&clause->node); - free(clause); } + +/* ovsdb_idl_condition. */ -/* Clears all of the conditional clauses from table 'tc', so that all of the - * rows in the table will be replicated. (This is the default, so this - * function has an effect only if some clauses were added to 'tc' using - * ovsdb_idl_condition_add_clause().) */ void -ovsdb_idl_condition_reset(struct ovsdb_idl *idl, - const struct ovsdb_idl_table_class *tc) -{ - struct ovsdb_idl_clause *c, *next; - struct ovsdb_idl_table *table = ovsdb_idl_table_from_class(idl, tc); - - LIST_FOR_EACH_SAFE (c, next, node, &table->condition.clauses) { - ovsdb_idl_clause_free(c); - } - idl->cond_changed = table->cond_changed = true; -} - -static void ovsdb_idl_condition_init(struct ovsdb_idl_condition *cnd) { - ovs_list_init(&cnd->clauses); + hmap_init(&cnd->clauses); + cnd->is_true = false; +} + +void +ovsdb_idl_condition_destroy(struct ovsdb_idl_condition *cond) +{ + if (cond) { + ovsdb_idl_condition_clear(cond); + hmap_destroy(&cond->clauses); + } +} + +void +ovsdb_idl_condition_clear(struct ovsdb_idl_condition *cond) +{ + struct ovsdb_idl_clause *clause, *next; + HMAP_FOR_EACH_SAFE (clause, next, hmap_node, &cond->clauses) { + hmap_remove(&cond->clauses, &clause->hmap_node); + ovsdb_idl_clause_destroy(clause); + } + cond->is_true = false; +} + +bool +ovsdb_idl_condition_is_true(const struct ovsdb_idl_condition *condition) +{ + return condition->is_true; } static struct ovsdb_idl_clause * -ovsdb_idl_condition_find_clause(struct ovsdb_idl_table *table, - enum ovsdb_function function, - const struct ovsdb_idl_column *column, - const struct ovsdb_datum *arg) +ovsdb_idl_condition_find_clause(const struct ovsdb_idl_condition *condition, + const struct ovsdb_idl_clause *target, + uint32_t hash) { - struct ovsdb_idl_clause *c; - LIST_FOR_EACH (c, node, &table->condition.clauses) { - if (c->function == function && - (!column || (c->column == column && - ovsdb_datum_equals(&c->arg, - arg, &column->type)))) { - return c; + struct ovsdb_idl_clause *clause; + HMAP_FOR_EACH_WITH_HASH (clause, hmap_node, hash, &condition->clauses) { + if (ovsdb_idl_clause_equals(clause, target)) { + return clause; } } return NULL; } +static void +ovsdb_idl_condition_add_clause__(struct ovsdb_idl_condition *condition, + const struct ovsdb_idl_clause *src, + uint32_t hash) +{ + struct ovsdb_idl_clause *clause = xmalloc(sizeof *clause); + clause->function = src->function; + clause->column = src->column; + ovsdb_datum_clone(&clause->arg, &src->arg, &src->column->type); + hmap_insert(&condition->clauses, &clause->hmap_node, hash); +} + /* Adds a clause to the condition for replicating the table with class 'tc' in * 'idl'. * - * By default, a table has no clauses, and in that case the IDL replicates all - * its rows. When a table has one or more clauses, the IDL replicates only - * rows that satisfy at least one clause. + * The IDL replicates only rows in a table that satisfy at least one clause in + * the table's condition. The default condition for a table has a single + * clause with function OVSDB_F_TRUE, so that the IDL replicates all rows in + * the table. When the IDL client replaces the default condition by one of its + * own, the condition can have any number of clauses. If it has no conditions, + * then no rows are replicated. * - * Two distinct of clauses can be added: + * Two distinct of clauses can usefully be added: * - * - A 'function' of OVSDB_F_FALSE or OVSDB_F_TRUE adds a Boolean clause. A - * "false" clause by itself prevents any rows from being replicated; in - * combination with other clauses it has no effect. A "true" clause - * causes every row to be replicated, regardless of whether other clauses - * exist (thus, a condition that includes "true" is like a condition - * without any clauses at all). + * - A 'function' of OVSDB_F_TRUE. A "true" clause causes every row to be + * replicated, regardless of whether other clauses exist. 'column' and + * 'arg' are ignored. * - * 'column' should be NULL and 'arg' should be an empty datum (initialized - * with ovsdb_datum_init_empty()). - * - * - Other 'functions' add a clause of the form " ", - * e.g. "column == 5" or "column <= 10". In this case, 'arg' must have a - * type that is compatible with 'column'. + * - Binary 'functions' add a clause of the form " + * ", e.g. "column == 5" or "column <= 10". In this case, 'arg' must + * have a type that is compatible with 'column'. */ void -ovsdb_idl_condition_add_clause(struct ovsdb_idl *idl, - const struct ovsdb_idl_table_class *tc, +ovsdb_idl_condition_add_clause(struct ovsdb_idl_condition *condition, enum ovsdb_function function, const struct ovsdb_idl_column *column, const struct ovsdb_datum *arg) { - struct ovsdb_idl_table *table = ovsdb_idl_table_from_class(idl, tc); - - /* Return without doing anything, if this would be a duplicate clause. */ - if (ovsdb_idl_condition_find_clause(table, function, column, arg)) { - return; + if (condition->is_true) { + /* Adding a clause to an always-true condition has no effect. */ + } else if (function == OVSDB_F_TRUE) { + ovsdb_idl_condition_add_clause_true(condition); + } else if (function == OVSDB_F_FALSE) { + /* Adding a "false" clause never has any effect. */ + } else { + struct ovsdb_idl_clause clause = { + .function = function, + .column = column, + .arg = *arg, + }; + uint32_t hash = ovsdb_idl_clause_hash(&clause); + if (!ovsdb_idl_condition_find_clause(condition, &clause, hash)) { + ovsdb_idl_condition_add_clause__(condition, &clause, hash); + } } - - struct ovsdb_idl_clause *clause = xzalloc(sizeof *clause); - ovs_list_init(&clause->node); - clause->function = function; - clause->column = column; - ovsdb_datum_clone(&clause->arg, arg, - column ? &column->type : &ovsdb_type_boolean); - ovs_list_push_back(&table->condition.clauses, &clause->node); - idl->cond_changed = table->cond_changed = true; - poll_immediate_wake(); } -/* If a clause matching (function, column, arg) is included in the condition - * for 'tc' within 'idl', removes it. (If this was the last clause included in - * the table's condition, then this means that the IDL will begin replicating - * every row in the table.) */ void -ovsdb_idl_condition_remove_clause(struct ovsdb_idl *idl, - const struct ovsdb_idl_table_class *tc, - enum ovsdb_function function, - const struct ovsdb_idl_column *column, - const struct ovsdb_datum *arg) +ovsdb_idl_condition_add_clause_true(struct ovsdb_idl_condition *condition) +{ + if (!condition->is_true) { + ovsdb_idl_condition_clear(condition); + condition->is_true = true; + } +} + +static bool +ovsdb_idl_condition_equals(const struct ovsdb_idl_condition *a, + const struct ovsdb_idl_condition *b) +{ + if (hmap_count(&a->clauses) != hmap_count(&b->clauses)) { + return false; + } + if (a->is_true != b->is_true) { + return false; + } + + const struct ovsdb_idl_clause *clause; + HMAP_FOR_EACH (clause, hmap_node, &a->clauses) { + if (!ovsdb_idl_condition_find_clause(b, clause, + clause->hmap_node.hash)) { + return false; + } + } + return true; +} + +static void +ovsdb_idl_condition_clone(struct ovsdb_idl_condition *dst, + const struct ovsdb_idl_condition *src) +{ + ovsdb_idl_condition_init(dst); + + dst->is_true = src->is_true; + + const struct ovsdb_idl_clause *clause; + HMAP_FOR_EACH (clause, hmap_node, &src->clauses) { + ovsdb_idl_condition_add_clause__(dst, clause, clause->hmap_node.hash); + } +} + +/* Sets the replication condition for 'tc' in 'idl' to 'condition' and arranges + * to send the new condition to the database server. */ +void +ovsdb_idl_set_condition(struct ovsdb_idl *idl, + const struct ovsdb_idl_table_class *tc, + const struct ovsdb_idl_condition *condition) { struct ovsdb_idl_table *table = ovsdb_idl_table_from_class(idl, tc); - struct ovsdb_idl_clause *c - = ovsdb_idl_condition_find_clause(table, function, column, arg); - if (c) { - ovsdb_idl_clause_free(c); + if (!ovsdb_idl_condition_equals(condition, &table->condition)) { + ovsdb_idl_condition_destroy(&table->condition); + ovsdb_idl_condition_clone(&table->condition, condition); idl->cond_changed = table->cond_changed = true; poll_immediate_wake(); } @@ -986,18 +1050,25 @@ ovsdb_idl_condition_remove_clause(struct ovsdb_idl *idl, static struct json * ovsdb_idl_condition_to_json(const struct ovsdb_idl_condition *cnd) { - struct json **clauses; - size_t i = 0, n_clauses = ovs_list_size(&cnd->clauses); - struct ovsdb_idl_clause *c; - - clauses = xmalloc(n_clauses * sizeof *clauses); - LIST_FOR_EACH (c, node, &cnd->clauses) { - clauses[i++] = ovsdb_idl_clause_to_json(c); + if (cnd->is_true) { + return json_array_create_empty(); } - return json_array_create(clauses, n_clauses); -} + size_t n = hmap_count(&cnd->clauses); + if (!n) { + return json_array_create_1(json_boolean_create(false)); + } + struct json **clauses = xmalloc(n * sizeof *clauses); + const struct ovsdb_idl_clause *clause; + size_t i = 0; + HMAP_FOR_EACH (clause, hmap_node, &cnd->clauses) { + clauses[i++] = ovsdb_idl_clause_to_json(clause); + } + ovs_assert(i == n); + return json_array_create(clauses, n); +} + static struct json * ovsdb_idl_create_cond_change_req(struct ovsdb_idl_table *table) { @@ -1398,8 +1469,8 @@ ovsdb_idl_send_monitor_request__(struct ovsdb_idl *idl, monitor_request = json_object_create(); json_object_put(monitor_request, "columns", columns); - if (!strcmp(method, "monitor_cond") && table->cond_changed && - ovs_list_size(&table->condition.clauses) > 0) { + if (!strcmp(method, "monitor_cond") + && !ovsdb_idl_condition_is_true(&table->condition)) { where = ovsdb_idl_condition_to_json(&table->condition); json_object_put(monitor_request, "where", where); table->cond_changed = false; diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h index 8786a418f..f0326b449 100644 --- a/lib/ovsdb-idl.h +++ b/lib/ovsdb-idl.h @@ -326,19 +326,31 @@ int ovsdb_idl_loop_commit_and_wait(struct ovsdb_idl_loop *); * replicates every row in the table. These functions allow the client to * specify that only selected rows should be replicated, by constructing a * per-table condition that specifies the rows to replicate. + * + * A condition is a disjunction of clauses. The condition is true, and thus a + * row is replicated, if any of the clauses evaluates to true for a given row. + * (Thus, a condition with no clauses is always false.) */ -void ovsdb_idl_condition_reset(struct ovsdb_idl *idl, - const struct ovsdb_idl_table_class *tc); -void ovsdb_idl_condition_add_clause(struct ovsdb_idl *idl, - const struct ovsdb_idl_table_class *tc, +struct ovsdb_idl_condition { + struct hmap clauses; /* Contains "struct ovsdb_idl_clause"s. */ + bool is_true; /* Is the condition unconditionally true? */ +}; +#define OVSDB_IDL_CONDITION_INIT(CONDITION) \ + { HMAP_INITIALIZER(&(CONDITION)->clauses), false } + +void ovsdb_idl_condition_init(struct ovsdb_idl_condition *); +void ovsdb_idl_condition_clear(struct ovsdb_idl_condition *); +void ovsdb_idl_condition_destroy(struct ovsdb_idl_condition *); +void ovsdb_idl_condition_add_clause(struct ovsdb_idl_condition *, enum ovsdb_function function, const struct ovsdb_idl_column *column, const struct ovsdb_datum *arg); -void ovsdb_idl_condition_remove_clause(struct ovsdb_idl *idl, - const struct ovsdb_idl_table_class *tc, - enum ovsdb_function function, - const struct ovsdb_idl_column *column, - const struct ovsdb_datum *arg); +void ovsdb_idl_condition_add_clause_true(struct ovsdb_idl_condition *); +bool ovsdb_idl_condition_is_true(const struct ovsdb_idl_condition *); + +void ovsdb_idl_set_condition(struct ovsdb_idl *, + const struct ovsdb_idl_table_class *, + const struct ovsdb_idl_condition *); #endif /* ovsdb-idl.h */ diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in index cf6cd5845..f1c7a3598 100755 --- a/ovsdb/ovsdb-idlc.in +++ b/ovsdb/ovsdb-idlc.in @@ -243,7 +243,7 @@ bool %(s)s_is_updated(const struct %(s)s *, enum %(s)s_column_id); print 'void %(s)s_update_%(c)s_delvalue(const struct %(s)s *, ' % {'s': structName, 'c': columnName}, print '%(valtype)s);' % {'valtype':column.type.key.to_const_c_type(prefix)} - print 'void %(s)s_add_clause_%(c)s(struct ovsdb_idl *idl, enum ovsdb_function function,' % {'s': structName, 'c': columnName}, + print 'void %(s)s_add_clause_%(c)s(struct ovsdb_idl_condition *, enum ovsdb_function function,' % {'s': structName, 'c': columnName}, if column.type.is_smap(): args = ['const struct smap *'] else: @@ -252,22 +252,7 @@ bool %(s)s_is_updated(const struct %(s)s *, enum %(s)s_column_id); args = ['%(type)s%(name)s' % member for member in members] print '%s);' % ', '.join(args) - print 'void %s_add_clause_true(struct ovsdb_idl *idl);' % structName - print 'void %s_add_clause_false(struct ovsdb_idl *idl);' % structName - - print - for columnName, column in sorted_columns(table): - print 'void %(s)s_remove_clause_%(c)s(struct ovsdb_idl *idl, enum ovsdb_function function,' % {'s': structName, 'c': columnName}, - if column.type.is_smap(): - args = ['const struct smap *'] - else: - comment, members = cMembers(prefix, tableName, columnName, - column, True, refTable=False) - args = ['%(type)s%(name)s' % member for member in members] - print '%s);' % ', '.join(args) - - print 'void %s_remove_clause_true(struct ovsdb_idl *idl);' % structName - print 'void %s_remove_clause_false(struct ovsdb_idl *idl);' % structName + print 'void %(s)s_set_condition(struct ovsdb_idl *, struct ovsdb_idl_condition *);' % {'s': structName}, print @@ -870,7 +855,7 @@ void if type.is_smap(): print comment print """void -%(s)s_add_clause_%(c)s(struct ovsdb_idl *idl, enum ovsdb_function function, const struct smap *%(c)s) +%(s)s_add_clause_%(c)s(struct ovsdb_idl_condition *cond, enum ovsdb_function function, const struct smap *%(c)s) { struct ovsdb_datum datum; @@ -880,8 +865,7 @@ void ovsdb_datum_init_empty(&datum); } - ovsdb_idl_condition_add_clause(idl, - &%(p)stable_%(tl)s, + ovsdb_idl_condition_add_clause(cond, function, &%(s)s_col_%(c)s, &datum); @@ -911,7 +895,7 @@ void print comment print 'void' - print '%(s)s_add_clause_%(c)s(struct ovsdb_idl *idl, enum ovsdb_function function, %(args)s)' % \ + print '%(s)s_add_clause_%(c)s(struct ovsdb_idl_condition *cond, enum ovsdb_function function, %(args)s)' % \ {'s': structName, 'c': columnName, 'args': ', '.join(['%(type)s%(name)s' % m for m in members])} print "{" @@ -975,7 +959,7 @@ void print " ovsdb_datum_sort_unique(&datum, %s, %s);" % ( type.key.toAtomicType(), valueType) - print""" ovsdb_idl_condition_add_clause(idl, &%(p)stable_%(tl)s, + print""" ovsdb_idl_condition_add_clause(cond, function, &%(s)s_col_%(c)s, &datum);\ @@ -990,177 +974,14 @@ void print " free(%s);" % var print "}" - print """\ + print """ void -%(s)s_add_clause_false(struct ovsdb_idl *idl) +%(s)s_set_condition(struct ovsdb_idl *idl, struct ovsdb_idl_condition *condition) { - struct ovsdb_datum datum; - - ovsdb_datum_init_empty(&datum); - ovsdb_idl_condition_add_clause(idl, &%(p)stable_%(tl)s, OVSDB_F_FALSE, NULL, &datum); -}""" % {'s': structName, - 'tl': tableName.lower(), - 'p': prefix, - 'P': prefix.upper()} - - print """void -%(s)s_add_clause_true(struct ovsdb_idl *idl) -{ - struct ovsdb_datum datum; - - ovsdb_datum_init_empty(&datum); - ovsdb_idl_condition_add_clause(idl, &%(p)stable_%(tl)s, OVSDB_F_TRUE, NULL, &datum); -}""" % {'s': structName, - 'tl': tableName.lower(), - 'p': prefix, - 'P': prefix.upper()} - - # Remove clause functions. - for columnName, column in sorted_columns(table): - type = column.type - - comment, members = cMembers(prefix, tableName, columnName, - column, True, refTable=False) - - if type.is_smap(): - print comment - print """void -%(s)s_remove_clause_%(c)s(struct ovsdb_idl *idl, enum ovsdb_function function, const struct smap *%(c)s) -{ - struct ovsdb_datum datum; - - if (%(c)s) { - ovsdb_datum_from_smap(&datum, %(c)s); - } else { - ovsdb_datum_init_empty(&datum); - } - - ovsdb_idl_condition_remove_clause(idl, &%(p)stable_%(tl)s, - function, - &%(s)s_col_%(c)s, - &datum); - - ovsdb_datum_destroy(&datum, &%(s)s_col_%(c)s.type); -} -""" % {'tl': tableName.lower(), - 'p': prefix, - 'P': prefix.upper(), - 's': structName, - 'S': structName.upper(), - 'c': columnName} - continue - - keyVar = members[0]['name'] - nVar = None - valueVar = None - if type.value: - valueVar = members[1]['name'] - if len(members) > 2: - nVar = members[2]['name'] - else: - if len(members) > 1: - nVar = members[1]['name'] - - print comment - print 'void' - print '%(s)s_remove_clause_%(c)s(struct ovsdb_idl *idl, enum ovsdb_function function, %(args)s)' % \ - {'s': structName, 'c': columnName, - 'args': ', '.join(['%(type)s%(name)s' % m for m in members])} - print "{" - print " struct ovsdb_datum datum;" - free = [] - if type.n_min == 1 and type.n_max == 1: - print " union ovsdb_atom key;" - if type.value: - print " union ovsdb_atom value;" - print - print " datum.n = 1;" - print " datum.keys = &key;" - print " " + type.key.assign_c_value_casting_away_const("key.%s" % type.key.type.to_string(), keyVar, refTable=False) - if type.value: - print " datum.values = &value;" - print " "+ type.value.assign_c_value_casting_away_const("value.%s" % type.value.type.to_string(), valueVar, refTable=False) - else: - print " datum.values = NULL;" - elif type.is_optional_pointer(): - print " union ovsdb_atom key;" - print - print " if (%s) {" % keyVar - print " datum.n = 1;" - print " datum.keys = &key;" - print " " + type.key.assign_c_value_casting_away_const("key.%s" % type.key.type.to_string(), keyVar, refTable=False) - print " } else {" - print " datum.n = 0;" - print " datum.keys = NULL;" - print " }" - print " datum.values = NULL;" - elif type.n_max == 1: - print " union ovsdb_atom key;" - print - print " if (%s) {" % nVar - print " datum.n = 1;" - print " datum.keys = &key;" - print " " + type.key.assign_c_value_casting_away_const("key.%s" % type.key.type.to_string(), "*" + keyVar, refTable=False) - print " } else {" - print " datum.n = 0;" - print " datum.keys = NULL;" - print " }" - print " datum.values = NULL;" - else: - print " datum.n = %s;" % nVar - print " datum.keys = %s ? xmalloc(%s * sizeof *datum.keys) : NULL;" % (nVar, nVar) - free += ['datum.keys'] - if type.value: - free += ['datum.values'] - print " datum.values = xmalloc(%s * sizeof *datum.values);" % nVar - else: - print " datum.values = NULL;" - print " for (size_t i = 0; i < %s; i++) {" % nVar - print " " + type.key.assign_c_value_casting_away_const("datum.keys[i].%s" % type.key.type.to_string(), "%s[i]" % keyVar, refTable=False) - if type.value: - print " " + type.value.assign_c_value_casting_away_const("datum.values[i].%s" % type.value.type.to_string(), "%s[i]" % valueVar, refTable=False) - print " }" - if type.value: - valueType = type.value.toAtomicType() - else: - valueType = "OVSDB_TYPE_VOID" - print " ovsdb_datum_sort_unique(&datum, %s, %s);" % ( - type.key.toAtomicType(), valueType) - - print""" ovsdb_idl_condition_remove_clause(idl, &%(p)stable_%(tl)s, - function, - &%(s)s_col_%(c)s, - &datum);\ -""" % {'tl': tableName.lower(), - 'p': prefix, - 'P': prefix.upper(), - 's': structName, - 'S': structName.upper(), - 'c': columnName} - for var in free: - print " free(%s);" % var - print "}" - - print """void -%(s)s_remove_clause_false(struct ovsdb_idl *idl) -{ - struct ovsdb_datum datum; - - ovsdb_datum_init_empty(&datum); - ovsdb_idl_condition_remove_clause(idl, &%(p)stable_%(tl)s, OVSDB_F_FALSE, NULL, &datum); -} - -void -%(s)s_remove_clause_true(struct ovsdb_idl *idl) -{ - struct ovsdb_datum datum; - - ovsdb_datum_init_empty(&datum); - ovsdb_idl_condition_remove_clause(idl, &%(p)stable_%(tl)s, OVSDB_F_TRUE, NULL, &datum); -}""" % {'s': structName, - 'tl': tableName.lower(), - 'p': prefix, - 'P': prefix.upper(),} + ovsdb_idl_set_condition(idl, &%(p)stable_%(tl)s, condition); +}""" % {'p': prefix, + 's': structName, + 'tl': tableName.lower()} # Table columns. for columnName, column in sorted_columns(table): diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py index 0178050d9..e43457c0b 100644 --- a/python/ovs/db/idl.py +++ b/python/ovs/db/idl.py @@ -1,4 +1,4 @@ -# Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc. +# Copyright (c) 2009, 2010, 2011, 2012, 2013, 2016 Nicira, Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -144,7 +144,7 @@ class Idl(object): table.need_table = False table.rows = {} table.idl = self - table.condition = [] + table.condition = [True] table.cond_changed = False def close(self): @@ -265,23 +265,23 @@ class Idl(object): self.__send_cond_change(table, table.condition) table.cond_changed = False - def cond_change(self, table_name, add_cmd, cond): - """Change conditions for this IDL session. If session is not already - connected, add condtion to table and submit it on send_monitor_request. - Otherwise send monitor_cond_change method with the requested - changes.""" + def cond_change(self, table_name, cond): + """Sets the condition for 'table_name' to 'cond', which should be a + conditional expression suitable for use directly in the OVSDB + protocol, with the exception that the empty condition [] + matches no rows (instead of matching every row). That is, [] + is equivalent to [False], not to [True]. + """ table = self.tables.get(table_name) if not table: raise error.Error('Unknown table "%s"' % table_name) - if add_cmd: - table.condition += cond - else: - for c in cond: - table.condition.remove(c) - - table.cond_changed = True + if cond == []: + cond = [False] + if table.condition != cond: + table.condition = cond + table.cond_changed = True def wait(self, poller): """Arranges for poller.block() to wake up when self.run() has something @@ -420,8 +420,7 @@ class Idl(object): (column not in self.readonly[table.name])): columns.append(column) monitor_requests[table.name] = {"columns": columns} - if method == "monitor_cond" and table.cond_changed and \ - table.condition: + if method == "monitor_cond" and table.condition != [True]: monitor_requests[table.name]["where"] = table.condition table.cond_change = False diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at index 9ed431bf9..d2c1ea6f3 100644 --- a/tests/ovsdb-idl.at +++ b/tests/ovsdb-idl.at @@ -393,8 +393,8 @@ OVSDB_CHECK_IDL([simple idl, conditional, false condition], "row": {"i": 1, "r": 2.0, "b": true}}]']], - [['condition add simple [false]' \ - 'condition remove simple [false]']], + [['condition simple []' \ + 'condition simple [true]']], [[000: change conditions 001: empty 002: change conditions @@ -409,8 +409,8 @@ OVSDB_CHECK_IDL([simple idl, conditional, true condition], "row": {"i": 1, "r": 2.0, "b": true}}]']], - [['condition add simple [false]' \ - 'condition add simple [true]']], + [['condition simple []' \ + 'condition simple [true]']], [[000: change conditions 001: empty 002: change conditions @@ -430,8 +430,8 @@ OVSDB_CHECK_IDL([simple idl, conditional, multiple clauses in condition], "row": {"i": 2, "r": 3.0, "b": true}}]']], - [['condition add simple [false]' \ - 'condition add simple [["i","==",1],["i","==",2]]']], + [['condition simple []' \ + 'condition simple [["i","==",1],["i","==",2]]']], [[000: change conditions 001: empty 002: change conditions @@ -447,8 +447,8 @@ OVSDB_CHECK_IDL([simple idl, conditional, modify as insert due to condition], "row": {"i": 1, "r": 2.0, "b": true}}]']], - [['condition add simple [false]' \ - 'condition add simple [["i","==",1]]']], + [['condition simple []' \ + 'condition simple [["i","==",1]]']], [[000: change conditions 001: empty 002: change conditions @@ -463,9 +463,9 @@ OVSDB_CHECK_IDL([simple idl, conditional, modify as delete due to condition], "row": {"i": 1, "r": 2.0, "b": true}}]']], - [['condition add simple [false]' \ - 'condition add simple [["i","==",1],["i","==",2]]' \ - 'condition remove simple [["i","==",1]]' \ + [['condition simple []' \ + 'condition simple [["i","==",1],["i","==",2]]' \ + 'condition simple [["i","==",2]]' \ '["idltest", {"op": "insert", "table": "simple", @@ -498,10 +498,10 @@ OVSDB_CHECK_IDL([simple idl, conditional, multiple tables], "table": "link2", "row": {"i": 2}, "uuid-name": "row0"}]']], - [['condition add simple [false];condition add link1 [false];condition add link2 [false]' \ - 'condition add simple [["i","==",1]]' \ - 'condition add link1 [["i","==",0]]' \ - 'condition add link2 [["i","==",3]]' \ + [['condition simple [];link1 [];link2 []' \ + 'condition simple [["i","==",1]]' \ + 'condition link1 [["i","==",0]]' \ + 'condition link2 [["i","==",3]]' \ '+["idltest", {"op": "insert", "table": "link2", diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 6a7d46753..968c1de22 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -2184,145 +2184,50 @@ find_table_class(const char *name) } static void -parse_simple_json_clause(struct ovsdb_idl *idl, bool add_cmd, - struct json *json) +parse_simple_json_clause(struct ovsdb_idl_condition *cond, + enum ovsdb_function function, + const char *column, const struct json *arg) { - const char *c; - struct ovsdb_error *error; - enum ovsdb_function function; - - if (json->type == JSON_TRUE) { - add_cmd ? idltest_simple_add_clause_true(idl) : - idltest_simple_remove_clause_true(idl); - return; - } else if (json->type == JSON_FALSE) { - add_cmd ? idltest_simple_add_clause_false(idl) : - idltest_simple_remove_clause_false(idl); - return; - } - if (json->type != JSON_ARRAY || json->u.array.n != 3) { - ovs_fatal(0, "Error parsing condition"); - } - - c = json_string(json->u.array.elems[0]); - error = ovsdb_function_from_string(json_string(json->u.array.elems[1]), - &function); - if (error) { - ovs_fatal(0, "Error parsing clause function %s", - json_string(json->u.array.elems[1])); - } - - /* add clause according to column */ - if (!strcmp(c, "b")) { - add_cmd ? idltest_simple_add_clause_b(idl, function, - json_boolean(json->u.array.elems[2])) : - idltest_simple_remove_clause_b(idl, function, - json_boolean(json->u.array.elems[2])); - } else if (!strcmp(c, "i")) { - add_cmd ? idltest_simple_add_clause_i(idl, function, - json_integer(json->u.array.elems[2])) : - idltest_simple_remove_clause_i(idl, function, - json_integer(json->u.array.elems[2])); - } else if (!strcmp(c, "s")) { - add_cmd ? idltest_simple_add_clause_s(idl, function, - json_string(json->u.array.elems[2])) : - idltest_simple_remove_clause_s(idl, function, - json_string(json->u.array.elems[2])); - } else if (!strcmp(c, "u")) { + if (!strcmp(column, "b")) { + idltest_simple_add_clause_b(cond, function, json_boolean(arg)); + } else if (!strcmp(column, "i")) { + idltest_simple_add_clause_i(cond, function, json_integer(arg)); + } else if (!strcmp(column, "s")) { + idltest_simple_add_clause_s(cond, function, json_string(arg)); + } else if (!strcmp(column, "u")) { struct uuid uuid; - if (!uuid_from_string(&uuid, - json_string(json->u.array.elems[2]))) { - ovs_fatal(0, "\"%s\" is not a valid UUID", - json_string(json->u.array.elems[2])); + if (!uuid_from_string(&uuid, json_string(arg))) { + ovs_fatal(0, "\"%s\" is not a valid UUID", json_string(arg)); } - add_cmd ? idltest_simple_add_clause_u(idl, function, uuid) : - idltest_simple_remove_clause_u(idl, function, uuid); - } else if (!strcmp(c, "r")) { - add_cmd ? idltest_simple_add_clause_r(idl, function, - json_real(json->u.array.elems[2])) : - idltest_simple_remove_clause_r(idl, function, - json_real(json->u.array.elems[2])); + idltest_simple_add_clause_u(cond, function, uuid); + } else if (!strcmp(column, "r")) { + idltest_simple_add_clause_r(cond, function, json_real(arg)); } else { - ovs_fatal(0, "Unsupported columns name %s", c); + ovs_fatal(0, "Unsupported columns name %s", column); } } static void -parse_link1_json_clause(struct ovsdb_idl *idl, bool add_cmd, - struct json *json) +parse_link1_json_clause(struct ovsdb_idl_condition *cond, + enum ovsdb_function function, + const char *column, const struct json *arg) { - const char *c; - struct ovsdb_error *error; - enum ovsdb_function function; - - if (json->type == JSON_TRUE) { - add_cmd ? idltest_link1_add_clause_true(idl) : - idltest_link1_remove_clause_true(idl); - return; - } else if (json->type == JSON_FALSE) { - add_cmd ? idltest_link1_add_clause_false(idl) : - idltest_link1_remove_clause_false(idl); - return; - } - if (json->type != JSON_ARRAY || json->u.array.n != 3) { - ovs_fatal(0, "Error parsing condition"); - } - - c = json_string(json->u.array.elems[0]); - error = ovsdb_function_from_string(json_string(json->u.array.elems[1]), - &function); - if (error) { - ovs_fatal(0, "Error parsing clause function %s", - json_string(json->u.array.elems[1])); - } - - /* add clause according to column */ - if (!strcmp(c, "i")) { - add_cmd ? idltest_link1_add_clause_i(idl, function, - json_integer(json->u.array.elems[2])) : - idltest_link1_remove_clause_i(idl, function, - json_integer(json->u.array.elems[2])); + if (!strcmp(column, "i")) { + idltest_link1_add_clause_i(cond, function, json_integer(arg)); } else { - ovs_fatal(0, "Unsupported columns name %s", c); + ovs_fatal(0, "Unsupported columns name %s", column); } } static void -parse_link2_json_clause(struct ovsdb_idl *idl, bool add_cmd, struct json *json) +parse_link2_json_clause(struct ovsdb_idl_condition *cond, + enum ovsdb_function function, + const char *column, const struct json *arg) { - const char *c; - struct ovsdb_error *error; - enum ovsdb_function function; - - if (json->type == JSON_TRUE) { - add_cmd ? idltest_link2_add_clause_true(idl) : - idltest_link2_remove_clause_true(idl); - return; - } else if (json->type == JSON_FALSE) { - add_cmd ? idltest_link2_add_clause_false(idl) : - idltest_link2_remove_clause_false(idl); - return; - } - if (json->type != JSON_ARRAY || json->u.array.n != 3) { - ovs_fatal(0, "Error parsing condition"); - } - - c = json_string(json->u.array.elems[0]); - error = ovsdb_function_from_string(json_string(json->u.array.elems[1]), - &function); - if (error) { - ovs_fatal(0, "Error parsing clause function %s", - json_string(json->u.array.elems[1])); - } - - /* add clause according to column */ - if (!strcmp(c, "i")) { - add_cmd ? idltest_link2_add_clause_i(idl, function, - json_integer(json->u.array.elems[2])) : - idltest_link2_remove_clause_i(idl, function, - json_integer(json->u.array.elems[2])); + if (!strcmp(column, "i")) { + idltest_link2_add_clause_i(cond, function, json_integer(arg)); } else { - ovs_fatal(0, "Unsupported columns name %s", c); + ovs_fatal(0, "Unsupported columns name %s", column); } } @@ -2331,19 +2236,9 @@ update_conditions(struct ovsdb_idl *idl, char *commands) { char *cmd, *save_ptr1 = NULL; const struct ovsdb_idl_table_class *tc; - bool add_cmd = false; for (cmd = strtok_r(commands, ";", &save_ptr1); cmd; cmd = strtok_r(NULL, ";", &save_ptr1)) { - if (strstr(cmd, "condition add")) { - cmd += strlen("condition add "); - add_cmd = true; - } else if (strstr(cmd, "condition remove")) { - cmd += strlen("condition remove "); - } else { - ovs_fatal(0, "condition command should be add or remove"); - } - char *save_ptr2 = NULL; char *table_name = strtok_r(cmd, " ", &save_ptr2); struct json *json = parse_json(save_ptr2); @@ -2358,17 +2253,37 @@ update_conditions(struct ovsdb_idl *idl, char *commands) ovs_fatal(0, "Table %s does not exist", table_name); } - //ovsdb_idl_condition_reset(idl, tc); - + struct ovsdb_idl_condition cond = OVSDB_IDL_CONDITION_INIT(&cond); for (i = 0; i < json->u.array.n; i++) { - if (!strcmp(table_name, "simple")) { - parse_simple_json_clause(idl, add_cmd, json->u.array.elems[i]); - } else if (!strcmp(table_name, "link1")) { - parse_link1_json_clause(idl, add_cmd, json->u.array.elems[i]); - } else if (!strcmp(table_name, "link2")) { - parse_link2_json_clause(idl, add_cmd, json->u.array.elems[i]); + const struct json *clause = json->u.array.elems[i]; + if (clause->type == JSON_TRUE) { + ovsdb_idl_condition_add_clause_true(&cond); + } else if (clause->type != JSON_ARRAY || clause->u.array.n != 3 + || clause->u.array.elems[0]->type != JSON_STRING + || clause->u.array.elems[1]->type != JSON_STRING) { + ovs_fatal(0, "Error parsing condition"); + } else { + enum ovsdb_function function; + const char *function_s = json_string(clause->u.array.elems[1]); + struct ovsdb_error *error = ovsdb_function_from_string( + function_s, &function); + if (error) { + ovs_fatal(0, "unknown clause function %s", function_s); + } + + const char *column = json_string(clause->u.array.elems[0]); + const struct json *arg = clause->u.array.elems[2]; + if (!strcmp(table_name, "simple")) { + parse_simple_json_clause(&cond, function, column, arg); + } else if (!strcmp(table_name, "link1")) { + parse_link1_json_clause(&cond, function, column, arg); + } else if (!strcmp(table_name, "link2")) { + parse_link2_json_clause(&cond, function, column, arg); + } } } + ovsdb_idl_set_condition(idl, tc, &cond); + ovsdb_idl_condition_destroy(&cond); json_destroy(json); } } @@ -2409,8 +2324,9 @@ do_idl(struct ovs_cmdl_context *ctx) setvbuf(stdout, NULL, _IONBF, 0); symtab = ovsdb_symbol_table_create(); - if (ctx->argc > 2 && strstr(ctx->argv[2], "condition ")) { - update_conditions(idl, ctx->argv[2]); + const char cond_s[] = "condition "; + if (ctx->argc > 2 && strstr(ctx->argv[2], cond_s)) { + update_conditions(idl, ctx->argv[2] + strlen(cond_s)); printf("%03d: change conditions\n", step++); i = 3; } else { @@ -2451,8 +2367,8 @@ do_idl(struct ovs_cmdl_context *ctx) if (!strcmp(arg, "reconnect")) { printf("%03d: reconnect\n", step++); ovsdb_idl_force_reconnect(idl); - } else if (strstr(arg, "condition ")) { - update_conditions(idl, arg); + } else if (!strncmp(arg, cond_s, strlen(cond_s))) { + update_conditions(idl, arg + strlen(cond_s)); printf("%03d: change conditions\n", step++); } else if (arg[0] != '[') { idl_set(idl, arg, step++); diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py index 185215ae6..dced56b99 100644 --- a/tests/test-ovsdb.py +++ b/tests/test-ovsdb.py @@ -1,4 +1,4 @@ -# Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc. +# Copyright (c) 2009, 2010, 2011, 2012, 2016 Nicira, Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -531,25 +531,17 @@ def idl_set(idl, commands, step): def update_condition(idl, commands): - commands = commands.split(";") + commands = commands[len("condition "):].split(";") for command in commands: - command = command[len("condition "):] - if "add" in command: - add_cmd = True - command = command[len("add "):] - else: - add_cmd = False - command = command[len("remove "):] - command = command.split(" ") if(len(command) != 2): - sys.stderr.write("Error parsong condition %s\n" % command) + sys.stderr.write("Error parsing condition %s\n" % command) sys.exit(1) table = command[0] cond = ovs.json.from_string(command[1]) - idl.cond_change(table, add_cmd, cond) + idl.cond_change(table, cond) def do_idl(schema_file, remote, *commands):