[ovs-dev,4/5] ovsdb: Preserve column diffs read from the storage.

diff --git a/ovsdb/execution.c b/ovsdb/execution.c
index 5587ef96f..8c20c3b54 100644
--- a/ovsdb/execution.c
+++ b/ovsdb/execution.c
@@ -490,9 +490,11 @@  update_row_cb(const struct ovsdb_row *row, void *ur_)
 
     ur->n_matches++;
     if (!ovsdb_row_equal_columns(row, ur->row, ur->columns)) {
+        struct ovsdb_row *rw_row;
+
+        ovsdb_txn_row_modify(ur->txn, row, &rw_row, NULL);
         ovsdb_error_assert(ovsdb_row_update_columns(
-                               ovsdb_txn_row_modify(ur->txn, row),
-                               ur->row, ur->columns, false));
+                               rw_row, ur->row, ur->columns, false));
     }
 
     return true;
@@ -572,10 +574,14 @@  static bool
 mutate_row_cb(const struct ovsdb_row *row, void *mr_)
 {
     struct mutate_row_cbdata *mr = mr_;
+    struct ovsdb_row *rw_row;
+
+    /* Not trying to track the row diff here, because user transactions
+     * may attempt to add duplicates or remove elements that do not exist. */
+    ovsdb_txn_row_modify(mr->txn, row, &rw_row, NULL);
 
     mr->n_matches++;
-    *mr->error = ovsdb_mutation_set_execute(ovsdb_txn_row_modify(mr->txn, row),
-                                            mr->mutations);
+    *mr->error = ovsdb_mutation_set_execute(rw_row, mr->mutations);
     return *mr->error == NULL;
 }
 
diff --git a/ovsdb/file.c b/ovsdb/file.c
index 8bd1d4af3..77a89fd1a 100644
--- a/ovsdb/file.c
+++ b/ovsdb/file.c
@@ -80,8 +80,8 @@  ovsdb_file_column_diff_disable(void)
 }
 
 static struct ovsdb_error *
