ovsdb-idl: Preserve references for rows deleted in same IDL run as their insertion.

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 run, 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 run.

[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: 91e1ff5dde ("ovsdb-idl: Don't reparse orphaned rows.")
Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

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);

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
+]])

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);