[ovs-dev] [PATCH v3 2/3] ovsdb-idl: Preserve references for deleted rows.

Dumitru Ceara dceara at redhat.com
Fri Mar 12 12:07:43 UTC 2021


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 <dceara at redhat.com>
---
 lib/ovsdb-idl.c     |  135 ++++++++++++++++++++++++++--------
 tests/ovsdb-idl.at  |  203 +++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/test-ovsdb.c  |   50 +++++++++++++
 tests/test-ovsdb.py |   14 ++++
 4 files changed, 370 insertions(+), 32 deletions(-)

diff --git a/lib/ovsdb-idl.c b/lib/ovsdb-idl.c
index 9e1e787..1542da9 100644
--- a/lib/ovsdb-idl.c
+++ b/lib/ovsdb-idl.c
@@ -92,6 +92,7 @@ struct ovsdb_idl {
     struct ovsdb_idl_txn *txn;
     struct hmap outstanding_txns;
     bool verify_write_only;
+    struct ovs_list orphan_rows; /* All deleted but still referenced rows. */
 };
 
 static struct ovsdb_cs_ops ovsdb_idl_cs_ops;
@@ -144,6 +145,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_process_orphans(struct ovsdb_idl *);
 
 static void ovsdb_idl_txn_process_reply(struct ovsdb_idl *,
                                         const struct jsonrpc_msg *);
@@ -163,6 +165,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 +254,7 @@ ovsdb_idl_create_unconnected(const struct ovsdb_idl_class *class,
         .txn = NULL,
         .outstanding_txns = HMAP_INITIALIZER(&idl->outstanding_txns),
         .verify_write_only = false,
+        .orphan_rows = OVS_LIST_INITIALIZER(&idl->orphan_rows),
     };
 
     uint8_t default_mode = (monitor_everything_by_default
@@ -351,6 +358,12 @@ ovsdb_idl_set_leader_only(struct ovsdb_idl *idl, bool leader_only)
 static void
 ovsdb_idl_clear(struct ovsdb_idl *db)
 {
+    /* Process orphan rows, removing them from the 'orphan_rows' list. */
+    ovsdb_idl_process_orphans(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 +380,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->orphan_rows));
     db->change_seqno++;
 }
 
@@ -415,7 +437,7 @@ ovsdb_idl_run(struct ovsdb_idl *idl)
         }
         ovsdb_cs_event_destroy(event);
     }
-
+    ovsdb_idl_process_orphans(idl);
     ovsdb_idl_row_destroy_postprocess(idl);
 }
 
@@ -1226,13 +1248,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 +1367,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_process_orphans(struct ovsdb_idl *db)
+{
+    struct ovsdb_idl_row *row, *next;
+
+    LIST_FOR_EACH_SAFE (row, next, track_node, &db->orphan_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 +1446,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 +1494,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);
         }
     }
@@ -2016,12 +2055,48 @@ ovsdb_idl_row_reparse_backrefs(struct ovsdb_idl_row *row)
     LIST_FOR_EACH_SAFE (arc, next, dst_node, &row->dst_arcs) {
         struct ovsdb_idl_row *ref = arc->src;
 
+        /* Skip reparsing deleted refs.  If ref's table has change tracking
+         * enabled, the users need the original information (and that will be
+         * cleaned up by ovsdb_idl_track_clear()).  Otherwise, 'row' is
+         * already orphan and is not accessible to the IDL user and will be
+         * cleaned up at the end of the current IDL run.
+         */
+        if (ovsdb_idl_row_get_seqno(row, OVSDB_IDL_CHANGE_DELETE)) {
+            continue;
+        }
+
         ovsdb_idl_row_unparse(ref);
         ovsdb_idl_row_clear_arcs(ref, false);
         ovsdb_idl_row_parse(ref);
     }
 }
 
+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 +2123,25 @@ 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
+ * 'orphan_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->orphan_rows, &row->track_node);
     }
 }
 
@@ -2153,12 +2231,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 610680c..7ac3c20 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?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,177 @@ 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 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
+]])
+
 OVSDB_CHECK_IDL_TRACK([track, simple idl, initially empty, various ops],
   [],
   [['["idltest",
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index f50e88d..72d0163 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -1947,6 +1947,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)
 {
     struct ds updates = DS_EMPTY_INITIALIZER;
@@ -2090,6 +2107,20 @@ print_idl_row_simple3(const struct idltest_simple3 *s3, int 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)
 {
     struct ds msg = DS_EMPTY_INITIALIZER;
@@ -2126,6 +2157,7 @@ print_idl_row_singleton(const struct idltest_singleton *sng, int step)
 static void
 print_idl(struct ovsdb_idl *idl, int step)
 {
+    const struct idltest_simple6 *s6;
     const struct idltest_simple *s;
     const struct idltest_link1 *l1;
     const struct idltest_link2 *l2;
@@ -2144,6 +2176,10 @@ print_idl(struct ovsdb_idl *idl, int step)
         print_idl_row_link2(l2, 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 +2192,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 +2212,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 +2450,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 bc7ea4f..b8b8e03 100644
--- a/tests/test-ovsdb.py
+++ b/tests/test-ovsdb.py
@@ -162,6 +162,8 @@ def get_simple_printable_row_string(row, columns):
             if isinstance(value, dict):
                 value = sorted((row_to_uuid(k), row_to_uuid(v))
                                for k, v in value.items())
+            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)
@@ -193,6 +195,11 @@ def get_simple5_table_printable_row(row):
     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:
@@ -253,6 +260,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():



More information about the dev mailing list