ovsdb-idl: Plug hole in state machine.

[sliver-openvswitch.git] / lib / ovsdb-idl.c

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index e1b53f4..51a62dd 100644 (file)
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -1108,6 +1108,15 @@ ovsdb_idl_get(const struct ovsdb_idl_row *row,
 
     return ovsdb_idl_read(row, column);
 }
+
+/* Returns false if 'row' was obtained from the IDL, true if it was initialized
+ * to all-zero-bits by some other entity.  If 'row' was set up some other way
+ * then the return value is indeterminate. */
+bool
+ovsdb_idl_row_is_synthetic(const struct ovsdb_idl_row *row)
+{
+    return row->table == NULL;
+}
 \f
 /* Transactions. */
 
@@ -1118,6 +1127,8 @@ const char *
 ovsdb_idl_txn_status_to_string(enum ovsdb_idl_txn_status status)
 {
     switch (status) {
+    case TXN_UNCOMMITTED:
+        return "uncommitted";
     case TXN_UNCHANGED:
         return "unchanged";
     case TXN_INCOMPLETE:
@@ -1144,7 +1155,7 @@ ovsdb_idl_txn_create(struct ovsdb_idl *idl)
     txn->request_id = NULL;
     txn->idl = idl;
     hmap_init(&txn->txn_rows);
-    txn->status = TXN_INCOMPLETE;
+    txn->status = TXN_UNCOMMITTED;
     txn->error = NULL;
     txn->dry_run = false;
     ds_init(&txn->comment);
@@ -1217,7 +1228,7 @@ ovsdb_idl_txn_destroy(struct ovsdb_idl_txn *txn)
 void
 ovsdb_idl_txn_wait(const struct ovsdb_idl_txn *txn)
 {
-    if (txn->status != TXN_INCOMPLETE) {
+    if (txn->status != TXN_UNCOMMITTED && txn->status != TXN_INCOMPLETE) {
         poll_immediate_wake();
     }
 }
@@ -1420,6 +1431,8 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
             } else {
                 struct ovsdb_idl_txn_insert *insert;
 
+                any_updates = true;
+
                 json_object_put(op, "uuid-name",
                                 json_string_create_nocopy(
                                     uuid_name_from_uuid(&row->uuid)));
@@ -1447,13 +1460,22 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
                                             ovsdb_datum_to_json(&row->new[idx],
                                                                 &column->type),
                                             txn));
+
+                        /* If anything really changed, consider it an update.
+                         * We can't suppress not-really-changed values earlier
+                         * or transactions would become nonatomic (see the big
+                         * comment inside ovsdb_idl_txn_write()). */
+                        if (!any_updates && row->old &&
+                            !ovsdb_datum_equals(&row->old[idx], &row->new[idx],
+                                                &column->type)) {
+                            any_updates = true;
+                        }
                     }
                 }
             }
 
             if (!row->old || !shash_is_empty(json_object(row_json))) {
                 json_array_add(operations, op);
-                any_updates = true;
             } else {
                 json_destroy(op);
             }
@@ -1512,6 +1534,7 @@ ovsdb_idl_txn_commit(struct ovsdb_idl_txn *txn)
                        "transact", operations, &txn->request_id))) {
         hmap_insert(&txn->idl->outstanding_txns, &txn->hmap_node,
                     json_hash(txn->request_id, 0));
+        txn->status = TXN_INCOMPLETE;
     } else {
         txn->status = TXN_TRY_AGAIN;
     }
@@ -1549,7 +1572,7 @@ void
 ovsdb_idl_txn_abort(struct ovsdb_idl_txn *txn)
 {
     ovsdb_idl_txn_disassemble(txn);
-    if (txn->status == TXN_INCOMPLETE) {
+    if (txn->status == TXN_UNCOMMITTED || txn->status == TXN_INCOMPLETE) {
         txn->status = TXN_ABORTED;
     }
 }
@@ -1620,7 +1643,11 @@ ovsdb_idl_txn_complete(struct ovsdb_idl_txn *txn,
  * been disabled (by calling ovsdb_idl_omit()).
  *
  * Usually this function is used indirectly through one of the "set" functions
- * generated by ovsdb-idlc. */
+ * generated by ovsdb-idlc.
+ *
+ * Takes ownership of what 'datum' points to (and in some cases destroys that
+ * data before returning) but makes a copy of 'datum' itself.  (Commonly
+ * 'datum' is on the caller's stack.) */
 void
 ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_,
                     const struct ovsdb_idl_column *column,
@@ -1642,7 +1669,10 @@ ovsdb_idl_txn_write(const struct ovsdb_idl_row *row_,
      *
      * We don't do this for read/write columns because that would break
      * atomicity of transactions--some other client might have written a
-     * different value in that column since we read it. */
+     * different value in that column since we read it.  (But if a whole
+     * 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)) {