From b3f8c32edad62acf8c6e93b0f8a6910f147d1897 Mon Sep 17 00:00:00 2001 From: Mike Pattrick Date: Tue, 12 Dec 2023 12:43:34 -0500 Subject: [PATCH] ovsdb-idl: Preserve change_seqno when deleting rows. In the case of a weak reference, clearing all change_seqno's can delete useful information. Instead of clearing all seqno's when removing track_node, only clear those values in cases including row insertion, and row deleting if no dst_arcs remain. Fixes: 95689f166818 ("ovsdb-idl: Preserve references for deleted rows.") Reported-at: https://issues.redhat.com/browse/FDP-193 Signed-off-by: Mike Pattrick Acked-by: Dumitru Ceara Signed-off-by: Simon Horman --- lib/ovsdb-idl.c | 11 ++++++++-- tests/ovsdb-idl.at | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c index 917868c54a1..a06d78c0de9 100644 --- a/lib/ovsdb-idl.c +++ b/lib/ovsdb-idl.c @@ -177,6 +177,7 @@ static void ovsdb_idl_row_mark_backrefs_for_reparsing(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_row_clear_changeseqno(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 *, @@ -1353,6 +1354,7 @@ ovsdb_idl_track_clear__(struct ovsdb_idl *idl, bool flush_all) row->updated = NULL; } ovsdb_idl_row_untrack_change(row); + ovsdb_idl_row_clear_changeseqno(row); if (ovsdb_idl_row_is_orphan(row)) { ovsdb_idl_row_unparse(row); @@ -1572,6 +1574,7 @@ ovsdb_idl_process_update(struct ovsdb_idl_table *table, ru->columns); } else if (ovsdb_idl_row_is_orphan(row)) { ovsdb_idl_row_untrack_change(row); + ovsdb_idl_row_clear_changeseqno(row); ovsdb_idl_insert_row(row, ru->columns); } else { VLOG_ERR_RL(&semantic_rl, "cannot add existing row "UUID_FMT" to " @@ -2223,11 +2226,15 @@ ovsdb_idl_row_untrack_change(struct ovsdb_idl_row *row) return; } + ovs_list_remove(&row->track_node); + ovs_list_init(&row->track_node); +} + +static void ovsdb_idl_row_clear_changeseqno(struct ovsdb_idl_row *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); } static struct ovsdb_idl_row * diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at index bacb7f161a0..fdb2273ba26 100644 --- a/tests/ovsdb-idl.at +++ b/tests/ovsdb-idl.at @@ -1422,6 +1422,56 @@ OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, references, singl 006: done ]]) +OVSDB_CHECK_IDL_TRACK([track, simple idl, initially populated, weak references, insert+delete batch], + [['["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": "insert", + "table": "simple", + "row": {"s": "row1_s"}, + "uuid-name": "uuid_row1_s"}, + {"op": "mutate", + "table": "simple6", + "where": [["name", "==", "row0_s6"]], + "mutations": [["weak_ref", "insert", ["set", [["named-uuid", "uuid_row1_s"]]]]]}]' \ + '+["idltest", + {"op": "delete", + "table": "simple", + "where": [["s", "==", "row1_s"]]}]' \ + '["idltest", + {"op": "insert", + "table": "simple", + "row": {"s": "row2_s"}}]']], + [[000: simple6: conditions unchanged +000: simple: conditions unchanged +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":[{"uuid":["uuid","<3>"]},{"count":1}]} +003: {"error":null,"result":[{"count":1}]} +004: table simple6: name=row0_s6 weak_ref=[<0>] uuid=<1> +004: table simple6: updated columns: weak_ref +004: table simple: inserted/deleted row: i=0 r=0 b=false s=row1_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<3> +004: table simple: updated columns: s +005: {"error":null,"result":[{"uuid":["uuid","<4>"]}]} +006: table simple6: name=row0_s6 weak_ref=[<0>] uuid=<1> +006: table simple: i=0 r=0 b=false s=row0_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<0> +006: table simple: inserted row: i=0 r=0 b=false s=row2_s u=<2> ia=[] ra=[] ba=[] sa=[] ua=[] uuid=<4> +006: table simple: updated columns: s +007: 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],