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 inserted in the IDL client's in-memory view of
the database, if row 'b' is also deleted in the same transaction, it should
generate the following tracked changes:
- for table A:
- inserted records: a = {A._uuid=<U1>}
- for table B:
- inserted records: b = {B._uuid=<U2>, B.ref_a=<U1>}
- deleted records: b = {B._uuid=<U2>, B.ref_a=<U1>}
Before this patch, inserted and deleted records in table B
would (in some cases [0]) be b = {B._uuid=<U2>, B.ref_a=[]}.
Having B.ref_a=[] would violate the integrity of the database from client
perspective.
test-ovsdb has also been updated to show that one row can be
both inserted and deleted within one IDL.
[0] In ovn-controller the fact that the reference is NULL caused a crash
in the following case, when both commands were handled by ovn-controller
within the same loop:
$ ovn-nbctl ls-add sw0 -- lsp-add sw0 sw0-port1 -- lsp-set-addresses
sw0-port1 "50:54:00:00:00:01 192.168.0.2"
$ ovn-nbctl lsp-del sw0-port1
Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2126450
Fixes: 91e1ff5dde39 ("ovsdb-idl: Don't reparse orphaned rows.")
Signed-off-by: Xavier Simonart <[email protected]>
---
lib/ovsdb-idl.c | 4 +++
tests/ovsdb-idl.at | 89 ++++++++++++++++++++++++++++++++++++++++++++++
tests/test-ovsdb.c | 13 ++++---
3 files changed, 101 insertions(+), 5 deletions(-)
diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 576979f9e..e33d2a5c5 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -2419,6 +2419,10 @@ ovsdb_idl_insert_row(struct ovsdb_idl_row *row, const
struct shash *data)
static void
ovsdb_idl_delete_row(struct ovsdb_idl_row *row)
{
+ /* If row has to be reparsed, reparse it before it's deleted */
+ if (!ovs_list_is_empty(&row->reparse_node)) {
+ ovsdb_idl_row_parse(row);
+ }
ovsdb_idl_remove_from_indexes(row);
ovsdb_idl_row_clear_arcs(row, true);
ovsdb_idl_row_destroy(row);
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index d1cfa59c5..8e75d00d7 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -2466,3 +2466,92 @@ unix:socket2 remote has col id in table simple7
OVSDB_SERVER_SHUTDOWN
AT_CLEANUP
+
+dnl This test checks that inserting and deleting the source of a reference
+dnl doesn't remove the reference in the (deleted) source tracked record.
+OVSDB_CHECK_IDL_TRACK([track, insert and delete, refs to link1],
+ [],
+ [['["idltest",
+ {"op": "insert",
+ "table": "link2",
+ "uuid-name": "l2row0",
+ "row": {"i": 1, "l1": ["set", [["named-uuid", "l1row0"]]]}
+ },
+ {"op": "insert",
+ "table": "link1",
+ "uuid-name": "l1row0",
+ "row": {"i": 1, "k": ["named-uuid", "l1row0"]}
+ },
+ {"op": "insert",
+ "table": "link2",
+ "uuid-name": "l2row1",
+ "row": {"i": 2, "l1": ["set", [["named-uuid", "l1row0"]]]}
+ }
+ ]' \
+ '+["idltest",
+ {"op": "delete",
+ "table": "link2",
+ "where": [["i", "==", 2]]}
+ ]' \
+ '["idltest",
+ {"op": "delete",
+ "table": "link2",
+ "where": [["i", "==", 1]]}
+ ]'
+ ]],
+ [[000: empty
+001:
{"error":null,"result":[{"uuid":["uuid","<0>"]},{"uuid":["uuid","<1>"]},{"uuid":["uuid","<2>"]}]}
+002: {"error":null,"result":[{"count":1}]}
+003: table link1: inserted row: i=1 k=1 ka=[] l2= uuid=<1>
+003: table link1: updated columns: i k
+003: table link2: inserted row: i=1 l1=1 uuid=<0>
+003: table link2: inserted/deleted row: i=2 l1=1 uuid=<2>
+003: table link2: updated columns: i l1
+003: table link2: updated columns: i l1
+004: {"error":null,"result":[{"count":1}]}
+005: table link1: i=1 k=1 ka=[] l2= uuid=<1>
+006: done
+]])
+OVSDB_CHECK_IDL_TRACK([track, insert and delete, refs to link2],
+ [],
+ [['["idltest",
+ {"op": "insert",
+ "table": "link1",
+ "uuid-name": "l1row0",
+ "row": {"i": 1, "k": ["named-uuid", "l1row0"], "l2": ["set",
[["named-uuid", "l2row0"]]]}
+ },
+ {"op": "insert",
+ "table": "link2",
+ "uuid-name": "l2row0",
+ "row": {"i": 1}
+ },
+ {"op": "insert",
+ "table": "link1",
+ "uuid-name": "l1row1",
+ "row": {"i": 2, "k": ["named-uuid", "l1row0"], "l2": ["set",
[["named-uuid", "l2row0"]]]}
+ }
+ ]' \
+ '+["idltest",
+ {"op": "delete",
+ "table": "link1",
+ "where": [["i", "==", 2]]}
+ ]' \
+ '["idltest",
+ {"op": "delete",
+ "table": "link1",
+ "where": [["i", "==", 1]]}
+ ]'
+ ]],
+ [[000: empty
+001:
{"error":null,"result":[{"uuid":["uuid","<0>"]},{"uuid":["uuid","<1>"]},{"uuid":["uuid","<2>"]}]}
+002: {"error":null,"result":[{"count":1}]}
+003: table link1: inserted row: i=1 k=1 ka=[] l2=1 uuid=<0>
+003: table link1: inserted/deleted row: i=2 k=1 ka=[] l2=1 uuid=<2>
+003: table link1: updated columns: i k l2
+003: table link1: updated columns: i k l2
+003: table link2: inserted row: i=1 l1= uuid=<1>
+003: table link2: updated columns: i
+004: {"error":null,"result":[{"count":1}]}
+005: table link2: i=1 l1= uuid=<1>
+006: done
+]])
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 965b0f913..5f7110f41 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -1953,11 +1953,14 @@ format_idl_row(const struct ovsdb_idl_row *row, int
step, const char *contents,
const char *change_str =
!ovsdb_idl_track_is_set(row->table)
? ""
- : ovsdb_idl_row_get_seqno(row, OVSDB_IDL_CHANGE_INSERT) > 0
- ? "inserted row: "
- : ovsdb_idl_row_get_seqno(row, OVSDB_IDL_CHANGE_DELETE) > 0
- ? "deleted row: "
- : "";
+ : ovsdb_idl_row_get_seqno(row, OVSDB_IDL_CHANGE_INSERT) > 0 &&
+ ovsdb_idl_row_get_seqno(row, OVSDB_IDL_CHANGE_DELETE) > 0
+ ? "inserted/deleted row: "
+ : ovsdb_idl_row_get_seqno(row, OVSDB_IDL_CHANGE_INSERT) > 0
+ ? "inserted row: "
+ : ovsdb_idl_row_get_seqno(row, OVSDB_IDL_CHANGE_DELETE) > 0
+ ? "deleted row: "
+ : "";
if (terse) {
return xasprintf("%03d: table %s", step, row->table->class_->name);
--
2.31.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
