ovsdb-idl: Add a way to assert IDL txn as read only.

almusil · igsilya · commit 42d99eb38725 · 2025-10-08T21:46:37.000+02:00
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.

Acked-by: Dumitru Ceara &lt;dceara@redhat.com&gt;
Signed-off-by: Ales Musil &lt;amusil@redhat.com&gt;
Signed-off-by: Ilya Maximets &lt;i.maximets@ovn.org&gt;
diff --git a/NEWS b/NEWS
@@ -5,6 +5,10 @@ Post-v3.6.0
        connections, instead of limiting these commands to IPv6 only.
    - DPDK:
      * OVS validated with DPDK 24.11.3.
+   - OVSDB-IDL:
+     * New ovsdb_idl_txn_assert_read_only() interface to mark transactions
+       as read-only and trigger assertion failure when application attempts
+       to modify the database data through this transaction.
 
 
 v3.6.0 - 18 Aug 2025
diff --git 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;
@@ -3560,6 +3563,18 @@ ovsdb_idl_txn_abort(struct ovsdb_idl_txn *txn)
     }
 }
 
+/* If 'assert' is true, configures the IDL to generate an assertion
+ * failure when a write operation is attempted on the transaction.
+ * Otherwise, write operations are allowed on the transaction.
+ * 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;
+    }
+}
+
 /* Returns a string that reports the error status for 'txn'.  The caller must
  * not modify or free the returned string.  A call to ovsdb_idl_txn_destroy()
  * for 'txn' may free the returned string.
@@ -3650,6 +3665,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 +3850,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 +3880,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 +4239,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 +4277,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;
diff --git a/lib/ovsdb-idl.h b/lib/ovsdb-idl.h
@@ -351,6 +351,7 @@ void ovsdb_idl_txn_wait(const struct ovsdb_idl_txn *);
 enum ovsdb_idl_txn_status ovsdb_idl_txn_commit(struct ovsdb_idl_txn *);
 enum ovsdb_idl_txn_status ovsdb_idl_txn_commit_block(struct ovsdb_idl_txn *);
 void ovsdb_idl_txn_abort(struct ovsdb_idl_txn *);
+void ovsdb_idl_txn_assert_read_only(struct ovsdb_idl_txn *, bool assert);
 
 const char *ovsdb_idl_txn_get_error(const struct ovsdb_idl_txn *);
 
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
@@ -3000,3 +3000,60 @@ 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_SKIP_IF([grep -q "Assertion tests are not available due to NDEBUG" stderr])
+AT_CHECK([grep -qE "assertion !.*txn->assert_read_only failed" stderr])
+
+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_SKIP_IF([grep -q "Assertion tests are not available due to NDEBUG" stderr])
+AT_CHECK([grep -qE "assertion !.*txn->assert_read_only failed" stderr])
+
+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_SKIP_IF([grep -q "Assertion tests are not available due to NDEBUG" stderr])
+AT_CHECK([grep -qE "assertion !.*txn->assert_read_only failed" stderr])
+
+OVSDB_SERVER_SHUTDOWN
+AT_CLEANUP
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
@@ -16,6 +16,11 @@
 
 #include <config.h>
 
+#ifdef NDEBUG
+#define TEST_OVSDB_SKIP_ASSERT 1
+#undef NDEBUG
+#endif
+
 #include <fcntl.h>
 #include <getopt.h>
 #include <inttypes.h>
@@ -57,6 +62,7 @@ VLOG_DEFINE_THIS_MODULE(test_ovsdb);
 
 struct test_ovsdb_pvt_context {
     bool write_changed_only;
+    bool assert_read_only;
     bool track;
 };
 
@@ -93,6 +99,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 +138,13 @@ parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt)
             pvt->write_changed_only = true;
             break;
 
+        case 'r':
+#ifdef TEST_OVSDB_SKIP_ASSERT
+            ovs_fatal(0, "Assertion tests are not available due to NDEBUG");
+#endif
+            pvt->assert_read_only = true;
+            break;
+
         case OPT_MAGIC:
             magic = optarg;
             break;
@@ -2459,7 +2473,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 +2486,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 +2926,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->assert_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'
