Switch many macros from using CONTAINER_OF to using OBJECT_CONTAINING.
[sliver-openvswitch.git] / lib / ovsdb-idl.c
index ca77cc2..b7ee097 100644 (file)
 #include "poll-loop.h"
 #include "shash.h"
 #include "util.h"
-
-#define THIS_MODULE VLM_ovsdb_idl
 #include "vlog.h"
 
+VLOG_DEFINE_THIS_MODULE(ovsdb_idl)
+
 /* An arc from one idl_row to another.  When row A contains a UUID that
  * references row B, this is represented by an arc from A (the source) to B
  * (the destination).
@@ -112,13 +112,13 @@ static void ovsdb_idl_send_monitor_request(struct ovsdb_idl *);
 static void ovsdb_idl_parse_update(struct ovsdb_idl *, const struct json *);
 static struct ovsdb_error *ovsdb_idl_parse_update__(struct ovsdb_idl *,
                                                     const struct json *);
-static void ovsdb_idl_process_update(struct ovsdb_idl_table *,
+static bool ovsdb_idl_process_update(struct ovsdb_idl_table *,
                                      const struct uuid *,
                                      const struct json *old,
                                      const struct json *new);
 static void ovsdb_idl_insert_row(struct ovsdb_idl_row *, const struct json *);
 static void ovsdb_idl_delete_row(struct ovsdb_idl_row *);
-static void ovsdb_idl_modify_row(struct ovsdb_idl_row *, const struct json *);
+static bool ovsdb_idl_modify_row(struct ovsdb_idl_row *, const struct json *);
 
 static bool ovsdb_idl_row_is_orphan(const struct ovsdb_idl_row *);
 static struct ovsdb_idl_row *ovsdb_idl_row_create__(
@@ -136,6 +136,11 @@ static void ovsdb_idl_txn_abort_all(struct ovsdb_idl *);
 static bool ovsdb_idl_txn_process_reply(struct ovsdb_idl *,
                                         const struct jsonrpc_msg *msg);
 
+/* Creates and returns a connection to database 'remote', which should be in a
+ * form acceptable to jsonrpc_session_open().  The connection will maintain an
+ * in-memory replica of the remote database whose schema is described by
+ * 'class'.  (Ordinarily 'class' is compiled from an OVSDB schema automatically
+ * by ovsdb-idlc.) */
 struct ovsdb_idl *
 ovsdb_idl_create(const char *remote, const struct ovsdb_idl_class *class)
 {
@@ -152,15 +157,15 @@ ovsdb_idl_create(const char *remote, const struct ovsdb_idl_class *class)
         struct ovsdb_idl_table *table = &idl->tables[i];
         size_t j;
 
-        assert(!shash_find(&idl->table_by_name, tc->name));
-        shash_add(&idl->table_by_name, tc->name, table);
+        shash_add_assert(&idl->table_by_name, tc->name, table);
         table->class = tc;
+        table->modes = xmalloc(tc->n_columns);
+        memset(table->modes, OVSDB_IDL_MODE_RW, tc->n_columns);
         shash_init(&table->columns);
         for (j = 0; j < tc->n_columns; j++) {
             const struct ovsdb_idl_column *column = &tc->columns[j];
 
-            assert(!shash_find(&table->columns, column->name));
-            shash_add(&table->columns, column->name, column);
+            shash_add_assert(&table->columns, column->name, column);
         }
         hmap_init(&table->rows);
         table->idl = idl;
@@ -171,6 +176,7 @@ ovsdb_idl_create(const char *remote, const struct ovsdb_idl_class *class)
     return idl;
 }
 
+/* Destroys 'idl' and all of the data structures that it manages. */
 void
 ovsdb_idl_destroy(struct ovsdb_idl *idl)
 {
@@ -185,6 +191,7 @@ ovsdb_idl_destroy(struct ovsdb_idl *idl)
             struct ovsdb_idl_table *table = &idl->tables[i];
             shash_destroy(&table->columns);
             hmap_destroy(&table->rows);
+            free(table->modes);
         }
         shash_destroy(&idl->table_by_name);
         free(idl->tables);
@@ -208,15 +215,13 @@ ovsdb_idl_clear(struct ovsdb_idl *idl)
         }
 
         changed = true;
