ovsdb: transaction: Fix logging order of duplicate index rows.

The message about conflicting rows is trying to order the two rows
in a consistent manner, so the log messages do not change in tests.
But it fails to do so, because the order of columns in the column
set depends on the order of columns inside the hash map, which
depends on the hash function and the internal implementation details
of the hash map.  This results in random test failures, when two
rows end up in the opposite order.

Uncovered while testing a different hash map implementation, but the
failure is technically possible even without any changes in the code,
e.g., by running on a different CPU architecture or with different
compiler flags.

Fix that by introducing a new function that constructs the column
set with columns in a predictable order and without UUID columns that
have random values in most cases and so not actually comparable.

Fixes: 6910a6e ("ovsdb: Implement table uniqueness constraints ("indexes").")
Reported-by: Rosemarie O'Riorden <[email protected]>
Acked-by: Eelco Chaudron [email protected]
Acked-by: Dumitru Ceara <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>

Author: igsilya

ovsdb/column.c

@@ -254,6 +254,25 @@ ovsdb_column_set_add_all(struct ovsdb_column_set *set,
     }
 }
 
+void
+ovsdb_column_set_add_all_comparable(struct ovsdb_column_set *set,
+                                    const struct ovsdb_table *table)
+{
+    const struct shash_node **nodes = shash_sort(&table->schema->columns);
+    size_t i, n = shash_count(&table->schema->columns);
+
+    for (i = 0; i < n; i++) {
+        const struct ovsdb_column *column = nodes[i]->data;
+
+        /* UUIDs would cause a random order if used for comparison. */
+        if (column->type.key.type != OVSDB_TYPE_UUID
+            && column->type.value.type != OVSDB_TYPE_UUID) {
+            ovsdb_column_set_add(set, column);
+        }
+    }
+    free(nodes);
+}
+
 bool
 ovsdb_column_set_contains(const struct ovsdb_column_set *set,
                           unsigned int column_index)

ovsdb/column.h

@@ -79,6 +79,8 @@ void ovsdb_column_set_add(struct ovsdb_column_set *,
                           const struct ovsdb_column *);
 void ovsdb_column_set_add_all(struct ovsdb_column_set *,
                               const struct ovsdb_table *);
+void ovsdb_column_set_add_all_comparable(struct ovsdb_column_set *,
+                                         const struct ovsdb_table *);
 bool ovsdb_column_set_contains(const struct ovsdb_column_set *,
                                unsigned int column_index);
 bool ovsdb_column_set_equals(const struct ovsdb_column_set *,

ovsdb/transaction.c

@@ -950,7 +950,7 @@ duplicate_index_row(const struct ovsdb_column_set *index,
     /* Put 'a' and 'b' in a predictable order to make error messages
      * reproducible for testing. */
     ovsdb_column_set_init(&all_columns);
-    ovsdb_column_set_add_all(&all_columns, a->table);
+    ovsdb_column_set_add_all_comparable(&all_columns, a->table);
     if (ovsdb_row_compare_columns_3way(a, b, &all_columns) < 0) {
         const struct ovsdb_row *tmp = a;
         a = b;