Considering two DB rows, 'a' from table A and 'b' from table B (with column 'ref_a' a reference to table A): a = {A._uuid=<U1>} b = {B._uuid=<U2>, B.ref_a=<U1>}
Assuming both records are present in the IDL client's in-memory view of the database, depending whether row 'b' is also deleted in the same transaction or not, deletion of row 'a' should generate the following tracked changes: 1. only row 'a' is deleted: - for table A: - deleted records: a = {A._uuid=<U1>} - for table B: - updated records: b = {B._uuid=<U2>, B.ref_a=[]} 2. row 'a' and row 'b' are deleted in the same update: - for table A: - deleted records: a = {A._uuid=<U1>} - for table B: - deleted records: b = {B._uuid=<U2>, B.ref_a=<U1>} To ensure this, we now delay reparsing row backrefs for deleted rows until all updates in the current run have been processed. Without this change, in scenario 2 above, the tracked changes for table B would be: - deleted records: b = {B._uuid=<U2>, B.ref_a=[]} In particular, for strong references, row 'a' can never be deleted in a transaction that happens strictly before row 'b' is deleted. In some cases [0] both rows are deleted in the same transaction and having B.ref_a=[] would violate the integrity of the database from client perspective. This would force the client to always validate that strong reference fields are non-NULL. This is not really an option because the information in the original reference is required for incrementally processing the record deletion. [0] with ovn-monitor-all=true, the following command triggers a crash in ovn-controller because a strong reference field becomes NULL: $ ovn-nbctl --wait=hv -- lr-add r -- lrp-add r rp 00:00:00:00:00:01 1.0.0.1/24 $ ovn-nbctl lr-del r Reported-at: https://bugzilla.redhat.com/1932642 Fixes: 72aeb243a52a ("ovsdb-idl: Tracking - preserve data for deleted rows.") Signed-off-by: Dumitru Ceara <dce...@redhat.com> --- v4: - Rename orphan_rows to deleted_untracked_rows. - Rename ovsdb_idl_process_orphans() to ovsdb_idl_reparse_deleted(). - Revert changes to ovsdb_idl_row_reparse_backrefs(). - Unified test-ovsdb.c and test-ovsdb.py output for simple3's uset and uref columns. - Added two more tests for deletion of strong references due to monitor condition change. --- lib/ovsdb-idl.c | 131 ++++++++++++++++----- tests/ovsdb-idl.at | 312 ++++++++++++++++++++++++++++++++++++++++++++++++--- tests/test-ovsdb.c | 64 ++++++++++ tests/test-ovsdb.py | 28 ++++- 4 files changed, 481 insertions(+), 54 deletions(-) diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 9e1e7876e7..9043de0fab 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -92,6 +92,9 @@ struct ovsdb_idl { struct ovsdb_idl_txn *txn; struct hmap outstanding_txns; bool verify_write_only; + struct ovs_list deleted_untracked_rows; /* Stores rows deleted in the + * current run, that are not yet + * added to the track_list. */ }; static struct ovsdb_cs_ops ovsdb_idl_cs_ops; @@ -144,6 +147,7 @@ static bool ovsdb_idl_modify_row(struct ovsdb_idl_row *, const struct shash *values, bool xor); static void ovsdb_idl_parse_update(struct ovsdb_idl *, const struct ovsdb_cs_update_event *); +static void ovsdb_idl_reparse_deleted(struct ovsdb_idl *); static void ovsdb_idl_txn_process_reply(struct ovsdb_idl *, const struct jsonrpc_msg *); @@ -163,6 +167,10 @@ static void ovsdb_idl_row_unparse(struct ovsdb_idl_row *); static void ovsdb_idl_row_clear_old(struct ovsdb_idl_row *); static void ovsdb_idl_row_clear_new(struct ovsdb_idl_row *); static void ovsdb_idl_row_clear_arcs(struct ovsdb_idl_row *, bool destroy_dsts); +static void ovsdb_idl_row_reparse_backrefs(struct ovsdb_idl_row *); +static void ovsdb_idl_row_track_change(struct ovsdb_idl_row *, + enum ovsdb_idl_change); +static void ovsdb_idl_row_untrack_change(struct ovsdb_idl_row *); static void ovsdb_idl_txn_abort_all(struct ovsdb_idl *); static bool ovsdb_idl_txn_extract_mutations(struct ovsdb_idl_row *, @@ -248,6 +256,8 @@ ovsdb_idl_create_unconnected(const struct ovsdb_idl_class *class, .txn = NULL, .outstanding_txns = HMAP_INITIALIZER(&idl->outstanding_txns), .verify_write_only = false, + .deleted_untracked_rows + = OVS_LIST_INITIALIZER(&idl->deleted_untracked_rows), }; uint8_t default_mode = (monitor_everything_by_default @@ -351,6 +361,14 @@ ovsdb_idl_set_leader_only(struct ovsdb_idl *idl, bool leader_only) static void ovsdb_idl_clear(struct ovsdb_idl *db) { + /* Process deleted rows, removing them from the 'deleted_untracked_rows' + * list and reparsing their backrefs. + */ + ovsdb_idl_reparse_deleted(db); + + /* Cleanup all rows; each row gets added to its own table's + * 'track_list'. + */ for (size_t i = 0; i < db->class_->n_tables; i++) { struct ovsdb_idl_table *table = &db->tables[i]; struct ovsdb_idl_row *row, *next_row; @@ -367,17 +385,26 @@ ovsdb_idl_clear(struct ovsdb_idl *db) ovsdb_idl_row_unparse(row); } LIST_FOR_EACH_SAFE (arc, next_arc, src_node, &row->src_arcs) { + ovs_list_remove(&arc->src_node); + ovs_list_remove(&arc->dst_node); + free(arc); + } + LIST_FOR_EACH_SAFE (arc, next_arc, dst_node, &row->dst_arcs) { + ovs_list_remove(&arc->src_node); + ovs_list_remove(&arc->dst_node); free(arc); } - /* No need to do anything with dst_arcs: some node has those arcs - * as forward arcs and will destroy them itself. */ ovsdb_idl_row_destroy(row); } } + + /* Free rows deleted from tables with change tracking disabled. */ ovsdb_idl_row_destroy_postprocess(db); + /* Free rows deleted from tables with change tracking enabled. */ ovsdb_idl_track_clear__(db, true); + ovs_assert(ovs_list_is_empty(&db->deleted_untracked_rows)); db->change_seqno++; } @@ -415,7 +442,7 @@ ovsdb_idl_run(struct ovsdb_idl *idl) } ovsdb_cs_event_destroy(event); } - + ovsdb_idl_reparse_deleted(idl); ovsdb_idl_row_destroy_postprocess(idl); } @@ -1226,13 +1253,8 @@ ovsdb_idl_track_clear__(struct ovsdb_idl *idl, bool flush_all) free(row->updated); row->updated = NULL; } + ovsdb_idl_row_untrack_change(row); - row->change_seqno[OVSDB_IDL_CHANGE_INSERT] = - row->change_seqno[OVSDB_IDL_CHANGE_MODIFY] = - row->change_seqno[OVSDB_IDL_CHANGE_DELETE] = 0; - - ovs_list_remove(&row->track_node); - ovs_list_init(&row->track_node); if (ovsdb_idl_row_is_orphan(row)) { ovsdb_idl_row_unparse(row); if (row->tracked_old_datum) { @@ -1350,6 +1372,32 @@ ovsdb_idl_parse_update(struct ovsdb_idl *idl, } } +/* Reparses references to rows that have been deleted in the current IDL run. + * + * To ensure that reference sources that are deleted are not reparsed, + * this function must be called after all updates have been processed in + * the current IDL run, i.e., after all calls to ovsdb_idl_parse_update(). + */ +static void +ovsdb_idl_reparse_deleted(struct ovsdb_idl *db) +{ + struct ovsdb_idl_row *row, *next; + + LIST_FOR_EACH_SAFE (row, next, track_node, &db->deleted_untracked_rows) { + ovsdb_idl_row_untrack_change(row); + ovsdb_idl_row_reparse_backrefs(row); + + /* Orphan rows that are still unreferenced or are part of tables that + * have hange tracking enabled should be added to their table's + * 'track_list'. + */ + if (ovs_list_is_empty(&row->dst_arcs) + || ovsdb_idl_track_is_set(row->table)) { + ovsdb_idl_row_track_change(row, OVSDB_IDL_CHANGE_DELETE); + } + } +} + static struct ovsdb_idl_row * ovsdb_idl_get_row(struct ovsdb_idl_table *table, const struct uuid *uuid) { @@ -1403,6 +1451,7 @@ ovsdb_idl_process_update(struct ovsdb_idl_table *table, ovsdb_idl_insert_row(ovsdb_idl_row_create(table, uuid), ru->columns); } else if (ovsdb_idl_row_is_orphan(row)) { + ovsdb_idl_row_untrack_change(row); ovsdb_idl_insert_row(row, ru->columns); } else { VLOG_ERR_RL(&semantic_rl, "cannot add existing row "UUID_FMT" to " @@ -1450,13 +1499,8 @@ add_tracked_change_for_references(struct ovsdb_idl_row *row) if (ovs_list_is_empty(&ref->track_node) && ovsdb_idl_track_is_set(ref->table)) { - ovs_list_push_back(&ref->table->track_list, - &ref->track_node); - - ref->change_seqno[OVSDB_IDL_CHANGE_MODIFY] - = ref->table->change_seqno[OVSDB_IDL_CHANGE_MODIFY] - = ref->table->idl->change_seqno + 1; + ovsdb_idl_row_track_change(ref, OVSDB_IDL_CHANGE_MODIFY); add_tracked_change_for_references(ref); } } @@ -2022,6 +2066,32 @@ ovsdb_idl_row_reparse_backrefs(struct ovsdb_idl_row *row) } } +static void +ovsdb_idl_row_track_change(struct ovsdb_idl_row *row, + enum ovsdb_idl_change change) +{ + row->change_seqno[change] + = row->table->change_seqno[change] + = row->table->idl->change_seqno + 1; + if (ovs_list_is_empty(&row->track_node)) { + ovs_list_push_back(&row->table->track_list, &row->track_node); + } +} + +static void +ovsdb_idl_row_untrack_change(struct ovsdb_idl_row *row) +{ + if (ovs_list_is_empty(&row->track_node)) { + return; + } + + row->change_seqno[OVSDB_IDL_CHANGE_INSERT] = + row->change_seqno[OVSDB_IDL_CHANGE_MODIFY] = + row->change_seqno[OVSDB_IDL_CHANGE_DELETE] = 0; + ovs_list_remove(&row->track_node); + ovs_list_init(&row->track_node); +} + static struct ovsdb_idl_row * ovsdb_idl_row_create__(const struct ovsdb_idl_table_class *class) { @@ -2048,22 +2118,26 @@ ovsdb_idl_row_create(struct ovsdb_idl_table *table, const struct uuid *uuid) return row; } +/* If 'row' is not referenced anymore, removes 'row' from the table hmap, + * clears the old datum and adds 'row' to the table's track_list. + * + * If 'row' is still referenced, i.e., became "orphan", queues 'row' for + * reparsing after all updates have been processed by adding it to the + * 'deleted_untracked_rows' list. + */ static void ovsdb_idl_row_destroy(struct ovsdb_idl_row *row) { - if (row) { - ovsdb_idl_row_clear_old(row); + ovsdb_idl_row_clear_old(row); + if (ovs_list_is_empty(&row->dst_arcs)) { hmap_remove(&row->table->rows, &row->hmap_node); ovsdb_idl_destroy_all_map_op_lists(row); ovsdb_idl_destroy_all_set_op_lists(row); - if (ovsdb_idl_track_is_set(row->table)) { - row->change_seqno[OVSDB_IDL_CHANGE_DELETE] - = row->table->change_seqno[OVSDB_IDL_CHANGE_DELETE] - = row->table->idl->change_seqno + 1; - } - if (ovs_list_is_empty(&row->track_node)) { - ovs_list_push_back(&row->table->track_list, &row->track_node); - } + ovsdb_idl_row_track_change(row, OVSDB_IDL_CHANGE_DELETE); + } else { + ovsdb_idl_row_untrack_change(row); + ovs_list_push_back(&row->table->idl->deleted_untracked_rows, + &row->track_node); } } @@ -2153,12 +2227,7 @@ ovsdb_idl_delete_row(struct ovsdb_idl_row *row) { ovsdb_idl_remove_from_indexes(row); ovsdb_idl_row_clear_arcs(row, true); - ovsdb_idl_row_clear_old(row); - if (ovs_list_is_empty(&row->dst_arcs)) { - ovsdb_idl_row_destroy(row); - } else { - ovsdb_idl_row_reparse_backrefs(row); - } + ovsdb_idl_row_destroy(row); } /* Returns true if a column with mode OVSDB_IDL_MODE_RW changed, false diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at index f96cc17d46..6f09257738 100644 --- a/tests/ovsdb-idl.at +++ b/tests/ovsdb-idl.at @@ -141,7 +141,7 @@ m4_define([OVSDB_CHECK_IDL_REGISTER_COLUMNS_PY], AT_CHECK([ovsdb_start_idltest]) m4_if([$2], [], [], [AT_CHECK([ovsdb-client transact unix:socket $2], [0], [ignore], [ignore])]) - AT_CHECK([$PYTHON3 $srcdir/test-ovsdb.py -t10 idl $srcdir/idltest.ovsschema unix:socket ?simple:b,ba,i,ia,r,ra,s,sa,u,ua?link1:i,k,ka,l2?link2:i,l1?singleton:name $3], + AT_CHECK([$PYTHON3 $srcdir/test-ovsdb.py -t10 idl $srcdir/idltest.ovsschema unix:socket ?simple:b,ba,i,ia,r,ra,s,sa,u,ua?simple3:name,uset,uref?simple4:name?simple6:name,weak_ref?link1:i,k,ka,l2?link2:i,l1?singleton:name $3], [0], [stdout], [ignore]) AT_CHECK([sort stdout | uuidfilt]m4_if([$6],,, [[| $6]]), [0], [$4]) @@ -874,6 +874,35 @@ OVSDB_CHECK_IDL([singleton idl, constraints], 006: done ]]) +dnl This test creates a database with references and checks that deleting both +dnl source and destination rows of a reference in a single update doesn't leak +dnl rows that got orphaned when processing the update. +OVSDB_CHECK_IDL([simple idl, references, multiple deletes], + [['["idltest", + {"op": "insert", + "table": "simple", + "row": {"s": "row0_s"}, + "uuid-name": "weak_row0"}, + {"op": "insert", + "table": "simple6", + "row": {"name": "first_row", + "weak_ref": ["set", + [["named-uuid", "weak_row0"]] + ]}}]']], + [['["idltest", + {"op": "delete", + "table": "simple", + "where": [["s", "==", "row0_s"]]}, + {"op": "delete", + "table": "simple6", + "where": [["name", "==", "first_row"]]}]']], + [[000: table simple6: name=first_row weak_ref=[<0>] uuid=<1> +000: table simple: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<0> +001: {"error":null,"result":[{"count":1},{"count":1}]} +002: empty +003: done +]]) + OVSDB_CHECK_IDL_PY([external-linking idl, insert ops], [], [['linktest']], @@ -1257,6 +1286,7 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, orphan rows, cond 003: table simple: inserted row: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> 003: table simple: updated columns: s 004: change conditions +005: table simple: deleted row: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> 005: table simple: inserted row: i=0 r=0 b=false s=row1_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> 005: table simple: updated columns: s 006: change conditions @@ -1269,6 +1299,244 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, orphan rows, cond 010: done ]]) +dnl This test checks that deleting the destination of a weak reference +dnl without deleting the source, through monitor condition change, updates +dnl the source tracked record. +OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, references, conditional delete], + [['["idltest", + {"op": "insert", + "table": "simple", + "row": {"s": "row0_s", "i": 0}, + "uuid-name": "weak_row0"}, + {"op": "insert", + "table": "simple", + "row": {"s": "row1_s", "i": 1}, + "uuid-name": "weak_row1"}, + {"op": "insert", + "table": "simple6", + "row": {"name": "first_row", + "weak_ref": ["set", + [["named-uuid", "weak_row0"], + ["named-uuid", "weak_row1"]] + ]}}]']], + [['condition simple []' \ + 'condition simple [["s","==","row0_s"]]' \ + 'condition simple [["s","==","row1_s"]]' \ + '["idltest", + {"op": "update", + "table": "simple6", + "where": [], + "row": {"name": "new_name"}}]' \ + '["idltest", + {"op": "delete", + "table": "simple6", + "where": []}]']], + [[000: change conditions +001: table simple6: inserted row: name=first_row weak_ref=[] uuid=<0> +001: table simple6: updated columns: name weak_ref +002: change conditions +003: table simple6: name=first_row weak_ref=[<1>] uuid=<0> +003: table simple: inserted row: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> +003: table simple: updated columns: s +004: change conditions +005: table simple6: name=first_row weak_ref=[<3>] uuid=<0> +005: table simple: deleted row: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<1> +005: table simple: inserted row: i=1 r=0 b=false s=row1_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> +005: table simple: updated columns: i s +006: {"error":null,"result":[{"count":1}]} +007: table simple6: name=new_name weak_ref=[<3>] uuid=<0> +007: table simple6: updated columns: name +008: {"error":null,"result":[{"count":1}]} +009: table simple: i=1 r=0 b=false s=row1_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> +010: done +]]) + +dnl This test checks that deleting the destination of a reference updates the +dnl source tracked record. +OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, references, single delete], + [['["idltest", + {"op": "insert", + "table": "simple", + "row": {"s": "row0_s"}, + "uuid-name": "uuid_row0_s"}, + {"op": "insert", + "table": "simple6", + "row": {"name": "row0_s6", + "weak_ref": ["set", + [["named-uuid", "uuid_row0_s"]] + ]}}]']], + [['condition simple [true];simple6 [true]' \ + '["idltest", + {"op": "delete", + "table": "simple", + "where": []}]' \ + '["idltest", + {"op": "insert", + "table": "simple", + "row": {"s": "row0_s"}}]']], + [[000: change conditions +001: table simple6: inserted row: name=row0_s6 weak_ref=[<0>] uuid=<1> +001: table simple6: updated columns: name weak_ref +001: table simple: inserted row: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<0> +001: table simple: updated columns: s +002: {"error":null,"result":[{"count":1}]} +003: table simple6: name=row0_s6 weak_ref=[] uuid=<1> +003: table simple6: updated columns: weak_ref +003: table simple: deleted row: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<0> +004: {"error":null,"result":[{"uuid":["uuid","<3>"]}]} +005: table simple6: name=row0_s6 weak_ref=[] uuid=<1> +005: table simple: inserted row: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> +005: table simple: updated columns: s +006: done +]]) + +dnl This test checks that deleting both the destination and source of the +dnl reference doesn't remove the reference in the source tracked record. +OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, weak references, multiple deletes], + [['["idltest", + {"op": "insert", + "table": "simple", + "row": {"s": "row0_s"}, + "uuid-name": "uuid_row0_s"}, + {"op": "insert", + "table": "simple6", + "row": {"name": "row0_s6", + "weak_ref": ["set", + [["named-uuid", "uuid_row0_s"]] + ]}}]']], + [['condition simple [true];simple6 [true]' \ + '["idltest", + {"op": "delete", + "table": "simple", + "where": []}, + {"op": "delete", + "table": "simple6", + "where": []}]' \ + '["idltest", + {"op": "insert", + "table": "simple", + "row": {"s": "row0_s"}}]']], + [[000: change conditions +001: table simple6: inserted row: name=row0_s6 weak_ref=[<0>] uuid=<1> +001: table simple6: updated columns: name weak_ref +001: table simple: inserted row: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<0> +001: table simple: updated columns: s +002: {"error":null,"result":[{"count":1},{"count":1}]} +003: table simple6: deleted row: name=row0_s6 weak_ref=[<0>] uuid=<1> +003: table simple: deleted row: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<0> +004: {"error":null,"result":[{"uuid":["uuid","<3>"]}]} +005: table simple: inserted row: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> +005: table simple: updated columns: s +006: done +]]) + +dnl This test checks that deleting both the destination and source of the +dnl reference doesn't remove the reference in the source tracked record. +OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, strong references, multiple deletes], + [['["idltest", + {"op": "insert", + "table": "simple4", + "row": {"name": "row0_s4"}, + "uuid-name": "uuid_row0_s4"}, + {"op": "insert", + "table": "simple3", + "row": {"name": "row0_s3", + "uref": ["set", + [["named-uuid", "uuid_row0_s4"]] + ]}}]']], + [['condition simple [true];simple3 [true];simple4 [true]' \ + '["idltest", + {"op": "delete", + "table": "simple3", + "where": []}, + {"op": "delete", + "table": "simple4", + "where": []}]' \ + '["idltest", + {"op": "insert", + "table": "simple", + "row": {"s": "row0_s"}}]']], + [[000: change conditions +001: table simple3: inserted row: name=row0_s3 uset=[] uref=[<0>] uuid=<1> +001: table simple3: updated columns: name uref +001: table simple4: inserted row: name=row0_s4 uuid=<0> +001: table simple4: updated columns: name +002: {"error":null,"result":[{"count":1},{"count":1}]} +003: table simple3: deleted row: name=row0_s3 uset=[] uref=[<0>] uuid=<1> +003: table simple4: deleted row: name=row0_s4 uuid=<0> +004: {"error":null,"result":[{"uuid":["uuid","<2>"]}]} +005: table simple: inserted row: i=0 r=0 b=false s=row0_s u=<3> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<2> +005: table simple: updated columns: s +006: done +]]) + +dnl This test checks that changing conditions to not include the target of +dnl a strong reference also updates the source row when change tracking is +dnl enabled. +OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, strong references, conditional], + [['["idltest", + {"op": "insert", + "table": "simple4", + "row": {"name": "row0_s4"}, + "uuid-name": "uuid_row0_s4"}, + {"op": "insert", + "table": "simple3", + "row": {"name": "row0_s3", + "uref": ["set", + [["named-uuid", "uuid_row0_s4"]] + ]}}]']], + [['condition simple [true];simple3 [true];simple4 [true]' \ + 'condition simple4 []' \ + '["idltest", + {"op": "insert", + "table": "simple", + "row": {"s": "row0_s"}}]']], + [[000: change conditions +001: table simple3: inserted row: name=row0_s3 uset=[] uref=[<0>] uuid=<1> +001: table simple3: updated columns: name uref +001: table simple4: inserted row: name=row0_s4 uuid=<0> +001: table simple4: updated columns: name +002: change conditions +003: table simple4: deleted row: name=row0_s4 uuid=<0> +004: {"error":null,"result":[{"uuid":["uuid","<2>"]}]} +005: table simple3: name=row0_s3 uset=[] uref=[] uuid=<1> +005: table simple: inserted row: i=0 r=0 b=false s=row0_s u=<3> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<2> +005: table simple: updated columns: s +006: done +]]) + +dnl This test checks that changing conditions to not include the target of +dnl a strong reference also updates the source row when change tracking is +dnl disabled. +OVSDB_CHECK_IDL([simple idl, initially populated, strong references, conditional], + [['["idltest", + {"op": "insert", + "table": "simple4", + "row": {"name": "row0_s4"}, + "uuid-name": "uuid_row0_s4"}, + {"op": "insert", + "table": "simple3", + "row": {"name": "row0_s3", + "uref": ["set", + [["named-uuid", "uuid_row0_s4"]] + ]}}]']], + [['condition simple [true];simple3 [true];simple4 [true]' \ + 'condition simple4 []' \ + '["idltest", + {"op": "insert", + "table": "simple", + "row": {"s": "row0_s"}}]']], + [[000: change conditions +001: table simple3: name=row0_s3 uset=[] uref=[<0>] uuid=<1> +001: table simple4: name=row0_s4 uuid=<0> +002: change conditions +003: table simple3: name=row0_s3 uset=[] uref=[] uuid=<1> +004: {"error":null,"result":[{"uuid":["uuid","<2>"]}]} +005: table simple3: name=row0_s3 uset=[] uref=[] uuid=<1> +005: table simple: i=0 r=0 b=false s=row0_s u=<3> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<2> +006: done +]]) + OVSDB_CHECK_IDL_TRACK([track, simple idl, initially empty, various ops], [], [['["idltest", @@ -1403,10 +1671,10 @@ OVSDB_CHECK_IDL_PY([partial-map update set refmap idl], [['["idltest", {"op":"insert", "table":"simple3", "row":{"name":"myString1"}}, {"op":"insert", "table":"simple5", "row":{"name":"myString2"}}]']], ['partialmapmutateirefmap'], -[[000: table simple3: name=myString1 uset=[] uuid=<0> +[[000: table simple3: name=myString1 uset=[] uref=[] uuid=<0> 000: table simple5: name=myString2 irefmap=[] uuid=<1> 001: commit, status=success -002: table simple3: name=myString1 uset=[] uuid=<0> +002: table simple3: name=myString1 uset=[] uref=[] uuid=<0> 002: table simple5: name=myString2 irefmap=[(1 <0>)] uuid=<1> 003: done ]]) @@ -1430,17 +1698,17 @@ OVSDB_CHECK_IDL_PARTIAL_UPDATE_SET_COLUMN([set, simple3 idl-partial-update-set-c ], [], [[000: Getting records -001: table simple3: name=mySet1 uset=[[<0>],[<1>]] uref=[] uuid=<2> +001: table simple3: name=mySet1 uset=[<0>,<1>] uref=[] uuid=<2> 002: After rename+add new value -003: table simple3: name=String2 uset=[[<0>],[<1>],[<3>]] uref=[] uuid=<2> +003: table simple3: name=String2 uset=[<0>,<1>,<3>] uref=[] uuid=<2> 004: After add new value -005: table simple3: name=String2 uset=[[<0>],[<1>],[<3>],[<4>]] uref=[] uuid=<2> +005: table simple3: name=String2 uset=[<0>,<1>,<3>,<4>] uref=[] uuid=<2> 006: After delete value -007: table simple3: name=String2 uset=[[<0>],[<1>],[<4>]] uref=[] uuid=<2> +007: table simple3: name=String2 uset=[<0>,<1>,<4>] uref=[] uuid=<2> 008: After trying to delete a deleted value -009: table simple3: name=String2 uset=[[<0>],[<1>],[<4>]] uref=[] uuid=<2> +009: table simple3: name=String2 uset=[<0>,<1>,<4>] uref=[] uuid=<2> 010: After add to other table + set of strong ref -011: table simple3: name=String2 uset=[[<0>],[<1>],[<4>]] uref=[[<5>]] uuid=<2> +011: table simple3: name=String2 uset=[<0>,<1>,<4>] uref=[<5>] uuid=<2> 012: End test ]]) @@ -1452,22 +1720,26 @@ OVSDB_CHECK_IDL_PY([partial-set idl], "mutations": [["uset", "insert", ["set", [["uuid", "000d2f6a-76af-412f-b59d-e7bcd3e84eff"]]]]]}]'] ], ['partialrenamesetadd' 'partialduplicateadd' 'partialsetdel' 'partialsetref' 'partialsetoverrideops' 'partialsetadddelete' 'partialsetmutatenew'], -[[000: table simple3: name=mySet1 uset=[<0> <1>] uuid=<2> +[[000: table simple3: name=mySet1 uset=[<0> <1>] uref=[] uuid=<2> 001: commit, status=success -002: table simple3: name=String2 uset=[<0> <1> <3>] uuid=<2> +002: table simple3: name=String2 uset=[<0> <1> <3>] uref=[] uuid=<2> 003: commit, status=success -004: table simple3: name=String2 uset=[<0> <1> <3> <4>] uuid=<2> +004: table simple3: name=String2 uset=[<0> <1> <3> <4>] uref=[] uuid=<2> 005: commit, status=success -006: table simple3: name=String2 uset=[<0> <1> <4>] uuid=<2> +006: table simple3: name=String2 uset=[<0> <1> <4>] uref=[] uuid=<2> 007: commit, status=success -008: table simple3: name=String2 uset=[<0> <1> <4>] uuid=<2> +008: table simple3: name=String2 uset=[<0> <1> <4>] uref=[<5>] uuid=<2> +008: table simple4: name=test uuid=<5> 009: commit, status=success -010: table simple3: name=String2 uset=[<4>] uuid=<2> +010: table simple3: name=String2 uset=[<4>] uref=[<5>] uuid=<2> +010: table simple4: name=test uuid=<5> 011: commit, status=success -012: table simple3: name=String2 uset=[<5> <6>] uuid=<2> +012: table simple3: name=String2 uset=[<6> <7>] uref=[<5>] uuid=<2> +012: table simple4: name=test uuid=<5> 013: commit, status=success -014: table simple3: name=String2 uset=[<5> <6>] uuid=<2> -014: table simple3: name=String3 uset=[<7>] uuid=<8> +014: table simple3: name=String2 uset=[<6> <7>] uref=[<5>] uuid=<2> +014: table simple3: name=String3 uset=[<8>] uref=[] uuid=<9> +014: table simple4: name=test uuid=<5> 015: done ]]) @@ -1899,10 +2171,10 @@ OVSDB_CHECK_IDL_COMPOUND_INDEX_WITH_REF([set, simple3 idl-compound-index-with-re [], [], [[000: After add to other table + set of strong ref -001: table simple3: name= uset=[] uref=[[<0>]] uuid=<1> +001: table simple3: name= uset=[] uref=[<0>] uuid=<1> 002: check simple4: not empty 003: Query using index with reference -004: table simple3: name= uset=[] uref=[[<0>]] uuid=<1> +004: table simple3: name= uset=[] uref=[<0>] uuid=<1> 005: After delete 007: check simple4: empty 008: End test diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 14a8d74a2a..a886f971e7 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -1946,6 +1946,23 @@ print_idl_row_updated_simple3(const struct idltest_simple3 *s3, int step) } } +static void +print_idl_row_updated_simple4(const struct idltest_simple4 *s4, int step) +{ + struct ds updates = DS_EMPTY_INITIALIZER; + for (size_t i = 0; i < IDLTEST_SIMPLE4_N_COLUMNS; i++) { + if (idltest_simple4_is_updated(s4, i)) { + ds_put_format(&updates, " %s", idltest_simple4_columns[i].name); + } + } + if (updates.length) { + print_and_log("%03d: table %s: updated columns:%s", + step, s4->header_.table->class_->name, + ds_cstr(&updates)); + ds_destroy(&updates); + } +} + static void print_idl_row_updated_simple6(const struct idltest_simple6 *s6, int step) { @@ -2069,13 +2086,13 @@ print_idl_row_simple3(const struct idltest_simple3 *s3, int step) ds_put_format(&msg, "name=%s uset=[", s3->name); for (i = 0; i < s3->n_uset; i++) { - ds_put_format(&msg, "["UUID_FMT"]%s", + ds_put_format(&msg, UUID_FMT"%s", UUID_ARGS(&s3->uset[i]), i < s3->n_uset - 1 ? "," : ""); } ds_put_cstr(&msg, "] uref=["); for (i = 0; i < s3->n_uref; i++) { - ds_put_format(&msg, "["UUID_FMT"]%s", + ds_put_format(&msg, UUID_FMT"%s", UUID_ARGS(&s3->uref[i]->header_.uuid), i < s3->n_uref -1 ? "," : ""); } @@ -2089,6 +2106,20 @@ print_idl_row_simple3(const struct idltest_simple3 *s3, int step) print_idl_row_updated_simple3(s3, step); } +static void +print_idl_row_simple4(const struct idltest_simple4 *s4, int step) +{ + struct ds msg = DS_EMPTY_INITIALIZER; + ds_put_format(&msg, "name=%s", s4->name); + + char *row_msg = format_idl_row(&s4->header_, step, ds_cstr(&msg)); + print_and_log("%s", row_msg); + ds_destroy(&msg); + free(row_msg); + + print_idl_row_updated_simple4(s4, step); +} + static void print_idl_row_simple6(const struct idltest_simple6 *s6, int step) { @@ -2126,6 +2157,9 @@ print_idl_row_singleton(const struct idltest_singleton *sng, int step) static void print_idl(struct ovsdb_idl *idl, int step) { + const struct idltest_simple3 *s3; + const struct idltest_simple4 *s4; + const struct idltest_simple6 *s6; const struct idltest_simple *s; const struct idltest_link1 *l1; const struct idltest_link2 *l2; @@ -2144,6 +2178,18 @@ print_idl(struct ovsdb_idl *idl, int step) print_idl_row_link2(l2, step); n++; } + IDLTEST_SIMPLE3_FOR_EACH (s3, idl) { + print_idl_row_simple3(s3, step); + n++; + } + IDLTEST_SIMPLE4_FOR_EACH (s4, idl) { + print_idl_row_simple4(s4, step); + n++; + } + IDLTEST_SIMPLE6_FOR_EACH (s6, idl) { + print_idl_row_simple6(s6, step); + n++; + } IDLTEST_SINGLETON_FOR_EACH (sng, idl) { print_idl_row_singleton(sng, step); n++; @@ -2156,6 +2202,8 @@ print_idl(struct ovsdb_idl *idl, int step) static void print_idl_track(struct ovsdb_idl *idl, int step) { + const struct idltest_simple3 *s3; + const struct idltest_simple4 *s4; const struct idltest_simple6 *s6; const struct idltest_simple *s; const struct idltest_link1 *l1; @@ -2174,6 +2222,14 @@ print_idl_track(struct ovsdb_idl *idl, int step) print_idl_row_link2(l2, step); n++; } + IDLTEST_SIMPLE3_FOR_EACH_TRACKED (s3, idl) { + print_idl_row_simple3(s3, step); + n++; + } + IDLTEST_SIMPLE4_FOR_EACH_TRACKED (s4, idl) { + print_idl_row_simple4(s4, step); + n++; + } IDLTEST_SIMPLE6_FOR_EACH_TRACKED (s6, idl) { print_idl_row_simple6(s6, step); n++; @@ -2404,6 +2460,10 @@ find_table_class(const char *name) return &idltest_table_link1; } else if (!strcmp(name, "link2")) { return &idltest_table_link2; + } else if (!strcmp(name, "simple3")) { + return &idltest_table_simple3; + } else if (!strcmp(name, "simple4")) { + return &idltest_table_simple4; } else if (!strcmp(name, "simple6")) { return &idltest_table_simple6; } diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py index bc2be6acff..72a319123e 100644 --- a/tests/test-ovsdb.py +++ b/tests/test-ovsdb.py @@ -164,6 +164,8 @@ def get_simple_printable_row_string(row, columns): for k, v in value.items()) if isinstance(value, (list, tuple)): value = sorted((row_to_uuid(v) for v in value)) + elif isinstance(value, list): + value = sorted(row_to_uuid(v) for v in value) s += "%s=%s " % (column, value) s = s.strip() s = re.sub('""|,|u?\'', "", s) @@ -187,15 +189,25 @@ def get_simple2_table_printable_row(row): def get_simple3_table_printable_row(row): - simple3_columns = ["name", "uset"] + simple3_columns = ["name", "uset", "uref"] return get_simple_printable_row_string(row, simple3_columns) +def get_simple4_table_printable_row(row): + simple4_columns = ["name"] + return get_simple_printable_row_string(row, simple4_columns) + + def get_simple5_table_printable_row(row): simple5_columns = ["name", "irefmap"] return get_simple_printable_row_string(row, simple5_columns) +def get_simple6_table_printable_row(row): + simple6_columns = ["name", "weak_ref"] + return get_simple_printable_row_string(row, simple6_columns) + + def get_link1_table_printable_row(row): s = ["i=%s k=" % row.i] if hasattr(row, "k") and row.k: @@ -249,6 +261,13 @@ def print_idl(idl, step): get_simple3_table_printable_row(row)) n += 1 + if "simple4" in idl.tables: + simple4 = idl.tables["simple4"].rows + for row in simple4.values(): + print_row("simple4", row, step, + get_simple4_table_printable_row(row)) + n += 1 + if "simple5" in idl.tables: simple5 = idl.tables["simple5"].rows for row in simple5.values(): @@ -256,6 +275,13 @@ def print_idl(idl, step): get_simple5_table_printable_row(row)) n += 1 + if "simple6" in idl.tables: + simple6 = idl.tables["simple6"].rows + for row in simple6.values(): + print_row("simple6", row, step, + get_simple6_table_printable_row(row)) + n += 1 + if "link1" in idl.tables: l1 = idl.tables["link1"].rows for row in l1.values(): _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev