Add a way to assert IDL txn as read only, that is useful in cases when we want to ensure that the program doesn't write anything into the txn. It is done via assert because this is considered a bug which should be fixed in the IDL caller.
Signed-off-by: Ales Musil <[email protected]> --- lib/ovsdb-idl.c | 21 +++++++++++++++++++++ lib/ovsdb-idl.h | 1 + tests/ovsdb-idl.at | 43 +++++++++++++++++++++++++++++++++++++++++++ tests/test-ovsdb.c | 11 +++++++++-- 4 files changed, 74 insertions(+), 2 deletions(-) diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index cd781f300..ed98c011e 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -111,6 +111,7 @@ struct ovsdb_idl_txn { enum ovsdb_idl_txn_status status; char *error; bool dry_run; + bool assert_read_only; struct ds comment; /* Increments. */ @@ -2771,6 +2772,7 @@ ovsdb_idl_txn_create(struct ovsdb_idl *idl) txn->status = TXN_UNCOMMITTED; txn->error = NULL; txn->dry_run = false; + txn->assert_read_only = false; ds_init(&txn->comment); txn->inc_table = NULL; @@ -2839,6 +2841,7 @@ ovsdb_idl_txn_increment(struct ovsdb_idl_txn *txn, ovs_assert(!txn->inc_table); ovs_assert(column->type.key.type == OVSDB_TYPE_INTEGER); ovs_assert(column->type.value.type == OVSDB_TYPE_VOID); + ovs_assert(!txn->assert_read_only); txn->inc_table = row->table->class_->name; txn->inc_column = column->name; @@ -3650,6 +3653,7 @@ ovsdb_idl_txn_write__(const struct ovsdb_idl_row *row_, ovs_assert(row->new_datum != NULL); ovs_assert(column_idx < class->n_columns); ovs_assert(row->old_datum == NULL || column_mode & OVSDB_IDL_MONITOR); + ovs_assert(!row->table->idl->txn->assert_read_only); if (row->table->idl->verify_write_only && !write_only) { VLOG_ERR("Bug: Attempt to write to a read/write column (%s:%s) when" @@ -3834,6 +3838,7 @@ ovsdb_idl_txn_delete(const struct ovsdb_idl_row *row_) ovs_assert(row->new_datum != NULL); ovs_assert(!is_index_row(row_)); + ovs_assert(!row->table->idl->txn->assert_read_only); ovsdb_idl_remove_from_indexes(row_); if (!row->old_datum) { ovsdb_idl_row_unparse(row); @@ -3863,6 +3868,7 @@ ovsdb_idl_txn_insert__(struct ovsdb_idl_txn *txn, struct ovsdb_idl_row *row = ovsdb_idl_row_create__(class); ovs_assert(uuid || !persist_uuid); + ovs_assert(!txn->assert_read_only); if (uuid) { ovs_assert(!ovsdb_idl_txn_get_row(txn, uuid)); row->uuid = *uuid; @@ -4221,6 +4227,8 @@ ovsdb_idl_txn_add_map_op(struct ovsdb_idl_row *row, struct ovsdb_datum *datum, enum map_op_type op_type) { + ovs_assert(!row->table->idl->txn->assert_read_only); + const struct ovsdb_idl_table_class *class; size_t column_idx; struct map_op *map_op; @@ -4257,6 +4265,8 @@ ovsdb_idl_txn_add_set_op(struct ovsdb_idl_row *row, struct ovsdb_datum *datum, enum set_op_type op_type) { + ovs_assert(!row->table->idl->txn->assert_read_only); + const struct ovsdb_idl_table_class *class; size_t column_idx; struct set_op *set_op; @@ -4566,3 +4576,14 @@ ovsdb_idl_loop_commit_and_wait(struct ovsdb_idl_loop *loop) return retval; } + +/* Set the IDL txn to be read only or read/write. When the txn is set to + * read only any write operation attempted on the txn will fail. + * The check will turn into no-op when building with NDEBUG. */ +void +ovsdb_idl_txn_assert_read_only(struct ovsdb_idl_txn *txn, bool assert) +{ + if (txn) { + txn->assert_read_only = assert; + } +} diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h index 0d4864b6f..18e64456e 100644 --- a/lib/ovsdb-idl.h +++ b/lib/ovsdb-idl.h @@ -385,6 +385,7 @@ const struct ovsdb_idl_row *ovsdb_idl_txn_insert_persist_uuid( const struct uuid *uuid); struct ovsdb_idl *ovsdb_idl_txn_get_idl (struct ovsdb_idl_txn *); +void ovsdb_idl_txn_assert_read_only(struct ovsdb_idl_txn *txn, bool assert); void ovsdb_idl_get_initial_snapshot(struct ovsdb_idl *); diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at index 8dcc4c75f..69836c416 100644 --- a/tests/ovsdb-idl.at +++ b/tests/ovsdb-idl.at @@ -3000,3 +3000,46 @@ m4_define([OVSDB_CHECK_IDL_CHANGE_AWARE], OVSDB_CHECK_IDL_CHANGE_AWARE([standalone]) OVSDB_CHECK_IDL_CHANGE_AWARE([clustered]) + + +AT_SETUP([ovsdb-idl - read only insert - C]) +AT_KEYWORDS([ovsdb server idl read only assert]) +OVSDB_START_IDLTEST + +AT_CHECK([[ovsdb-client transact unix:socket '["idltest", + {"op": "insert", + "table": "simple", + "row": {}}]']], [0], [ignore], [ignore]) +AT_CHECK([[test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc -t10 --assert-read-only idl unix:socket 'insert 1']], [ignore], [ignore], [stderr]) +AT_CHECK([grep -qE "assertion !.*txn->assert_read_only failed" stderr], [0]) + +OVSDB_SERVER_SHUTDOWN +AT_CLEANUP + +AT_SETUP([ovsdb-idl - read only set - C]) +AT_KEYWORDS([ovsdb server idl read only assert]) +OVSDB_START_IDLTEST + +AT_CHECK([[ovsdb-client transact unix:socket '["idltest", + {"op": "insert", + "table": "simple", + "row": {}}]']], [0], [ignore], [ignore]) +AT_CHECK([[test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc -t10 --assert-read-only idl unix:socket 'set 0 r 2.0']], [ignore], [ignore], [stderr]) +AT_CHECK([grep -qE "assertion !.*txn->assert_read_only failed" stderr], [0]) + +OVSDB_SERVER_SHUTDOWN +AT_CLEANUP + +AT_SETUP([ovsdb-idl - read only delete - C]) +AT_KEYWORDS([ovsdb server idl read only assert]) +OVSDB_START_IDLTEST + +AT_CHECK([[ovsdb-client transact unix:socket '["idltest", + {"op": "insert", + "table": "simple", + "row": {"i": 1}}]']], [0], [ignore], [ignore]) +AT_CHECK([[test-ovsdb '-vPATTERN:console:test-ovsdb|%c|%m' -vjsonrpc -t10 --assert-read-only idl unix:socket 'delete 1']], [ignore], [ignore], [stderr]) +AT_CHECK([grep -qE "assertion !.*txn->assert_read_only failed" stderr], [0]) + +OVSDB_SERVER_SHUTDOWN +AT_CLEANUP diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index d3bb199ef..4cd454892 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -58,6 +58,7 @@ VLOG_DEFINE_THIS_MODULE(test_ovsdb); struct test_ovsdb_pvt_context { bool write_changed_only; bool track; + bool aseert_read_only; }; /* Magic to pass to ovsdb_log_open(). */ @@ -93,6 +94,7 @@ parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt) {"verbose", optional_argument, NULL, 'v'}, {"change-track", optional_argument, NULL, 'c'}, {"write-changed-only", optional_argument, NULL, 'w'}, + {"assert-read-only", optional_argument, NULL, 'r'}, {"magic", required_argument, NULL, OPT_MAGIC}, {"no-rename-open-files", no_argument, NULL, OPT_NO_RENAME_OPEN_FILES}, {"help", no_argument, NULL, 'h'}, @@ -131,6 +133,10 @@ parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt) pvt->write_changed_only = true; break; + case 'r': + pvt->aseert_read_only = true; + break; + case OPT_MAGIC: magic = optarg; break; @@ -2459,7 +2465,7 @@ idltest_find_simple(struct ovsdb_idl *idl, int i) } static bool -idl_set(struct ovsdb_idl *idl, char *commands, int step) +idl_set(struct ovsdb_idl *idl, char *commands, int step, bool assert_read_only) { char *cmd, *save_ptr1 = NULL; struct ovsdb_idl_txn *txn; @@ -2472,6 +2478,7 @@ idl_set(struct ovsdb_idl *idl, char *commands, int step) txn = ovsdb_idl_txn_create(idl); ovsdb_idl_check_consistency(idl); + ovsdb_idl_txn_assert_read_only(txn, assert_read_only); for (cmd = strtok_r(commands, ",", &save_ptr1); cmd; cmd = strtok_r(NULL, ",", &save_ptr1)) { char *save_ptr2 = NULL; @@ -2911,7 +2918,7 @@ do_idl(struct ovs_cmdl_context *ctx) next_cond_seqno = update_conditions(idl, arg + strlen(cond_s), step++); } else if (arg[0] != '[') { - if (!idl_set(idl, arg, step++)) { + if (!idl_set(idl, arg, step++, pvt->aseert_read_only)) { /* If idl_set() returns false, then no transaction * was sent to the server and most likely 'seqno' * would remain the same. And the above 'Wait for update' -- 2.51.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