-        HMAP_FOR_EACH_SAFE (row, next_row, struct ovsdb_idl_row, hmap_node,
-                            &table->rows) {
+        HMAP_FOR_EACH_SAFE (row, next_row, hmap_node, &table->rows) {
             struct ovsdb_idl_arc *arc, *next_arc;
 
             if (!ovsdb_idl_row_is_orphan(row)) {
                 ovsdb_idl_row_unparse(row);
             }
-            LIST_FOR_EACH_SAFE (arc, next_arc, struct ovsdb_idl_arc, src_node,
-                                &row->src_arcs) {
+            LIST_FOR_EACH_SAFE (arc, next_arc, src_node, &row->src_arcs) {
                 free(arc);
             }
             /* No need to do anything with dst_arcs: some node has those arcs
@@ -231,9 +236,30 @@ ovsdb_idl_clear(struct ovsdb_idl *idl)
     }
 }
 
-void
+/* Processes a batch of messages from the database server on 'idl'.  Returns
+ * true if the database as seen through 'idl' changed, false if it did not
+ * change.  The initial fetch of the entire contents of the remote database is
+ * considered to be one kind of change.
+ *
+ * When this function returns false, the client may continue to use any data
+ * structures it obtained from 'idl' in the past.  But when it returns true,
+ * the client must not access any of these data structures again, because they
+ * could have freed or reused for other purposes.
+ *
+ * This function can return occasional false positives, that is, report that
+ * the database changed even though it didn't.  This happens if the connection
+ * to the database drops and reconnects, which causes the database contents to
+ * be reloaded even if they didn't change.  (It could also happen if the
+ * database server sends out a "change" that reflects what we already thought
+ * was in the database, but the database server is not supposed to do that.)
+ *
+ * As an alternative to checking the return value, the client may check for
+ * changes in the value returned by ovsdb_idl_get_seqno().
+ */
+bool
 ovsdb_idl_run(struct ovsdb_idl *idl)
 {
+    unsigned int initial_change_seqno = idl->change_seqno;
     int i;
 
     assert(!idl->txn);
@@ -265,6 +291,7 @@ ovsdb_idl_run(struct ovsdb_idl *idl)
         } else if (msg->type == JSONRPC_REPLY
                    && idl->monitor_request_id
                    && json_equal(idl->monitor_request_id, msg->id)) {
+            idl->change_seqno++;
             json_destroy(idl->monitor_request_id);
             idl->monitor_request_id = NULL;
             ovsdb_idl_clear(idl);
@@ -290,8 +317,12 @@ ovsdb_idl_run(struct ovsdb_idl *idl)
         }
         jsonrpc_msg_destroy(msg);
     }
+
+    return initial_change_seqno != idl->change_seqno;
 }
 
+/* Arranges for poll_block() to wake up when ovsdb_idl_run() has something to
+ * do or when activity occurs on a transaction on 'idl'. */
 void
 ovsdb_idl_wait(struct ovsdb_idl *idl)
 {
@@ -299,23 +330,94 @@ ovsdb_idl_wait(struct ovsdb_idl *idl)
     jsonrpc_session_recv_wait(idl->session);
 }
 
+/* Returns a number that represents the state of 'idl'.  When 'idl' is updated
+ * (by ovsdb_idl_run()), the return value changes. */
 unsigned int
 ovsdb_idl_get_seqno(const struct ovsdb_idl *idl)
 {
     return idl->change_seqno;
 }
 
+/* Returns true if 'idl' successfully connected to the remote database and
+ * retrieved its contents (even if the connection subsequently dropped and is
+ * in the process of reconnecting).  If so, then 'idl' contains an atomic
+ * snapshot of the database's contents (but it might be arbitrarily old if the
+ * connection dropped).
+ *
+ * Returns false if 'idl' has never connected or retrieved the database's
+ * contents.  If so, 'idl' is empty. */
 bool
 ovsdb_idl_has_ever_connected(const struct ovsdb_idl *idl)
 {
     return ovsdb_idl_get_seqno(idl) != 0;
 }
 
+/* Forces 'idl' to drop its connection to the database and reconnect.  In the
+ * meantime, the contents of 'idl' will not change. */
 void
 ovsdb_idl_force_reconnect(struct ovsdb_idl *idl)
 {
     jsonrpc_session_force_reconnect(idl->session);
 }
+
+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;
+        }
+    }
+
+    NOT_REACHED();
+}
+
+/* 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().  For any given column, this function may be
+ * called or ovsdb_idl_omit() may be called, but not both. */
+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);
+}
+
+/* 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().  For any given column, this function may be
+ * called or ovsdb_idl_set_write_only() may be called, but not both. */
+void
+ovsdb_idl_omit(struct ovsdb_idl *idl, const struct ovsdb_idl_column *column)
+{
+    ovsdb_idl_set_mode(idl, column, OVSDB_IDL_MODE_NONE);
+}
 \f
 static void
 ovsdb_idl_send_monitor_request(struct ovsdb_idl *idl)