-ovsdb_file_update_row_from_json(struct ovsdb_row *row, bool converting,
-                                bool row_contains_diff,
+ovsdb_file_update_row_from_json(struct ovsdb_row *row, struct ovsdb_row *diff,
+                                bool converting, bool row_contains_diff,
                                 const struct json *json)
 {
     struct ovsdb_table_schema *schema = row->table->schema;
@@ -122,6 +122,12 @@  ovsdb_file_update_row_from_json(struct ovsdb_row *row, bool converting,
             error = ovsdb_datum_apply_diff_in_place(
                                            &row->fields[column->index],
                                            &datum, &column->type);
+            if (!error && diff) {
+                ovs_assert(ovsdb_datum_is_default(&diff->fields[column->index],
+                                                  &column->type));
+                ovsdb_datum_swap(&diff->fields[column->index], &datum);
+            }
+
             ovsdb_datum_destroy(&datum, &column->type);
             if (error) {
                 return error;
@@ -150,16 +156,20 @@  ovsdb_file_txn_row_from_json(struct ovsdb_txn *txn, struct ovsdb_table *table,
         ovsdb_txn_row_delete(txn, row);
         return NULL;
     } else if (row) {
-        return ovsdb_file_update_row_from_json(ovsdb_txn_row_modify(txn, row),
-                                               converting, row_contains_diff,
-                                               json);
+        struct ovsdb_row *new, *diff = NULL;
+
+        ovsdb_txn_row_modify(txn, row, &new,
+                             row_contains_diff ? &diff : NULL);
+        return ovsdb_file_update_row_from_json(new, diff, converting,
+                                               row_contains_diff, json);
     } else {
         struct ovsdb_error *error;
         struct ovsdb_row *new;
 
         new = ovsdb_row_create(table);
         *ovsdb_row_get_uuid_rw(new) = *row_uuid;
-        error = ovsdb_file_update_row_from_json(new, converting, false, json);
+        error = ovsdb_file_update_row_from_json(new, NULL, converting,
+                                                false, json);
         if (error) {
             ovsdb_row_destroy(new);
         } else {
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 4d29043f4..dbf85fe3b 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -1111,7 +1111,7 @@  update_remote_row(const struct ovsdb_row *row, struct ovsdb_txn *txn,
         /* Bad remote spec or incorrect schema. */
         return;
     }
-    rw_row = ovsdb_txn_row_modify(txn, row);
+    ovsdb_txn_row_modify(txn, row, &rw_row, NULL);
     ovsdb_jsonrpc_server_get_remote_status(jsonrpc, target, &status);
 
     /* Update status information columns. */
@@ -1301,7 +1301,10 @@  update_server_status(struct shash *all_dbs)
         if (!db || !db->db) {
             ovsdb_txn_row_delete(txn, row);
         } else {
-            update_database_status(ovsdb_txn_row_modify(txn, row), db);
+            struct ovsdb_row *rw_row;
+
+            ovsdb_txn_row_modify(txn, row, &rw_row, NULL);
+            update_database_status(rw_row, db);
         }
     }
 
diff --git a/ovsdb/table.c b/ovsdb/table.c
index 0792e1580..3e89ddd44 100644
--- a/ovsdb/table.c
+++ b/ovsdb/table.c
@@ -415,8 +415,10 @@  ovsdb_table_execute_update(struct ovsdb_txn *txn, const struct uuid *row_uuid,
                                                     NULL, &columns, xor);
 
     if (!error && (xor || !ovsdb_row_equal_columns(row, update, &columns))) {
-        error = ovsdb_row_update_columns(ovsdb_txn_row_modify(txn, row),
-                                         update, &columns, xor);
+        struct ovsdb_row *rw_row;
+
+        ovsdb_txn_row_modify(txn, row, &rw_row, NULL);
+        error = ovsdb_row_update_columns(rw_row, update, &columns, xor);
     }
 
     ovsdb_column_set_destroy(&columns);
diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
index a482588a0..b69d03b5a 100644
--- a/ovsdb/transaction.c
+++ b/ovsdb/transaction.c
@@ -72,6 +72,8 @@  struct ovsdb_txn_table {
  *        'new'.
  *
  *      - A row modified by a transaction will have non-null 'old' and 'new'.
+ *        It may have non-null 'diff' as well in this case, but it is not
+ *        necessary.
  *
  *      - 'old' and 'new' both null indicates that a row was added then deleted
  *        within a single transaction.  Most of the time we instead delete the
@@ -83,6 +85,7 @@  struct ovsdb_txn_row {
     struct hmap_node hmap_node; /* In ovsdb_txn_table's txn_rows hmap. */
     struct ovsdb_row *old;      /* The old row. */
     struct ovsdb_row *new;      /* The new row. */
+    struct ovsdb_row *diff;     /* The difference between old and new. */
     size_t n_refs;              /* Number of remaining references. */
 
     /* These members are the same as the corresponding members of 'old' or
@@ -155,6 +158,7 @@  ovsdb_txn_row_abort(struct ovsdb_txn *txn OVS_UNUSED,
 {
     struct ovsdb_row *old = txn_row->old;
     struct ovsdb_row *new = txn_row->new;
+    struct ovsdb_row *diff = txn_row->diff;
 
     ovsdb_txn_row_prefree(txn_row);
     if (!old) {
@@ -184,6 +188,7 @@  ovsdb_txn_row_abort(struct ovsdb_txn *txn OVS_UNUSED,
     }
 
     ovsdb_row_destroy(new);
+    ovsdb_row_destroy(diff);
     free(txn_row);
 
     return NULL;
@@ -250,7 +255,10 @@  find_or_make_txn_row(struct ovsdb_txn *txn, const struct ovsdb_table *table,
     if (!txn_row) {
         const struct ovsdb_row *row = ovsdb_table_get_row(table, uuid);
         if (row) {
-            txn_row = ovsdb_txn_row_modify(txn, row)->txn_row;
+            struct ovsdb_row *rw_row;
+
+            ovsdb_txn_row_modify(txn, row, &rw_row, NULL);
+            txn_row = rw_row->txn_row;
         }
     }
     return txn_row;
@@ -433,6 +441,7 @@  delete_garbage_row(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row)
         return NULL;
     }
 
+    ovsdb_row_destroy(txn_row->diff);
     row = txn_row->new;
     txn_row->new = NULL;
     hmap_remove(&txn_row->table->rows, &row->hmap_node);
@@ -544,6 +553,7 @@  ovsdb_txn_row_commit(struct ovsdb_txn *txn OVS_UNUSED,
         txn_row->new->n_refs = txn_row->n_refs;
     }
     ovsdb_row_destroy(txn_row->old);
+    ovsdb_row_destroy(txn_row->diff);
     free(txn_row);
 
     return NULL;
@@ -1178,6 +1188,7 @@  ovsdb_txn_destroy_cloned(struct ovsdb_txn *txn)
             if (r->new) {
                 ovsdb_row_destroy(r->new);
             }
+            ovs_assert(!r->diff);
             hmap_remove(&t->txn_rows, &r->hmap_node);
             free(r);
         }
@@ -1439,7 +1450,8 @@  ovsdb_txn_create_txn_table(struct ovsdb_txn *txn, struct ovsdb_table *table)
 
 static struct ovsdb_txn_row *
 ovsdb_txn_row_create(struct ovsdb_txn *txn, struct ovsdb_table *table,
-                     const struct ovsdb_row *old_, struct ovsdb_row *new)
+                     const struct ovsdb_row *old_, struct ovsdb_row *new,
+                     struct ovsdb_row *diff)
 {
     const struct ovsdb_row *row = old_ ? old_ : new;
     struct ovsdb_row *old = CONST_CAST(struct ovsdb_row *, old_);
@@ -1453,6 +1465,7 @@  ovsdb_txn_row_create(struct ovsdb_txn *txn, struct ovsdb_table *table,
     txn_row->table = row->table;
     txn_row->old = old;
     txn_row->new = new;
+    txn_row->diff = diff;
     txn_row->n_refs = old ? old->n_refs : 0;
     txn_row->serial = serial - 1;
 
@@ -1465,6 +1478,9 @@  ovsdb_txn_row_create(struct ovsdb_txn *txn, struct ovsdb_table *table,
     if (new) {
         new->txn_row = txn_row;
     }
+    if (diff) {
+        diff->txn_row = txn_row;
+    }
 
     txn_table = ovsdb_txn_create_txn_table(txn, table);
     hmap_insert(&txn_table->txn_rows, &txn_row->hmap_node,
@@ -1473,24 +1489,38 @@  ovsdb_txn_row_create(struct ovsdb_txn *txn, struct ovsdb_table *table,
     return txn_row;
 }
 
-struct ovsdb_row *
-ovsdb_txn_row_modify(struct ovsdb_txn *txn, const struct ovsdb_row *ro_row_)
+void
+ovsdb_txn_row_modify(struct ovsdb_txn *txn, const struct ovsdb_row *ro_row_,
+                     struct ovsdb_row **rw_row, struct ovsdb_row **diff)
 {
     struct ovsdb_row *ro_row = CONST_CAST(struct ovsdb_row *, ro_row_);
 
+    ovs_assert(ro_row);
+    ovs_assert(rw_row);
+
     if (ro_row->txn_row) {
         ovs_assert(ro_row == ro_row->txn_row->new);
-        return ro_row;
+        *rw_row = ro_row;
+        if (diff) {
+            *diff = ro_row->txn_row->diff;
+        } else {
+            /* Caller will modify the row without updating the diff.
+             * Destroy the existing diff, if any, so it will not be
+             * used for this row anymore.  Modification will always
+             * return NULL from this point on. */
+            ovsdb_row_destroy(ro_row->txn_row->diff);
+            ro_row->txn_row->diff = NULL;
+        }
     } else {
         struct ovsdb_table *table = ro_row->table;
-        struct ovsdb_row *rw_row;
 
-        rw_row = ovsdb_row_clone(ro_row);
-        rw_row->n_refs = ro_row->n_refs;
-        ovsdb_txn_row_create(txn, table, ro_row, rw_row);
-        hmap_replace(&table->rows, &ro_row->hmap_node, &rw_row->hmap_node);
-
-        return rw_row;
+        *rw_row = ovsdb_row_clone(ro_row);
+        (*rw_row)->n_refs = ro_row->n_refs;
+        if (diff) {
+            *diff = ovsdb_row_create(table);
+        }
+        ovsdb_txn_row_create(txn, table, ro_row, *rw_row, diff ? *diff : NULL);
+        hmap_replace(&table->rows, &ro_row->hmap_node, &(*rw_row)->hmap_node);
     }
 }
 
@@ -1502,7 +1532,7 @@  ovsdb_txn_row_insert(struct ovsdb_txn *txn, struct ovsdb_row *row)
 
     uuid_generate(ovsdb_row_get_version_rw(row));
 
-    ovsdb_txn_row_create(txn, table, NULL, row);
+    ovsdb_txn_row_create(txn, table, NULL, row, NULL);
     hmap_insert(&table->rows, &row->hmap_node, hash);
 }
 
@@ -1518,9 +1548,11 @@  ovsdb_txn_row_delete(struct ovsdb_txn *txn, const struct ovsdb_row *row_)
     hmap_remove(&table->rows, &row->hmap_node);
 
     if (!txn_row) {
-        ovsdb_txn_row_create(txn, table, row, NULL);
+        ovsdb_txn_row_create(txn, table, row, NULL, NULL);
     } else {
         ovs_assert(txn_row->new == row);
+        ovsdb_row_destroy(txn_row->diff);
+        txn_row->diff = NULL;
         if (txn_row->old) {
             txn_row->new = NULL;
         } else {
diff --git a/ovsdb/transaction.h b/ovsdb/transaction.h
index 0e054eef3..f659838dc 100644
--- a/ovsdb/transaction.h
+++ b/ovsdb/transaction.h
@@ -21,6 +21,7 @@ 
 
 struct json;
 struct ovsdb;
+struct ovsdb_row;
 struct ovsdb_schema;
 struct ovsdb_table;
 struct uuid;
@@ -50,8 +51,9 @@  const struct ovsdb_error *ovsdb_txn_progress_get_error(
     const struct ovsdb_txn_progress *);
 void ovsdb_txn_progress_destroy(struct ovsdb_txn_progress *);
 
-struct ovsdb_row *ovsdb_txn_row_modify(struct ovsdb_txn *,
-                                       const struct ovsdb_row *);
+void ovsdb_txn_row_modify(struct ovsdb_txn *, const struct ovsdb_row *,
+                          struct ovsdb_row **row_new,
+                          struct ovsdb_row **row_diff);
 void ovsdb_txn_row_insert(struct ovsdb_txn *, struct ovsdb_row *);
 void ovsdb_txn_row_delete(struct ovsdb_txn *, const struct ovsdb_row *);
 
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index c761822e6..0394eafd2 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -1798,7 +1798,7 @@  do_transact_modify(struct ovs_cmdl_context *ctx)
     struct ovsdb_row *row_rw;
 
     row_ro = do_transact_find_row(ctx->argv[1]);
-    row_rw = ovsdb_txn_row_modify(do_transact_txn, row_ro);
+    ovsdb_txn_row_modify(do_transact_txn, row_ro, &row_rw, NULL);
     do_transact_set_i_j(row_rw, ctx->argv[2], ctx->argv[3]);
 }