[ovs-dev] [PATCH] ovsdb-idl: Style and comment improvements for conditional replication.
Flaviof
flavio at flaviof.com
Mon Aug 15 20:05:39 UTC 2016
On Sun, Aug 14, 2016 at 12:52 AM, Ben Pfaff <blp at ovn.org> wrote:
> The conditional replication code had hardly any comments. This adds some.
>
> This commit also fixes a number of style problems, factors out some code
> into a helper function, and moves some struct declarations from a public
> header, that were not used by client code, into more private locations.
>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
>
Acked-by: Flavio Fernandes <flavio at flaviof.com>
> ---
> lib/ovsdb-idl-provider.h | 7 ++-
> lib/ovsdb-idl.c | 112 ++++++++++++++++++++++++++++++
> ++---------------
> lib/ovsdb-idl.h | 26 +++++------
> 3 files changed, 94 insertions(+), 51 deletions(-)
>
> diff --git a/lib/ovsdb-idl-provider.h b/lib/ovsdb-idl-provider.h
> index 55ed793..e0221d0 100644
> --- a/lib/ovsdb-idl-provider.h
> +++ b/lib/ovsdb-idl-provider.h
> @@ -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.
> @@ -63,6 +63,11 @@ struct ovsdb_idl_table_class {
> void (*row_init)(struct ovsdb_idl_row *);
> };
>
> +struct ovsdb_idl_condition {
> + const struct ovsdb_idl_table_class *tc;
> + 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 7da25a5..df4193b 100644
> --- a/lib/ovsdb-idl.c
> +++ b/lib/ovsdb-idl.c
> @@ -710,6 +710,13 @@ ovsdb_idl_add_table(struct ovsdb_idl *idl,
>
> OVS_NOT_REACHED();
> }
> +
> +struct ovsdb_idl_clause {
> + struct ovs_list node;
> + enum ovsdb_function function;
> + const struct ovsdb_idl_column *column;
> + struct ovsdb_datum arg;
> +};
>
> static struct json *
> ovsdb_idl_clause_to_json(const struct ovsdb_idl_clause *clause)
> @@ -740,6 +747,10 @@ ovsdb_idl_clause_free(struct ovsdb_idl_clause *clause)
> free(clause);
> }
>
> +/* 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)
> @@ -761,59 +772,90 @@ ovsdb_idl_condition_init(struct ovsdb_idl_condition
> *cnd,
> ovs_list_init(&cnd->clauses);
> }
>
> -void ovsdb_idl_condition_add_clause(struct ovsdb_idl *idl,
> - const struct ovsdb_idl_table_class
> *tc,
> - enum ovsdb_function function,
> - const struct ovsdb_idl_column *column,
> - struct ovsdb_datum *arg)
> +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)
> {
> - struct ovsdb_idl_table *table = ovsdb_idl_table_from_class(idl, tc);
> - const struct ovsdb_type *type = NULL;
> struct ovsdb_idl_clause *c;
> -
> - LIST_FOR_EACH(c, node, &table->condition.clauses) {
> + 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;
> + return c;
> }
> }
> + return NULL;
> +}
> +
> +/* 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.
> + *
> + * Two distinct of clauses can 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).
> + *
> + * '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 "<column> <function>
> <arg>",
> + * 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,
> + 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;
> + }
>
> struct ovsdb_idl_clause *clause = xzalloc(sizeof *clause);
> ovs_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);
> + 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();
> }
>
> -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,
> - struct ovsdb_datum *arg)
> +/* 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)
> {
> - 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) {
> - if (c->function == function &&
> - (!column || (c->column == column &&
> - ovsdb_datum_equals(&c->arg,
> - arg, &column->type)))) {
> - ovsdb_idl_clause_free(c);
> - idl->cond_changed = table->cond_changed = true;
> - poll_immediate_wake();
> - return;
> - }
> + struct ovsdb_idl_clause *c
> + = ovsdb_idl_condition_find_clause(table, function, column, arg);
> + if (c) {
> + ovsdb_idl_clause_free(c);
> + idl->cond_changed = table->cond_changed = true;
> + poll_immediate_wake();
> }
> }
>
> @@ -826,13 +868,13 @@ ovsdb_idl_condition_to_json(const struct
> ovsdb_idl_condition *cnd)
>
> clauses = xmalloc(n_clauses * sizeof *clauses);
> LIST_FOR_EACH (c, node, &cnd->clauses) {
> - clauses[i++] = ovsdb_idl_clause_to_json(c);
> + clauses[i++] = ovsdb_idl_clause_to_json(c);
> }
>
> return json_array_create(clauses, n_clauses);
> }
>
> -static struct json*
> +static struct json *
> ovsdb_idl_create_cond_change_req(struct ovsdb_idl_table *table)
> {
> const struct ovsdb_idl_condition *cond = &table->condition;
> diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
> index 45befb0..a0b60e2 100644
> --- a/lib/ovsdb-idl.h
> +++ b/lib/ovsdb-idl.h
> @@ -48,7 +48,6 @@ 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,
> @@ -311,18 +310,15 @@ struct ovsdb_idl_loop {
> 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 *);
> -
> -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_function function;
> - const struct ovsdb_idl_column *column;
> - struct ovsdb_datum arg;
> -};
> +
> +/* Conditional Replication
> + * =======================
> + *
> + * By default, when the IDL replicates a particular table in the
> database, it
> + * 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.
> + */
>
> void ovsdb_idl_condition_reset(struct ovsdb_idl *idl,
> const struct ovsdb_idl_table_class *tc);
> @@ -330,11 +326,11 @@ void ovsdb_idl_condition_add_clause(struct
> ovsdb_idl *idl,
> const struct ovsdb_idl_table_class
> *tc,
> enum ovsdb_function function,
> const struct ovsdb_idl_column *column,
> - struct ovsdb_datum *arg);
> + 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,
> - struct ovsdb_datum *arg);
> + const struct ovsdb_datum *arg);
>
> #endif /* ovsdb-idl.h */
> --
> 2.1.3
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
More information about the dev
mailing list