@@ -329,13 +431,15 @@ ovsdb_idl_send_monitor_request(struct ovsdb_idl *idl)
         const struct ovsdb_idl_table *table = &idl->tables[i];
         const struct ovsdb_idl_table_class *tc = table->class;
         struct json *monitor_request, *columns;
-        size_t i;
+        size_t j;
 
         monitor_request = json_object_create();
         columns = json_array_create_empty();
-        for (i = 0; i < tc->n_columns; i++) {
-            const struct ovsdb_idl_column *column = &tc->columns[i];
-            json_array_add(columns, json_string_create(column->name));
+        for (j = 0; j < tc->n_columns; j++) {
+            const struct ovsdb_idl_column *column = &tc->columns[j];
+            if (table->modes[j] != OVSDB_IDL_MODE_NONE) {
+                json_array_add(columns, json_string_create(column->name));
+            }
         }
         json_object_put(monitor_request, "columns", columns);
         json_object_put(monitor_requests, tc->name, monitor_request);
@@ -353,11 +457,7 @@ ovsdb_idl_send_monitor_request(struct ovsdb_idl *idl)
 static void
 ovsdb_idl_parse_update(struct ovsdb_idl *idl, const struct json *table_updates)
 {
-    struct ovsdb_error *error;
-
-    idl->change_seqno++;
-
-    error = ovsdb_idl_parse_update__(idl, table_updates);
+    struct ovsdb_error *error = ovsdb_idl_parse_update__(idl, table_updates);
     if (error) {
         if (!VLOG_DROP_WARN(&syntax_rl)) {
             char *s = ovsdb_error_to_string(error);
@@ -437,7 +537,9 @@ ovsdb_idl_parse_update__(struct ovsdb_idl *idl,
                                           "and \"new\" members");
             }
 
-            ovsdb_idl_process_update(table, &uuid, old_json, new_json);
+            if (ovsdb_idl_process_update(table, &uuid, old_json, new_json)) {
+                idl->change_seqno++;
+            }
         }
     }
 
@@ -449,8 +551,7 @@ ovsdb_idl_get_row(struct ovsdb_idl_table *table, const struct uuid *uuid)
 {
     struct ovsdb_idl_row *row;
 
-    HMAP_FOR_EACH_WITH_HASH (row, struct ovsdb_idl_row, hmap_node,
-                             uuid_hash(uuid), &table->rows) {
+    HMAP_FOR_EACH_WITH_HASH (row, hmap_node, uuid_hash(uuid), &table->rows) {
         if (uuid_equals(&row->uuid, uuid)) {
             return row;
         }
@@ -458,7 +559,9 @@ ovsdb_idl_get_row(struct ovsdb_idl_table *table, const struct uuid *uuid)
     return NULL;
 }
 
-static void
+/* Returns true if a column with mode OVSDB_IDL_MODE_RW changed, false
+ * otherwise. */
+static bool
 ovsdb_idl_process_update(struct ovsdb_idl_table *table,
                          const struct uuid *uuid, const struct json *old,
                          const struct json *new)
@@ -475,6 +578,7 @@ ovsdb_idl_process_update(struct ovsdb_idl_table *table,
             VLOG_WARN_RL(&semantic_rl, "cannot delete missing row "UUID_FMT" "
                          "from table %s",
                          UUID_ARGS(uuid), table->class->name);
+            return false;
         }
     } else if (!old) {
         /* Insert row. */
@@ -485,14 +589,14 @@ ovsdb_idl_process_update(struct ovsdb_idl_table *table,
         } else {
             VLOG_WARN_RL(&semantic_rl, "cannot add existing row "UUID_FMT" to "
                          "table %s", UUID_ARGS(uuid), table->class->name);
-            ovsdb_idl_modify_row(row, new);
+            return ovsdb_idl_modify_row(row, new);
         }
     } else {
         /* Modify row. */
         if (row) {
             /* XXX perhaps we should check the 'old' values? */
             if (!ovsdb_idl_row_is_orphan(row)) {
-                ovsdb_idl_modify_row(row, new);
+                return ovsdb_idl_modify_row(row, new);
             } else {
                 VLOG_WARN_RL(&semantic_rl, "cannot modify missing but "
                              "referenced row "UUID_FMT" in table %s",
@@ -505,13 +609,18 @@ ovsdb_idl_process_update(struct ovsdb_idl_table *table,
             ovsdb_idl_insert_row(ovsdb_idl_row_create(table, uuid), new);
         }
     }
+
+    return true;
 }
 
-static void
+/* Returns true if a column with mode OVSDB_IDL_MODE_RW changed, false
+ * otherwise. */
+static bool
 ovsdb_idl_row_update(struct ovsdb_idl_row *row, const struct json *row_json)
 {
     struct ovsdb_idl_table *table = row->table;
     struct shash_node *node;
+    bool changed = false;
 
     SHASH_FOR_EACH (node, json_object(row_json)) {
         const char *column_name = node->name;
@@ -528,8 +637,19 @@ ovsdb_idl_row_update(struct ovsdb_idl_row *row, const struct json *row_json)
 
         error = ovsdb_datum_from_json(&datum, &column->type, node->data, NULL);
         if (!error) {
-            ovsdb_datum_swap(&row->old[column - table->class->columns],
-                             &datum);
+            unsigned int column_idx = column - table->class->columns;
+            struct ovsdb_datum *old = &row->old[column_idx];
+
+            if (!ovsdb_datum_equals(old, &datum, &column->type)) {
+                ovsdb_datum_swap(old, &datum);
+                if (table->modes[column_idx] == OVSDB_IDL_MODE_RW) {
+                    changed = true;
+                }
+            } else {
+                /* Didn't really change but the OVSDB monitor protocol always
+                 * includes every value in a row. */
+            }
+
             ovsdb_datum_destroy(&datum, &column->type);
         } else {
             char *s = ovsdb_error_to_string(error);
@@ -540,6 +660,7 @@ ovsdb_idl_row_update(struct ovsdb_idl_row *row, const struct json *row_json)
             ovsdb_error_destroy(error);
         }
     }
+    return changed;
 }
 
 /* When a row A refers to row B through a column with a "refTable" constraint,
@@ -627,8 +748,10 @@ ovsdb_idl_row_clear_new(struct ovsdb_idl_row *row)
             const struct ovsdb_idl_table_class *class = row->table->class;
             size_t i;
 
-            BITMAP_FOR_EACH_1 (i, class->n_columns, row->written) {
-                ovsdb_datum_destroy(&row->new[i], &class->columns[i].type);
+            if (row->written) {
+                BITMAP_FOR_EACH_1 (i, class->n_columns, row->written) {
+                    ovsdb_datum_destroy(&row->new[i], &class->columns[i].type);
+                }
             }
             free(row->new);
             free(row->written);
@@ -645,8 +768,7 @@ ovsdb_idl_row_clear_arcs(struct ovsdb_idl_row *row, bool destroy_dsts)
 
     /* Delete all forward arcs.  If 'destroy_dsts', destroy any orphaned rows
      * that this causes to be unreferenced. */
-    LIST_FOR_EACH_SAFE (arc, next, struct ovsdb_idl_arc, src_node,
-                        &row->src_arcs) {
+    LIST_FOR_EACH_SAFE (arc, next, src_node, &row->src_arcs) {
         list_remove(&arc->dst_node);
         if (destroy_dsts
             && ovsdb_idl_row_is_orphan(arc->dst)
@@ -674,8 +796,7 @@ ovsdb_idl_row_reparse_backrefs(struct ovsdb_idl_row *row)
      * (If duplicate arcs were possible then we would need to make sure that
      * 'next' didn't also point into 'arc''s destination, but we forbid
      * duplicate arcs.) */
-    LIST_FOR_EACH_SAFE (arc, next, struct ovsdb_idl_arc, dst_node,
-                        &row->dst_arcs) {
+    LIST_FOR_EACH_SAFE (arc, next, dst_node, &row->dst_arcs) {
         struct ovsdb_idl_row *ref = arc->src;
 
         ovsdb_idl_row_unparse(ref);
@@ -744,13 +865,19 @@ ovsdb_idl_delete_row(struct ovsdb_idl_row *row)
     }
 }
 
-static void
+/* Returns true if a column with mode OVSDB_IDL_MODE_RW changed, false
+ * otherwise. */
+static bool
 ovsdb_idl_modify_row(struct ovsdb_idl_row *row, const struct json *row_json)
 {
+    bool changed;
+
     ovsdb_idl_row_unparse(row);
     ovsdb_idl_row_clear_arcs(row, true);
-    ovsdb_idl_row_update(row, row_json);
+    changed = ovsdb_idl_row_update(row, row_json);
     ovsdb_idl_row_parse(row);
+
+    return changed;
 }
 
 static bool
@@ -865,6 +992,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);
+}
 \f
 /* Transactions. */
 
@@ -964,8 +1140,7 @@ ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *txn)
     free(txn->inc_table);
     free(txn->inc_column);
     json_destroy(txn->inc_where);
-    HMAP_FOR_EACH_SAFE (insert, next, struct ovsdb_idl_txn_insert, hmap_node,
-                        &txn->inserted_rows) {
+    HMAP_FOR_EACH_SAFE (insert, next, hmap_node, &txn->inserted_rows) {
         free(insert);
     }
     hmap_destroy(&txn->inserted_rows);
@@ -1015,8 +1190,7 @@ ovsdb_idl_txn_get_row(const struct ovsdb_idl_txn *txn, const struct uuid *uuid)
 {
     const struct ovsdb_idl_row *row;
 
-    HMAP_FOR_EACH_WITH_HASH (row, struct ovsdb_idl_row, txn_node,
-                             uuid_hash(uuid), &txn->txn_rows) {
+    HMAP_FOR_EACH_WITH_HASH (row, txn_node, uuid_hash(uuid), &txn->txn_rows) {
         if (uuid_equals(&row->uuid, uuid)) {
             return row;
         }
@@ -1074,8 +1248,7 @@ ovsdb_idl_txn_disassemble(struct ovsdb_idl_txn *txn)
      * transaction and fail to update the graph.  */
     txn->idl->txn = NULL;
 
-    HMAP_FOR_EACH_SAFE (row, next, struct ovsdb_idl_row, txn_node,
-                        &txn->txn_rows) {
+    HMAP_FOR_EACH_SAFE (row, next, txn_node, &txn->txn_rows) {
         if (row->old) {
             if (row->written) {
                 ovsdb_idl_row_unparse(row);
@@ -1119,7 +1292,7 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
         json_string_create(txn->idl->class->database));
 
     /* Add prerequisites and declarations of new rows. */
-    HMAP_FOR_EACH (row, struct ovsdb_idl_row, txn_node, &txn->txn_rows) {
+    HMAP_FOR_EACH (row, txn_node, &txn->txn_rows) {
         /* XXX check that deleted rows exist even if no prereqs? */
         if (row->prereqs) {
             const struct ovsdb_idl_table_class *class = row->table->class;
@@ -1151,7 +1324,7 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
 
     /* Add updates. */
     any_updates = false;
-    HMAP_FOR_EACH (row, struct ovsdb_idl_row, txn_node, &txn->txn_rows) {
+    HMAP_FOR_EACH (row, txn_node, &txn->txn_rows) {
         const struct ovsdb_idl_table_class *class = row->table->class;
 
         if (row->old == row->new) {
@@ -1190,18 +1363,22 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
             row_json = json_object_create();
             json_object_put(op, "row", row_json);
 
-            BITMAP_FOR_EACH_1 (idx, class->n_columns, row->written) {
-                const struct ovsdb_idl_column *column = &class->columns[idx];
-
-                if (row->old
-                    ? !ovsdb_datum_equals(&row->old[idx], &row->new[idx],
-                                          &column->type)
-                    : !ovsdb_datum_is_default(&row->new[idx], &column->type)) {
-                    json_object_put(row_json, column->name,
-                                    substitute_uuids(
-                                        ovsdb_datum_to_json(&row->new[idx],
-                                                            &column->type),
-                                        txn));
+            if (row->written) {
+                BITMAP_FOR_EACH_1 (idx, class->n_columns, row->written) {
+                    const struct ovsdb_idl_column *column =
+                                                        &class->columns[idx];
+
+                    if (row->old
+                        ? !ovsdb_datum_equals(&row->old[idx], &row->new[idx],
+                                              &column->type)
+                        : !ovsdb_datum_is_default(&row->new[idx],
+                                                  &column->type)) {
+                        json_object_put(row_json, column->name,
+                                        substitute_uuids(
+                                            ovsdb_datum_to_json(&row->new[idx],
+                                                                &column->type),
+                                            txn));
+                    }
                 }
             }
 
@@ -1345,7 +1522,7 @@ ovsdb_idl_txn_get_insert_uuid(const struct ovsdb_idl_txn *txn,
     const struct ovsdb_idl_txn_insert *insert;
 
     assert(txn->status == TXN_SUCCESS || txn->status == TXN_UNCHANGED);
-    HMAP_FOR_EACH_IN_BUCKET (insert, struct ovsdb_idl_txn_insert, hmap_node,
+    HMAP_FOR_EACH_IN_BUCKET (insert, hmap_node,
                              uuid_hash(uuid), &txn->inserted_rows) {
         if (uuid_equals(uuid, &insert->dummy)) {
             return &insert->real;
@@ -1362,24 +1539,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,
@@ -1391,6 +1550,8 @@ ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_,
 
     assert(row->new != NULL);
     assert(column_idx < class->n_columns);
+    assert(row->table->modes[column_idx] != OVSDB_IDL_MODE_NONE);
+
     if (hmap_node_is_null(&row->txn_node)) {
         hmap_insert(&row->table->idl->txn->txn_rows, &row->txn_node,
                     uuid_hash(&row->uuid));
@@ -1474,7 +1635,6 @@ ovsdb_idl_txn_insert(struct ovsdb_idl_txn *txn,
 
     row->table = ovsdb_idl_table_from_class(txn->idl, class);
     row->new = xmalloc(class->n_columns * sizeof *row->new);
-    row->written = bitmap_allocate(class->n_columns);
     hmap_insert(&row->table->rows, &row->hmap_node, uuid_hash(&row->uuid));
     hmap_insert(&txn->txn_rows, &row->txn_node, uuid_hash(&row->uuid));
     return row;
@@ -1485,8 +1645,7 @@ ovsdb_idl_txn_abort_all(struct ovsdb_idl *idl)
 {
     struct ovsdb_idl_txn *txn;
 
-    HMAP_FOR_EACH (txn, struct ovsdb_idl_txn, hmap_node,
-                   &idl->outstanding_txns) {
+    HMAP_FOR_EACH (txn, hmap_node, &idl->outstanding_txns) {
         ovsdb_idl_txn_complete(txn, TXN_TRY_AGAIN);
     }
 }
@@ -1496,7 +1655,7 @@ ovsdb_idl_txn_find(struct ovsdb_idl *idl, const struct json *id)
 {
     struct ovsdb_idl_txn *txn;
 
-    HMAP_FOR_EACH_WITH_HASH (txn, struct ovsdb_idl_txn, hmap_node,
+    HMAP_FOR_EACH_WITH_HASH (txn, hmap_node,
                              json_hash(id, 0), &idl->outstanding_txns) {
         if (json_equal(id, txn->request_id)) {
             return txn;
@@ -1676,8 +1835,7 @@ ovsdb_idl_txn_process_reply(struct ovsdb_idl *idl,
                 hard_errors++;
             }
 
-            HMAP_FOR_EACH (insert, struct ovsdb_idl_txn_insert, hmap_node,
-                           &txn->inserted_rows) {
+            HMAP_FOR_EACH (insert, hmap_node, &txn->inserted_rows) {
                 if (!ovsdb_idl_txn_process_insert_reply(insert, ops)) {
                     hard_errors++;
                 }