Skip to content

Commit

Permalink
ovsdb-idl: Preserve change_seqno when deleting rows.
Browse files Browse the repository at this point in the history
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: 95689f1 ("ovsdb-idl: Preserve references for deleted rows.")
Reported-at: https://issues.redhat.com/browse/FDP-193
Signed-off-by: Mike Pattrick <mkp@redhat.com>
Acked-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Simon Horman <horms@ovn.org>
  • Loading branch information
mkp-rh authored and Simon Horman committed Jan 4, 2024
1 parent d254aed commit b3f8c32
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 2 deletions.
11 changes: 9 additions & 2 deletions lib/ovsdb-idl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 *,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 "
Expand Down Expand Up @@ -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 *
Expand Down
50 changes: 50 additions & 0 deletions tests/ovsdb-idl.at
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down

0 comments on commit b3f8c32

Please sign in to comment.