diff --git a/include/openvswitch/list.h b/include/openvswitch/list.h index 5c2cca49c..ea5b3dbbb 100644 --- a/include/openvswitch/list.h +++ b/include/openvswitch/list.h @@ -288,4 +288,11 @@ ovs_list_is_short(const struct ovs_list *list) return list->next == list->prev; } +/* Transplant a list into another, and resets the origin list */ +static inline void +ovs_list_push_back_all(struct ovs_list *dst, struct ovs_list *src) +{ + ovs_list_splice(dst, src->next, src); +} + #endif /* list.h */ diff --git a/ovsdb/row.h b/ovsdb/row.h index 83024bc0f..5f0b8a7ba 100644 --- a/ovsdb/row.h +++ b/ovsdb/row.h @@ -36,9 +36,11 @@ struct ovsdb_column_set; * ovsdb_weak_ref" structures are created for them. */ struct ovsdb_weak_ref { - struct ovs_list src_node; /* In src->src_refs list. */ - struct ovs_list dst_node; /* In destination row's dst_refs list. */ - struct ovsdb_row *src; /* Source row. */ + struct ovs_list src_node; /* In src->src_refs list. */ + struct ovs_list dst_node; /* In destination row's dst_refs list. */ + struct ovsdb_row *src; /* Source row. */ + struct ovsdb_table *dst_table; /* Destination table. */ + struct uuid dst; /* Destination row uuid. */ }; /* A row in a database table. */ diff --git a/ovsdb/transaction.c b/ovsdb/transaction.c index 865e9b6b2..2e315804a 100644 --- a/ovsdb/transaction.c +++ b/ovsdb/transaction.c @@ -436,9 +436,48 @@ ovsdb_txn_row_commit(struct ovsdb_txn *txn OVS_UNUSED, return NULL; } +static struct ovsdb_error * +ovsdb_txn_update_weak_refs(struct ovsdb_txn *txn OVS_UNUSED, + struct ovsdb_txn_row *txn_row) +{ + struct ovsdb_weak_ref *weak, *next; + + /* Remove the weak references originating in the old version of the row. */ + if (txn_row->old) { + LIST_FOR_EACH_SAFE (weak, next, src_node, &txn_row->old->src_refs) { + ovs_list_remove(&weak->src_node); + ovs_list_remove(&weak->dst_node); + free(weak); + } + } + + /* Although the originating rows have the responsibility of updating the + * weak references in the dst, it is possible that some source rows aren't + * part of the transaction. In that situation this row needs to move the + * list of incoming weak references from the old row into the new one. + */ + if (txn_row->old && txn_row->new) { + /* Move the incoming weak references from old to new. */ + ovs_list_push_back_all(&txn_row->new->dst_refs, + &txn_row->old->dst_refs); + } + + /* Insert the weak references originating in the new version of the row. */ + struct ovsdb_row *dst_row; + if (txn_row->new) { + LIST_FOR_EACH (weak, src_node, &txn_row->new->src_refs) { + /* dst_row MUST exist. */ + dst_row = CONST_CAST(struct ovsdb_row *, + ovsdb_table_get_row(weak->dst_table, &weak->dst)); + ovs_list_insert(&dst_row->dst_refs, &weak->dst_node); + } + } + + return NULL; +} + static void -add_weak_ref(struct ovsdb_txn *txn, - const struct ovsdb_row *src_, const struct ovsdb_row *dst_) +add_weak_ref(const struct ovsdb_row *src_, const struct ovsdb_row *dst_) { struct ovsdb_row *src = CONST_CAST(struct ovsdb_row *, src_); struct ovsdb_row *dst = CONST_CAST(struct ovsdb_row *, dst_); @@ -448,8 +487,6 @@ add_weak_ref(struct ovsdb_txn *txn, return; } - dst = ovsdb_txn_row_modify(txn, dst); - if (!ovs_list_is_empty(&dst->dst_refs)) { /* Omit duplicates. */ weak = CONTAINER_OF(ovs_list_back(&dst->dst_refs), @@ -461,7 +498,10 @@ add_weak_ref(struct ovsdb_txn *txn, weak = xmalloc(sizeof *weak); weak->src = src; - ovs_list_push_back(&dst->dst_refs, &weak->dst_node); + weak->dst_table = dst->table; + weak->dst = *ovsdb_row_get_uuid(dst); + /* The dst_refs list is updated at commit time. */ + ovs_list_init(&weak->dst_node); ovs_list_push_back(&src->src_refs, &weak->src_node); } @@ -471,7 +511,7 @@ assess_weak_refs(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row) struct ovsdb_table *table; struct shash_node *node; - if (txn_row->old) { + if (txn_row->old && !txn_row->new) { /* Mark rows that have weak references to 'txn_row' as modified, so * that their weak references will get reassessed. */ struct ovsdb_weak_ref *weak, *next; @@ -506,7 +546,7 @@ assess_weak_refs(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row) row = ovsdb_table_get_row(column->type.key.u.uuid.refTable, &datum->keys[i].uuid); if (row) { - add_weak_ref(txn, txn_row->new, row); + add_weak_ref(txn_row->new, row); i++; } else { if (uuid_is_zero(&datum->keys[i].uuid)) { @@ -524,7 +564,7 @@ assess_weak_refs(struct ovsdb_txn *txn, struct ovsdb_txn_row *txn_row) row = ovsdb_table_get_row(column->type.value.u.uuid.refTable, &datum->values[i].uuid); if (row) { - add_weak_ref(txn, txn_row->new, row); + add_weak_ref(txn_row->new, row); i++; } else { if (uuid_is_zero(&datum->values[i].uuid)) { @@ -838,6 +878,7 @@ ovsdb_txn_commit_(struct ovsdb_txn *txn, bool durable) /* Finalize commit. */ txn->db->run_triggers = true; + ovsdb_error_assert(for_each_txn_row(txn, ovsdb_txn_update_weak_refs)); ovsdb_error_assert(for_each_txn_row(txn, ovsdb_txn_row_commit)); ovsdb_txn_free(txn); diff --git a/tests/library.at b/tests/library.at index e5095a0c1..bbf1e9d39 100644 --- a/tests/library.at +++ b/tests/library.at @@ -43,8 +43,8 @@ AT_SETUP([atomic operations]) AT_CHECK([ovstest test-atomic]) AT_CLEANUP -AT_SETUP([linked lists]) -AT_CHECK([ovstest test-list], [0], [... +AT_SETUP([test linked lists]) +AT_CHECK([ovstest test-list], [0], [.... ]) AT_CLEANUP diff --git a/tests/test-list.c b/tests/test-list.c index d413f30a2..6f1fb059b 100644 --- a/tests/test-list.c +++ b/tests/test-list.c @@ -185,6 +185,46 @@ test_list_for_each_pop(void) } } +/* Tests the transplant of one list into another */ +static void +test_list_push_back_all(void) +{ + struct ovs_list list_a, list_b; + struct element a, b, c, d; + + a.value = 0; + b.value = 1; + c.value = 2; + d.value = 3; + + ovs_list_init(&list_a); + ovs_list_init(&list_b); + + ovs_list_insert(&list_a, &a.node); + ovs_list_insert(&list_a, &b.node); + ovs_list_insert(&list_b, &c.node); + ovs_list_insert(&list_b, &d.node); + + /* Check test preconditions */ + assert(2 == ovs_list_size(&list_a)); + assert(2 == ovs_list_size(&list_b)); + + /* Perform transplant */ + ovs_list_push_back_all(&list_a, &list_b); + + /* Check expected result */ + assert(4 == ovs_list_size(&list_a)); + assert(0 == ovs_list_size(&list_b)); + + struct element *node; + int n = 0; + LIST_FOR_EACH(node, node, &list_a) { + assert(n == node->value); + n++; + } + assert(n == 4); +} + static void run_test(void (*function)(void)) { @@ -198,6 +238,7 @@ test_list_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) run_test(test_list_construction); run_test(test_list_for_each_safe); run_test(test_list_for_each_pop); + run_test(test_list_push_back_all); printf("\n"); }