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: 6910a6e6f25f ("ovsdb: Implement table uniqueness constraints
("indexes").")
Reported-by: Rosemarie O'Riorden <[email protected]>
Signed-off-by: Ilya Maximets <[email protected]>
---
ovsdb/column.c | 19 +++++++++++++++++++
ovsdb/column.h | 2 ++
ovsdb/transaction.c | 2 +-
3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/ovsdb/column.c b/ovsdb/column.c
index f4dc5bea6..a261cb61a 100644
--- a/ovsdb/column.c
+++ b/ovsdb/column.c
@@ -257,6 +257,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)
diff --git a/ovsdb/column.h b/ovsdb/column.h
index f75a1076d..bae62567f 100644
--- a/ovsdb/column.h
+++ b/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 *,
diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c
index d16f4daa3..0d0d27b61 100644
--- a/ovsdb/transaction.c
+++ b/ovsdb/transaction.c
@@ -924,7 +924,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;
--
2.51.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
