Andy Zhou <az...@ovn.org> wrote on 05/02/2016 01:03:17 PM:
> On Wed, Feb 3, 2016 at 5:53 AM, Liran Schour <lir...@il.ibm.com> wrote:
> Add API that allows the user to create condition iteratively
> and send condition_update to the server.
>
> Signed-off-by: Liran Schour <lir...@il.ibm.com>
>
> ---
> v2->v3:
> * Simplify API to allow iteratively adding clauses to condition
> before send monitor_cond_update
> * monitor_cond_update receives only a single json condition
> * Automaticly generate <dbName>_<tableName)_add_clause_<columnName()
> ---
> lib/ovsdb-idl-provider.h | 13 ++++
> lib/ovsdb-idl.c | 178 ++++++++++++++++++++++++++++++++++++
> ++++++++++-
> lib/ovsdb-idl.h | 34 +++++++++
> ovsdb/ovsdb-idlc.in | 168
++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 392 insertions(+), 1 deletion(-)
>
> diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h
> index 190acca..1288ad3 100644
> --- a/lib/ovsdb-idl-provider.h
> +++ b/lib/ovsdb-idl-provider.h
> @@ -51,6 +51,18 @@ struct ovsdb_idl_column {
> void (*unparse)(struct ovsdb_idl_row *);
> };
>
> +struct ovsdb_idl_condition {
> + const struct ovsdb_idl_table_class *tc;
> + struct ovs_list clauses;
> +};
> +
> +struct ovsdb_idl_clause {
> + struct ovs_list node;
> + enum ovsdb_idl_function function;
> + const struct ovsdb_idl_column *column;
> + struct ovsdb_datum arg;
> +};
> +
> struct ovsdb_idl_table_class {
> char *name;
> bool is_root;
> @@ -70,6 +82,7 @@ struct ovsdb_idl_table {
> struct ovsdb_idl *idl; /* Containing idl. */
> unsigned int change_seqno[OVSDB_IDL_CHANGE_MAX];
> struct ovs_list track_list; /* Tracked rows
> (ovsdb_idl_row.track_node). */
> + struct ovsdb_idl_condition condition;
> };
>
> struct ovsdb_idl_class {
> diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
> index a05b420..513b23f 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -261,6 +261,7 @@ ovsdb_idl_create(const char *remote, const
> struct ovsdb_idl_class *class,
> = table->change_seqno[OVSDB_IDL_CHANGE_MODIFY]
> = table->change_seqno[OVSDB_IDL_CHANGE_DELETE] = 0;
> table->idl = idl;
> + ovsdb_idl_condition_init(&table->condition, tc);
> }
>
> idl->state_seqno = UINT_MAX;
> @@ -646,6 +647,176 @@ ovsdb_idl_add_table(struct ovsdb_idl *idl,
> OVS_NOT_REACHED();
> }
>
> +struct ovsdb_error *
> +ovsdb_idl_function_from_string(const char *name,
> + enum ovsdb_idl_function *function)
> +{
> +#define OVSDB_IDL_FUNCTION(ENUM, NAME) \
> + if (!strcmp(name, NAME)) { \
> + *function = ENUM; \
> + return NULL; \
> + }
> + OVSDB_IDL_FUNCTIONS;
> +#undef OVSDB_IDL_FUNCTION
> +
> + return ovsdb_syntax_error(NULL, "unknown function",
> + "No function named %s.", name);
> +}
> +
> +static const char *
> +ovsdb_idl_function_to_string(enum ovsdb_idl_function function)
> +{
> + switch (function) {
> +#define OVSDB_IDL_FUNCTION(ENUM, NAME) case ENUM: return NAME;
> + OVSDB_IDL_FUNCTIONS;
> +#undef OVSDB_IDL_FUNCTION
> + }
> +
> + return NULL;
> +}
> +
> +static void
> +ovsdb_idl_condition_merge(struct ovsdb_idl_condition *a,
> + struct ovsdb_idl_condition *b)
> +{
> + while(!list_is_empty(&b->clauses)) {
> + list_push_back(&a->clauses, list_pop_front(&b->clauses));
> + }
> +}
> +
> +static struct json *
> +ovsdb_idl_clause_to_json(const struct ovsdb_idl_clause *clause)
> +{
> + if (clause->function != OVSDB_IDL_F_TRUE &&
> + clause->function != OVSDB_IDL_F_FALSE) {
> + const char *function = ovsdb_idl_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_IDL_F_TRUE ?
> + true : false);
> +}
> +
> +static void
> +ovsdb_idl_clause_free(struct ovsdb_idl_clause *clause)
> +{
> + if (clause->function != OVSDB_IDL_F_TRUE &&
> + clause->function != OVSDB_IDL_F_FALSE) {
> + ovsdb_datum_destroy(&clause->arg, &clause->column->type);
> + }
> +
> + list_remove(&clause->node);
> + free(clause);
> +}
> +
> +void
> +ovsdb_idl_condition_destroy(struct ovsdb_idl_condition *cnd)
> +{
> + struct ovsdb_idl_clause *c, *next;
> +
> + LIST_FOR_EACH_SAFE (c, next, node, &cnd->clauses) {
> + ovsdb_idl_clause_free(c);
> + }
> +}
> +
> +void
> +ovsdb_idl_condition_init(struct ovsdb_idl_condition *cnd,
> + const struct ovsdb_idl_table_class *tc)
> +{
> + cnd->tc = tc;
> + list_init(&cnd->clauses);
> +}
> +
> +void ovsdb_idl_condition_add_clause(struct ovsdb_idl_condition *cnd,
> + enum ovsdb_idl_function function,
> + const struct ovsdb_idl_column
*column,
> + struct ovsdb_datum *arg)
> +{
> + struct ovsdb_idl_clause *clause = xzalloc(sizeof *clause);
> + const struct ovsdb_type *type = NULL;
> +
> + list_init(&clause->node);
> + clause->function = function;
> + clause->column = column;
> + if (column) {
> + type = &column->type;
> + } else {
> + type = &ovsdb_type_boolean;
> + }
> + ovsdb_datum_clone(&clause->arg, arg, type);
> + list_push_back(&cnd->clauses, &clause->node);
> +}
> +
> +static struct json *
> +ovsdb_idl_condition_to_json(const struct ovsdb_idl_condition *cnd)
> +{
> + struct json **clauses;
> + size_t i = 0, n_clauses = 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);
> + }
> +
> + return json_array_create(clauses, n_clauses);
> +}
> +
> +static void
> +ovsdb_idl_send_cond_update__(struct ovsdb_idl *idl,
> + struct ovsdb_idl_table *table,
> + const struct ovsdb_idl_condition *cond)
> +{
> + char uuid[UUID_LEN + 1];
> + struct json *monitor_cond_update_requests = json_object_create();
> + struct json *monitor_cond_update_request = json_object_create();
> + struct json *cond_json = ovsdb_idl_condition_to_json(cond);
> + struct json *params, *json_uuid;
> + struct jsonrpc_msg *request;
> +
> + json_object_put(monitor_cond_update_request, "where", cond_json);
> + json_object_put(monitor_cond_update_requests,
> + table->class->name,
> + json_array_create_1(monitor_cond_update_request));
> +
> + snprintf(uuid, sizeof uuid, UUID_FMT,
> + UUID_ARGS(&idl->uuid));
> + json_uuid = json_string_create(uuid);
> +
> + /* Create a new uuid */
> + uuid_generate(&idl->uuid);
> + snprintf(uuid, sizeof uuid, UUID_FMT,
> + UUID_ARGS(&idl->uuid));
> + params = json_array_create_3(json_uuid, json_string_create(uuid),
> + monitor_cond_update_requests);
> +
> + request = jsonrpc_create_request("monitor_cond_update", params,
NULL);
> + jsonrpc_session_send(idl->session, request);
> +}
> +
> +/* Add conditions to the replicated tables. Ensure that tables are
> added to the
> + * replication.
> + */
> +bool
> +ovsdb_idl_cond_update(struct ovsdb_idl *idl,
> + struct ovsdb_idl_condition *cond)
> +{
> + struct ovsdb_idl_table *table;
> +
> + table = ovsdb_idl_table_from_class(idl, cond->tc);
> +
> + if (jsonrpc_session_is_connected(idl->session)) {
> + ovsdb_idl_send_cond_update__(idl, table, cond);
> + } else {
> + ovsdb_idl_condition_merge(&table->condition, cond);
> + }
> + return true;
> +}
> This interface seems to allow only condition updates to one table at
> a time. Did I mss something
> here? Also in patch 10. I did not find a test that covers updates
> with multiple tables.
The user can update one table on each ovsdb_idl_cond_update() call and can
do it iteratively on many tables. I think this API is good enough. (I
tested it to change the SB monitoring in ovn-controller to be
conditional).
Will add test that covers cond_update with multiple tables.
> +
> /* Turns off OVSDB_IDL_ALERT for 'column' in 'idl'.
> *
> * This function should be called between ovsdb_idl_create() and
> the first call
> @@ -945,7 +1116,7 @@ ovsdb_idl_send_monitor_request__(struct ovsdb_idl
*idl,
> for (i = 0; i < idl->class->n_tables; i++) {
> const struct ovsdb_idl_table *table = &idl->tables[i];
> const struct ovsdb_idl_table_class *tc = table->class;
> - struct json *monitor_request, *columns;
> + struct json *monitor_request, *columns, *where;
> const struct sset *table_schema;
> size_t j;
>
> @@ -983,6 +1154,11 @@ 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") &&
> + list_size(&table->condition.clauses) > 0) {
> + where = ovsdb_idl_condition_to_json(&table->condition);
> + json_object_put(monitor_request, "where", where);
> + }
> json_object_put(monitor_requests, tc->name,
monitor_request);
> }
> }
> diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
> index 1cbaf35..c93fdf3 100644
> --- a/lib/ovsdb-idl.h
> +++ b/lib/ovsdb-idl.h
> @@ -45,6 +45,7 @@ struct ovsdb_idl_class;
> struct ovsdb_idl_row;
> struct ovsdb_idl_column;
> struct ovsdb_idl_table_class;
> +struct ovsdb_idl_condition;
> struct uuid;
>
> struct ovsdb_idl *ovsdb_idl_create(const char *remote,
> @@ -277,4 +278,37 @@ void ovsdb_idl_loop_destroy(struct ovsdb_idl_loop
*);
> struct ovsdb_idl_txn *ovsdb_idl_loop_run(struct ovsdb_idl_loop *);
> void ovsdb_idl_loop_commit_and_wait(struct ovsdb_idl_loop *);
>
> +/* 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_IDL_FUNCTIONS \
> + OVSDB_IDL_FUNCTION(OVSDB_IDL_F_FALSE, "false") \
> + OVSDB_IDL_FUNCTION(OVSDB_IDL_F_TRUE, "true") \
> + OVSDB_IDL_FUNCTION(OVSDB_IDL_F_EQ, "==") \
> + OVSDB_IDL_FUNCTION(OVSDB_IDL_F_INCLUDES, "includes") \
> + OVSDB_IDL_FUNCTION(OVSDB_IDL_F_LE, "<=") \
> + OVSDB_IDL_FUNCTION(OVSDB_IDL_F_LT, "<") \
> + OVSDB_IDL_FUNCTION(OVSDB_IDL_F_GE, ">=") \
> + OVSDB_IDL_FUNCTION(OVSDB_IDL_F_GT, ">") \
> + OVSDB_IDL_FUNCTION(OVSDB_IDL_F_EXCLUDES, "excludes") \
> + OVSDB_IDL_FUNCTION(OVSDB_IDL_F_NE, "!=")
> +
> +enum ovsdb_idl_function {
> +#define OVSDB_IDL_FUNCTION(ENUM, NAME) ENUM,
> + OVSDB_IDL_FUNCTIONS
> +#undef OVSDB_IDL_FUNCTION
> +};
> +
> +void ovsdb_idl_condition_init(struct ovsdb_idl_condition *cnd,
> + const struct ovsdb_idl_table_class *tc);
> +void ovsdb_idl_condition_destroy(struct ovsdb_idl_condition *cnd);
> +void ovsdb_idl_condition_add_clause(struct ovsdb_idl_condition *cnd,
> + enum ovsdb_idl_function function,
> + const struct ovsdb_idl_column
*column,
> + struct ovsdb_datum *arg);
> +bool ovsdb_idl_cond_update(struct ovsdb_idl *idl,
> + struct ovsdb_idl_condition *cond);
> +
> +struct ovsdb_error *
> +ovsdb_idl_function_from_string(const char *name,
> + enum ovsdb_idl_function *function);
> #endif /* ovsdb-idl.h */
> diff --git a/ovsdb/ovsdb-idlc.in b/ovsdb/ovsdb-idlc.in
> index 26b0de4..596c2a7 100755
> --- a/ovsdb/ovsdb-idlc.in
> +++ b/ovsdb/ovsdb-idlc.in
> @@ -216,6 +216,20 @@ bool %(s)s_is_updated(const struct %(s)s *,
> enum %(s)s_column_id);
> print '%s);' % ', '.join(args)
>
> print
> + for columnName, column in sorted(table.columns.iteritems()):
> + print 'void %(s)s_add_clause_%(c)s(struct
> ovsdb_idl_condition *, enum ovsdb_idl_function function,' % {'s':
> structName, 'c': columnName},
> + if column.type.is_smap():
> + args = ['const struct smap *']
> + else:
> + comment, members = cMembers(prefix, tableName,
columnName,
> + column, True)
> + args = ['%(type)s%(name)s' % member for member in
members]
> + print '%s);' % ', '.join(args)
> +
> + print 'void %s_add_clause_true(struct ovsdb_idl_condition
> *);' % structName
> + print 'void %s_add_clause_false(struct ovsdb_idl_condition
> *);' % structName
> +
> + print
>
> # Table indexes.
> printEnum("%stable_id" % prefix.lower(), ["%sTABLE_%s" %
> (prefix.upper(), tableName.upper()) for tableName in sorted
> (schema.tables)] + ["%sN_TABLES" % prefix.upper()])
> @@ -747,6 +761,160 @@ const struct ovsdb_datum *
> 'C': columnName.upper()}
> print "}"
>
> + # Add clause functions.
> + for columnName, column in sorted(table.columns.iteritems()):
> + type = column.type
> +
> + comment, members = cMembers(prefix, tableName, columnName,
> + column, True)
> +
> + if type.is_smap():
> + print comment
> + print """void
> +%(s)s_add_clause_%(c)s(struct ovsdb_idl_condition *cnd, enum
> ovsdb_idl_function function, const struct smap *%(c)s)
> +{
> + struct ovsdb_datum datum;
> +
> + ovs_assert(inited);
> + if (%(c)s) {
> + struct smap_node *node;
> + size_t i;
> +
> + datum.n = smap_count(%(c)s);
> + datum.keys = xmalloc(datum.n * sizeof *datum.keys);
> + datum.values = xmalloc(datum.n * sizeof *datum.values);
> +
> + i = 0;
> + SMAP_FOR_EACH (node, %(c)s) {
> + datum.keys[i].string = xstrdup(node->key);
> + datum.values[i].string = xstrdup(node->value);
> + i++;
> + }
> + ovsdb_datum_sort_unique(&datum, OVSDB_TYPE_STRING,
> OVSDB_TYPE_STRING);
> + } else {
> + ovsdb_datum_init_empty(&datum);
> + }
> +
> + ovsdb_idl_condition_add_clause(cnd,
> + function,
> + &%(s)s_columns[%(S)s_COL_%(C)s],
> + &datum);
> +}
> +""" % {'t': tableName,
> + 's': structName,
> + 'S': structName.upper(),
> + 'c': columnName,
> + 'C': columnName.upper()}
> + 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_add_clause_%(c)s(struct
> ovsdb_idl_condition *cnd, enum ovsdb_idl_function function, %(args)s)' %
\
> + {'s': structName, 'c': columnName,
> + 'args': ', '.join(['%(type)s%(name)s' % m for m
inmembers])}
> + print "{"
> + print " struct ovsdb_datum datum;"
> + 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 " ovs_assert(inited);"
> + 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)
> + if type.value:
> + print " datum.values = &value;"
> + print " "+
> type.value.assign_c_value_casting_away_const("value.%s" %
> type.value.type.to_string(), valueVar)
> + else:
> + print " datum.values = NULL;"
> + elif type.is_optional_pointer():
> + print " union ovsdb_atom key;"
> + print
> + print " ovs_assert(inited);"
> + 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)
> + 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 " ovs_assert(inited);"
> + 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)
> + print " } else {"
> + print " datum.n = 0;"
> + print " datum.keys = NULL;"
> + print " }"
> + print " datum.values = NULL;"
> + else:
> + print " size_t i;"
> + print
> + print " ovs_assert(inited);"
> + print " datum.n = %s;" % nVar
> + print " datum.keys = %s ? xmalloc(%s * sizeof
> *datum.keys) : NULL;" % (nVar, nVar)
> + if type.value:
> + print " datum.values = xmalloc(%s * sizeof
> *datum.values);" % nVar
> + else:
> + print " datum.values = NULL;"
> + print " for (i = 0; i < %s; i++) {" % nVar
> + print " " + type.key.copyCValue("datum.keys
> [i].%s" % type.key.type.to_string(), "%s[i]" % keyVar)
> + if type.value:
> + print " " + type.value.copyCValue
> ("datum.values[i].%s" % type.value.type.to_string(), "%s[i]" % valueVar)
> + 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_add_clause(cnd,
> + function,
> + &%(s)s_columns[%(S)s_COL_%(C)s],
> + &datum);
> +}""" % {'t': tableName,
> + 's': structName,
> + 'S': structName.upper(),
> + 'c': columnName,
> + 'C': columnName.upper()}
> +
> + print """void
> +%s_add_clause_false(struct ovsdb_idl_condition *cnd)
> +{
> + struct ovsdb_datum datum;
> +
> + ovsdb_datum_init_empty(&datum);
> + ovsdb_idl_condition_add_clause(cnd, OVSDB_IDL_F_FALSE, NULL,
&datum);
> +}""" % structName
> +
> + print """void
> +%s_add_clause_true(struct ovsdb_idl_condition *cnd)
> +{
> + struct ovsdb_datum datum;
> +
> + ovsdb_datum_init_empty(&datum);
> + ovsdb_idl_condition_add_clause(cnd, OVSDB_IDL_F_TRUE, NULL,
&datum);
> +}""" % structName
> +
> # Table columns.
> print "\nstruct ovsdb_idl_column %s_columns[%s_N_COLUMNS];" % (
> structName, structName.upper())
> --
> 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
