diff mbox series

[ovs-dev] osvdb: Add some helpful comments.

Message ID 20181101162907.19059-1-blp@ovn.org
State Accepted
Headers show
Series [ovs-dev] osvdb: Add some helpful comments. | expand

Commit Message

Ben Pfaff Nov. 1, 2018, 4:29 p.m. UTC
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 ovsdb/row.h         |  2 +-
 ovsdb/transaction.c | 29 +++++++++++++++++++----------
 2 files changed, 20 insertions(+), 11 deletions(-)

Comments

Justin Pettit Dec. 1, 2018, 7:27 p.m. UTC | #1
> On Nov 1, 2018, at 9:29 AM, Ben Pfaff <blp@ovn.org> wrote:
> 
> Signed-off-by: Ben Pfaff <blp@ovn.org>

Acked-by: Justin Pettit <jpettit@ovn.org>

--Justin
Ben Pfaff Dec. 3, 2018, 8:58 p.m. UTC | #2
On Sat, Dec 01, 2018 at 11:27:55AM -0800, Justin Pettit wrote:
> 
> > On Nov 1, 2018, at 9:29 AM, Ben Pfaff <blp@ovn.org> wrote:
> > 
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> 
> Acked-by: Justin Pettit <jpettit@ovn.org>

Thanks, applied to master.
diff mbox series

Patch

diff --git a/ovsdb/row.h b/ovsdb/row.h
index 5f0b8a7baf29..2c441b5a4b3b 100644
--- a/ovsdb/row.h
+++ b/ovsdb/row.h
@@ -49,7 +49,7 @@  struct ovsdb_row {
     struct ovsdb_table *table;     /* Table to which this belongs. */
     struct ovsdb_txn_row *txn_row; /* Transaction that row is in, if any. */
 
-    /* Weak references. */
+    /* Weak references.  Updated and checked only at transaction commit. */
     struct ovs_list src_refs;   /* Weak references from this row. */
     struct ovs_list dst_refs;   /* Weak references to this row. */
 
diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
index 9801aca2b08f..5a43132e4ab8 100644
--- a/ovsdb/transaction.c
+++ b/ovsdb/transaction.c
@@ -763,31 +763,40 @@  static struct ovsdb_error * OVS_WARN_UNUSED_RESULT
 check_index_uniqueness(struct ovsdb_txn *txn OVS_UNUSED,
                        struct ovsdb_txn_row *txn_row)
 {
-    struct ovsdb_txn_table *txn_table = txn_row->table->txn_table;
-    struct ovsdb_table *table = txn_row->table;
+    /* Skip rows that are being deleted since they can't violate uniqueness. */
     struct ovsdb_row *row = txn_row->new;
-    size_t i;
-
     if (!row) {
         return NULL;
     }
 
-    for (i = 0; i < table->schema->n_indexes; i++) {
+    struct ovsdb_txn_table *txn_table = txn_row->table->txn_table;
+    struct ovsdb_table *table = txn_row->table;
+    for (size_t i = 0; i < table->schema->n_indexes; i++) {
         const struct ovsdb_column_set *index = &table->schema->indexes[i];
-        struct ovsdb_row *irow;
-        uint32_t hash;
-
-        hash = ovsdb_row_hash_columns(row, index, 0);
-        irow = ovsdb_index_search(&txn_table->txn_indexes[i], row, i, hash);
+        uint32_t hash = ovsdb_row_hash_columns(row, index, 0);
+
+        /* Check whether the row has a match in the temporary hash table that
+         * we're building.  If we add two rows with the same index data, then
+         * there's a duplicate within the rows added or modified in this
+         * transaction.*/
+        struct ovsdb_row *irow
+            = ovsdb_index_search(&txn_table->txn_indexes[i], row, i, hash);
         if (irow) {
             return duplicate_index_row(index, irow, row);
         }
 
+        /* Also check whether the row has a match in the table's real index
+         * (which won't be updated until transaction commit is certain).  If
+         * there's a match, and it's for a row that wasn't pulled into the
+         * transaction, then it's a duplicate.  (If it is for a row that is
+         * part of the transaction, then the first check has already handled
+         * it.) */
         irow = ovsdb_index_search(&table->indexes[i], row, i, hash);
         if (irow && !irow->txn_row) {
             return duplicate_index_row(index, irow, row);
         }
 
+        /* Add row to temporary hash table. */
         hmap_insert(&txn_table->txn_indexes[i],
                     ovsdb_row_get_index_node(row, i), hash);
     }