From: Ethan Jackson Date: Thu, 20 Sep 2012 18:13:15 +0000 (-0700) Subject: idl: Optionally warn when writing to read-write columns. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8cdec7256967ce4d9abd7aa55778181b6fa3bae8;p=openvswitch idl: Optionally warn when writing to read-write columns. ovs-vswitchd should only write to write-only columns. Furthermore, writing to a column which is not write-only can cause serious performance degradations. This patch causes ovs-vswitchd to log and reject writes to read-write columns. Signed-off-by: Ethan Jackson --- diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 61188524..be4b255a 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -70,6 +70,7 @@ struct ovsdb_idl { struct json *monitor_request_id; unsigned int last_monitor_request_seqno; unsigned int change_seqno; + bool verify_write_only; /* Database locking. */ char *lock_name; /* Name of lock we need, NULL if none. */ @@ -402,6 +403,16 @@ ovsdb_idl_force_reconnect(struct ovsdb_idl *idl) { jsonrpc_session_force_reconnect(idl->session); } + +/* Some IDL users should only write to write-only columns. Furthermore, + * writing to a column which is not write-only can cause serious performance + * degradations for these users. This function causes 'idl' to reject writes + * to columns which are not marked write only using ovsdb_idl_omit_alert(). */ +void +ovsdb_idl_verify_write_only(struct ovsdb_idl *idl) +{ + idl->verify_write_only = true; +} static unsigned char * ovsdb_idl_get_mode(struct ovsdb_idl *idl, @@ -1824,6 +1835,7 @@ ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_, struct ovsdb_idl_row *row = CONST_CAST(struct ovsdb_idl_row *, row_); const struct ovsdb_idl_table_class *class; size_t column_idx; + bool write_only; if (ovsdb_idl_row_is_synthetic(row)) { ovsdb_datum_destroy(datum, &column->type); @@ -1832,12 +1844,20 @@ ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_, class = row->table->class; column_idx = column - class->columns; + write_only = row->table->modes[column_idx] == OVSDB_IDL_MONITOR; assert(row->new != NULL); assert(column_idx < class->n_columns); assert(row->old == NULL || row->table->modes[column_idx] & OVSDB_IDL_MONITOR); + if (row->table->idl->verify_write_only && !write_only) { + VLOG_ERR("Bug: Attempt to write to a read/write column (%s:%s) when" + " explicitly configured not to.", class->name, column->name); + ovsdb_datum_destroy(datum, &column->type); + return; + } + /* If this is a write-only column and the datum being written is the same * as the one already there, just skip the update entirely. This is worth * optimizing because we have a lot of columns that get periodically @@ -1849,9 +1869,8 @@ ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_, * transaction only does writes of existing values, without making any real * changes, we will drop the whole transaction later in * ovsdb_idl_txn_commit().) */ - if (row->table->modes[column_idx] == OVSDB_IDL_MONITOR - && ovsdb_datum_equals(ovsdb_idl_read(row, column), - datum, &column->type)) { + if (write_only && ovsdb_datum_equals(ovsdb_idl_read(row, column), + datum, &column->type)) { ovsdb_datum_destroy(datum, &column->type); return; } diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h index c48ad1bd..27008b76 100644 --- a/lib/ovsdb-idl.h +++ b/lib/ovsdb-idl.h @@ -57,6 +57,7 @@ bool ovsdb_idl_is_lock_contended(const struct ovsdb_idl *); unsigned int ovsdb_idl_get_seqno(const struct ovsdb_idl *); bool ovsdb_idl_has_ever_connected(const struct ovsdb_idl *); void ovsdb_idl_force_reconnect(struct ovsdb_idl *); +void ovsdb_idl_verify_write_only(struct ovsdb_idl *); /* Choosing columns and tables to replicate. */ diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c index 940e5e7f..d161c4c1 100644 --- a/vswitchd/bridge.c +++ b/vswitchd/bridge.c @@ -274,6 +274,7 @@ bridge_init(const char *remote) idl = ovsdb_idl_create(remote, &ovsrec_idl_class, true); idl_seqno = ovsdb_idl_get_seqno(idl); ovsdb_idl_set_lock(idl, "ovs_vswitchd"); + ovsdb_idl_verify_write_only(idl); ovsdb_idl_omit_alert(idl, &ovsrec_open_vswitch_col_cur_cfg); ovsdb_idl_omit_alert(idl, &ovsrec_open_vswitch_col_statistics);