[ovs-dev] [PATCH 1/2] ovsdb-idl: Make it possible to omit or pay less attention to columns.
Justin Pettit
jpettit at nicira.com
Wed Aug 11 06:49:46 UTC 2010
On Jul 19, 2010, at 4:30 PM, Ben Pfaff wrote:
> +enum ovsdb_idl_mode {
> + /* Client reads and may write this column and wants to be alerted upon
> + * updates to it.
> + *
> + * This is the default. */
> + OVSDB_IDL_MODE_RW,
> +
> + /* Client may read and write this column, but doesn't care to be alerted
> + * when it is updated.
> + *
> + * This is useful for columns that a client treats as "write-only", that
> + * is, it updates them but doesn't want to get alerted about its own
> + * updates. It also won't be alerted about other clients' updates, so this
> + * is suitable only for use by a client that "owns" a particular column. */
> + OVSDB_IDL_MODE_WO,
> +
> + /* Client won't read or write this column at all. */
> + OVSDB_IDL_MODE_NONE
> +};
I don't believe that this code actually enforces that a client doesn't violate the mode; it only affects whether they're alerted about changes through a monitor. I think it would be good to be more explicit about this--especially in the case of NONE.
> +static void
> +ovsdb_idl_set_mode(struct ovsdb_idl *idl,
> + const struct ovsdb_idl_column *column,
> + enum ovsdb_idl_mode mode)
> +{
> + size_t i;
> +
> + 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;
> +
> + if (column >= tc->columns && column < &tc->columns[tc->n_columns]) {
> + unsigned char *modep = &table->modes[column - tc->columns];
> + assert(*modep == OVSDB_IDL_MODE_RW || *modep == mode);
> + *modep = mode;
> + return;
> + }
> + }
It may be nice to document that this function doesn't allow the mode to be changed once it's already been in a mode other than read-write.
> +/* By default, 'idl' replicates all of the columns in the remote database, and
> + * ovsdb_idl_run() returns true upon a change to any column in the database.
> + * Call this function to avoid alerting ovsdb_idl_run()'s caller upon changes
> + * to 'column'.
> + *
> + * This is useful for columns that a client treats as "write-only", that is, it
> + * updates them but doesn't want to get alerted about its own updates. It also
> + * won't be alerted about other clients' updates, so this is suitable only for
> + * use by a client that "owns" a particular column.
> + *
> + * The client must be careful not to retain pointers to data in 'column' across
> + * calls to ovsdb_idl_run(), even when that function returns false, because
> + * the client is not alerted to changes.
> + *
> + * This function should be called after ovsdb_idl_create(), but before the
> + * first call to ovsdb_idl_run(). This function is mutually exclusive with
> + * ovsdb_idl_omit() for any given column. */
> +void
> +ovsdb_idl_set_write_only(struct ovsdb_idl *idl,
> + const struct ovsdb_idl_column *column)
> +{
> + ovsdb_idl_set_mode(idl, column, OVSDB_IDL_MODE_WO);
> +}
It might be nice to note that there's no turning back once this function has been called, based on the assertion above.
> +/* By default, 'idl' replicates all of the columns in the remote database.
> + * Call this function to omit replicating 'column'. This saves CPU time and
> + * bandwidth to the database.
> + *
> + * This function should be called after ovsdb_idl_create(), but before the
> + * first call to ovsdb_idl_run().
> + *
> + * This function should be called after ovsdb_idl_create(), but before the
> + * first call to ovsdb_idl_run(). This function is mutually exclusive with
> + * ovsdb_idl_set_write_only() for any given column. */
> +void
> +ovsdb_idl_omit(struct ovsdb_idl *idl, const struct ovsdb_idl_column *column)
> +{
> + ovsdb_idl_set_mode(idl, column, OVSDB_IDL_MODE_NONE);
> +}
Ditto here.
> -static void
> +static bool
> ovsdb_idl_process_update(struct ovsdb_idl_table *table,
It may be nice to explain what the return type means.
> -static void
> +static bool
> ovsdb_idl_row_update(struct ovsdb_idl_row *row, const struct json *row_json)
Ditto here.
> /* When a row A refers to row B through a column with a "refTable" constraint,
> @@ -787,13 +860,17 @@ ovsdb_idl_delete_row(struct ovsdb_idl_row *row)
> }
> }
>
> -static void
> +static bool
> ovsdb_idl_modify_row(struct ovsdb_idl_row *row, const struct json *row_json)
And here, too, Swayze.
--Justin
More information about the dev
mailing list