From 8c3c2f30007534ced8dc6030ecc60d52c3333be4 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Wed, 16 Jun 2010 14:35:48 -0700 Subject: [PATCH] ovsdb-idl: Transition to better interfaces for reading table columns. The existing ovsdb_idl_txn_read() was somewhat difficult and expensive to use, because it always made a copy of the data in the column. This was necessary at the time it was introduced, because there was no way for it to return a "default" value for columns that had not yet been populated without allocating data and hence requiring the caller to free it. Now that ovsdb_datum_default() exists, this is no longer required. This commit introduces a pair of new functions, ovsdb_idl_read() and ovsdb_idl_get(), that return a pointer to existing data and do not do any copying. It also transitions all of ovsdb_idl_txn_read()'s callers to the new interfaces. --- lib/ovsdb-idl.c | 67 ++++++++++++++++++++++++++++---------- lib/ovsdb-idl.h | 11 +++++-- ovsdb/OVSDB.py | 3 ++ utilities/ovs-vsctl.c | 76 ++++++++++++++++++------------------------- 4 files changed, 92 insertions(+), 65 deletions(-) diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 2537db7e..f32fe0e6 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -908,6 +908,55 @@ ovsdb_idl_next_row(const struct ovsdb_idl_row *row) return next_real_row(table, hmap_next(&table->rows, &row->hmap_node)); } + +/* Reads and returns the value of 'column' within 'row'. If an ongoing + * transaction has changed 'column''s value, the modified value is returned. + * + * The caller must not modify or free the returned value. + * + * Various kinds of changes can invalidate the returned value: writing to the + * same 'column' in 'row' (e.g. with ovsdb_idl_txn_write()), deleting 'row' + * (e.g. with ovsdb_idl_txn_delete()), or completing an ongoing transaction + * (e.g. with ovsdb_idl_txn_commit() or ovsdb_idl_txn_abort()). If the + * returned value is needed for a long time, it is best to make a copy of it + * with ovsdb_datum_clone(). */ +const struct ovsdb_datum * +ovsdb_idl_read(const struct ovsdb_idl_row *row, + const struct ovsdb_idl_column *column) +{ + const struct ovsdb_idl_table_class *class = row->table->class; + size_t column_idx = column - class->columns; + + assert(row->new != NULL); + assert(column_idx < class->n_columns); + + if (row->written && bitmap_is_set(row->written, column_idx)) { + return &row->new[column_idx]; + } else if (row->old) { + return &row->old[column_idx]; + } else { + return ovsdb_datum_default(&column->type); + } +} + +/* Same as ovsdb_idl_read(), except that it also asserts that 'column' has key + * type 'key_type' and value type 'value_type'. (Scalar and set types will + * have a value type of OVSDB_TYPE_VOID.) + * + * This is useful in code that "knows" that a particular column has a given + * type, so that it will abort if someone changes the column's type without + * updating the code that uses it. */ +const struct ovsdb_datum * +ovsdb_idl_get(const struct ovsdb_idl_row *row, + const struct ovsdb_idl_column *column, + enum ovsdb_atomic_type key_type OVS_UNUSED, + enum ovsdb_atomic_type value_type OVS_UNUSED) +{ + assert(column->type.key.type == key_type); + assert(column->type.value.type == value_type); + + return ovsdb_idl_read(row, column); +} /* Transactions. */ @@ -1409,24 +1458,6 @@ ovsdb_idl_txn_complete(struct ovsdb_idl_txn *txn, hmap_remove(&txn->idl->outstanding_txns, &txn->hmap_node); } -void -ovsdb_idl_txn_read(const struct ovsdb_idl_row *row, - const struct ovsdb_idl_column *column, - struct ovsdb_datum *datum) -{ - const struct ovsdb_idl_table_class *class = row->table->class; - size_t column_idx = column - class->columns; - - assert(row->new != NULL); - if (row->written && bitmap_is_set(row->written, column_idx)) { - ovsdb_datum_clone(datum, &row->new[column_idx], &column->type); - } else if (row->old) { - ovsdb_datum_clone(datum, &row->old[column_idx], &column->type); - } else { - ovsdb_datum_init_default(datum, &column->type); - } -} - void ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_, const struct ovsdb_idl_column *column, diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h index 7832e4ee..c09ba9b0 100644 --- a/lib/ovsdb-idl.h +++ b/lib/ovsdb-idl.h @@ -33,6 +33,7 @@ #include #include #include "compiler.h" +#include "ovsdb-types.h" struct json; struct ovsdb_datum; @@ -59,6 +60,13 @@ const struct ovsdb_idl_row *ovsdb_idl_first_row( const struct ovsdb_idl *, const struct ovsdb_idl_table_class *); const struct ovsdb_idl_row *ovsdb_idl_next_row(const struct ovsdb_idl_row *); +const struct ovsdb_datum *ovsdb_idl_read(const struct ovsdb_idl_row *, + const struct ovsdb_idl_column *); +const struct ovsdb_datum *ovsdb_idl_get(const struct ovsdb_idl_row *, + const struct ovsdb_idl_column *, + enum ovsdb_atomic_type key_type, + enum ovsdb_atomic_type value_type); + enum ovsdb_idl_txn_status { TXN_UNCHANGED, /* Transaction didn't include any changes. */ TXN_INCOMPLETE, /* Commit in progress, please wait. */ @@ -90,9 +98,6 @@ int64_t ovsdb_idl_txn_get_increment_new_value(const struct ovsdb_idl_txn *); const struct uuid *ovsdb_idl_txn_get_insert_uuid(const struct ovsdb_idl_txn *, const struct uuid *); -void ovsdb_idl_txn_read(const struct ovsdb_idl_row *, - const struct ovsdb_idl_column *, - struct ovsdb_datum *); void ovsdb_idl_txn_write(const struct ovsdb_idl_row *, const struct ovsdb_idl_column *, struct ovsdb_datum *); diff --git a/ovsdb/OVSDB.py b/ovsdb/OVSDB.py index 0cd416e2..3acc7b87 100644 --- a/ovsdb/OVSDB.py +++ b/ovsdb/OVSDB.py @@ -298,6 +298,9 @@ class BaseType: 'boolean': 'bool ', 'string': 'char *'}[self.type] + def toAtomicType(self): + return "OVSDB_TYPE_%s" % self.type.upper() + def copyCValue(self, dst, src): args = {'dst': dst, 'src': src} if self.refTable: diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c index 0291fe81..b5ab2530 100644 --- a/utilities/ovs-vsctl.c +++ b/utilities/ovs-vsctl.c @@ -1989,23 +1989,21 @@ get_row_by_id(struct vsctl_context *ctx, const struct vsctl_table_class *table, const struct ovsdb_idl_row *row; unsigned int best_score = 0; - /* It might make sense to relax this assertion. */ - assert(id->name_column->type.key.type == OVSDB_TYPE_STRING); - referrer = NULL; for (row = ovsdb_idl_first_row(ctx->idl, id->table); row != NULL && best_score != UINT_MAX; row = ovsdb_idl_next_row(row)) { - struct ovsdb_datum name; + const struct ovsdb_datum *name; - ovsdb_idl_txn_read(row, id->name_column, &name); - if (name.n == 1) { + name = ovsdb_idl_get(row, id->name_column, + OVSDB_TYPE_STRING, OVSDB_TYPE_VOID); + if (name->n == 1) { unsigned int score; score = (partial_match_ok - ? score_partial_match(name.keys[0].string, record_id) - : !strcmp(name.keys[0].string, record_id)); + ? score_partial_match(name->keys[0].string, record_id) + : !strcmp(name->keys[0].string, record_id)); if (score > best_score) { referrer = row; best_score = score; @@ -2013,7 +2011,6 @@ get_row_by_id(struct vsctl_context *ctx, const struct vsctl_table_class *table, referrer = NULL; } } - ovsdb_datum_destroy(&name, &id->name_column->type); } if (best_score && !referrer) { vsctl_fatal("multiple rows in %s match \"%s\"", @@ -2026,17 +2023,14 @@ get_row_by_id(struct vsctl_context *ctx, const struct vsctl_table_class *table, final = NULL; if (id->uuid_column) { - struct ovsdb_datum uuid; - - assert(id->uuid_column->type.key.type == OVSDB_TYPE_UUID); - assert(id->uuid_column->type.value.type == OVSDB_TYPE_VOID); + const struct ovsdb_datum *uuid; - ovsdb_idl_txn_read(referrer, id->uuid_column, &uuid); - if (uuid.n == 1) { + uuid = ovsdb_idl_get(referrer, id->uuid_column, + OVSDB_TYPE_UUID, OVSDB_TYPE_VOID); + if (uuid->n == 1) { final = ovsdb_idl_get_row_for_uuid(ctx->idl, table->class, - &uuid.keys[0].uuid); + &uuid->keys[0].uuid); } - ovsdb_datum_destroy(&uuid, &id->uuid_column->type); } else { final = referrer; } @@ -2297,7 +2291,7 @@ cmd_get(struct vsctl_context *ctx) row = must_get_row(ctx, table, record_id); for (i = 3; i < ctx->argc; i++) { const struct ovsdb_idl_column *column; - struct ovsdb_datum datum; + const struct ovsdb_datum *datum; char *key_string; /* Special case for obtaining the UUID of a row. We can't just do this @@ -2313,7 +2307,7 @@ cmd_get(struct vsctl_context *ctx) &column, &key_string, NULL, NULL, 0, NULL)); - ovsdb_idl_txn_read(row, column, &datum); + datum = ovsdb_idl_read(row, column); if (key_string) { union ovsdb_atom key; unsigned int idx; @@ -2327,7 +2321,7 @@ cmd_get(struct vsctl_context *ctx) &column->type.key, key_string, ctx->symtab)); - idx = ovsdb_datum_find_key(&datum, &key, + idx = ovsdb_datum_find_key(datum, &key, column->type.key.type); if (idx == UINT_MAX) { if (!if_exists) { @@ -2336,15 +2330,14 @@ cmd_get(struct vsctl_context *ctx) column->name); } } else { - ovsdb_atom_to_string(&datum.values[idx], + ovsdb_atom_to_string(&datum->values[idx], column->type.value.type, out); } ovsdb_atom_destroy(&key, column->type.key.type); } else { - ovsdb_datum_to_string(&datum, &column->type, out); + ovsdb_datum_to_string(datum, &column->type, out); } ds_put_char(out, '\n'); - ovsdb_datum_destroy(&datum, &column->type); free(key_string); } @@ -2360,15 +2353,13 @@ list_record(const struct vsctl_table_class *table, UUID_ARGS(&row->uuid)); for (i = 0; i < table->class->n_columns; i++) { const struct ovsdb_idl_column *column = &table->class->columns[i]; - struct ovsdb_datum datum; + const struct ovsdb_datum *datum; - ovsdb_idl_txn_read(row, column, &datum); + datum = ovsdb_idl_read(row, column); ds_put_format(out, "%-20s: ", column->name); - ovsdb_datum_to_string(&datum, &column->type, out); + ovsdb_datum_to_string(datum, &column->type, out); ds_put_char(out, '\n'); - - ovsdb_datum_destroy(&datum, &column->type); } } @@ -2421,7 +2412,7 @@ set_column(const struct vsctl_table_class *table, if (key_string) { union ovsdb_atom key, value; - struct ovsdb_datum old, new; + struct ovsdb_datum datum; if (column->type.value.type == OVSDB_TYPE_VOID) { vsctl_fatal("cannot specify key to set for non-map column %s", @@ -2433,17 +2424,15 @@ set_column(const struct vsctl_table_class *table, die_if_error(ovsdb_atom_from_string(&value, &column->type.value, value_string, symtab)); - ovsdb_datum_init_empty(&new); - ovsdb_datum_add_unsafe(&new, &key, &value, &column->type); + ovsdb_datum_init_empty(&datum); + ovsdb_datum_add_unsafe(&datum, &key, &value, &column->type); ovsdb_atom_destroy(&key, column->type.key.type); ovsdb_atom_destroy(&value, column->type.value.type); - ovsdb_idl_txn_read(row, column, &old); - ovsdb_datum_union(&old, &new, &column->type, true); - ovsdb_idl_txn_write(row, column, &old); - - ovsdb_datum_destroy(&new, &column->type); + ovsdb_datum_union(&datum, ovsdb_idl_read(row, column), + &column->type, false); + ovsdb_idl_txn_write(row, column, &datum); } else { struct ovsdb_datum datum; @@ -2490,7 +2479,7 @@ cmd_add(struct vsctl_context *ctx) die_if_error(get_column(table, column_name, &column)); type = &column->type; - ovsdb_idl_txn_read(row, column, &old); + ovsdb_datum_clone(&old, ovsdb_idl_read(row, column), &column->type); for (i = 4; i < ctx->argc; i++) { struct ovsdb_type add_type; struct ovsdb_datum add; @@ -2531,7 +2520,7 @@ cmd_remove(struct vsctl_context *ctx) die_if_error(get_column(table, column_name, &column)); type = &column->type; - ovsdb_idl_txn_read(row, column, &old); + ovsdb_datum_clone(&old, ovsdb_idl_read(row, column), &column->type); for (i = 4; i < ctx->argc; i++) { struct ovsdb_type rm_type; struct ovsdb_datum rm; @@ -2681,8 +2670,8 @@ is_condition_satified(const struct vsctl_table_class *table, }; const struct ovsdb_idl_column *column; + const struct ovsdb_datum *have_datum; char *key_string, *value_string; - struct ovsdb_datum have_datum; const char *operator; unsigned int idx; char *error; @@ -2696,7 +2685,7 @@ is_condition_satified(const struct vsctl_table_class *table, vsctl_fatal("%s: missing value", arg); } - ovsdb_idl_txn_read(row, column, &have_datum); + have_datum = ovsdb_idl_read(row, column); if (key_string) { union ovsdb_atom want_key, want_value; @@ -2710,10 +2699,10 @@ is_condition_satified(const struct vsctl_table_class *table, die_if_error(ovsdb_atom_from_string(&want_value, &column->type.value, value_string, symtab)); - idx = ovsdb_datum_find_key(&have_datum, + idx = ovsdb_datum_find_key(have_datum, &want_key, column->type.key.type); if (idx != UINT_MAX) { - cmp = ovsdb_atom_compare_3way(&have_datum.values[idx], + cmp = ovsdb_atom_compare_3way(&have_datum->values[idx], &want_value, column->type.value.type); } @@ -2726,11 +2715,10 @@ is_condition_satified(const struct vsctl_table_class *table, die_if_error(ovsdb_datum_from_string(&want_datum, &column->type, value_string, symtab)); idx = 0; - cmp = ovsdb_datum_compare_3way(&have_datum, &want_datum, + cmp = ovsdb_datum_compare_3way(have_datum, &want_datum, &column->type); ovsdb_datum_destroy(&want_datum, &column->type); } - ovsdb_datum_destroy(&have_datum, &column->type); free(key_string); free(value_string); -- 2.30.2