The Python IDL notification mechanism was sending a notification
for each processed update in a transaction as it was processed.
This causes issues with multi-row changes that contain references
to each other.
For example, if a Logical_Router_Port is created along with a
Gateway_Chassis, and the LRP.gateway_chassis set to that GC, then
when the notify() passes the CREATE event for the LRP, the GC will
not yet have been processed, so __getattr__ when _uuid_to_row fails
to find the GC, will return the default value for LRP.gateway_chassis
which is [].
This patch has the process_update methods return the notifications
that would be produced when a row changes, so they can be queued
and sent after all rows have been processed.
Fixes: d7d417fcddf9 ("Allow subclasses of Idl to define a notification hook")
Signed-off-by: Terry Wilson <[email protected]>
---
python/ovs/db/idl.py | 39 ++++++++++++++++++++++++---------------
tests/ovsdb-idl.at | 22 ++++++++++++++++++++++
tests/test-ovsdb.py | 8 ++++++--
3 files changed, 52 insertions(+), 17 deletions(-)
diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py
index 5850ac7ab..4226d1cb2 100644
--- a/python/ovs/db/idl.py
+++ b/python/ovs/db/idl.py
@@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+import collections
import functools
import uuid
@@ -39,6 +40,10 @@ OVSDB_UPDATE2 = 1
CLUSTERED = "clustered"
+Notice = collections.namedtuple('Notice', ('event', 'row', 'updates'))
+Notice.__new__.__defaults__ = (None,) # default updates=None
+
+
class Idl(object):
"""Open vSwitch Database Interface Definition Language (OVSDB IDL).
@@ -614,6 +619,7 @@ class Idl(object):
raise error.Error("<table-updates> is not an object",
table_updates)
+ notices = []
for table_name, table_update in table_updates.items():
table = tables.get(table_name)
if not table:
@@ -639,7 +645,9 @@ class Idl(object):
% (table_name, uuid_string))
if version == OVSDB_UPDATE2:
- if self.__process_update2(table, uuid, row_update):
+ changes = self.__process_update2(table, uuid, row_update)
+ if changes:
+ notices.append(changes)
self.change_seqno += 1
continue
@@ -652,17 +660,20 @@ class Idl(object):
raise error.Error('<row-update> missing "old" and '
'"new" members', row_update)
- if self.__process_update(table, uuid, old, new):
+ changes = self.__process_update(table, uuid, old, new)
+ if changes:
+ notices.append(changes)
self.change_seqno += 1
+ for notice in notices:
+ self.notify(*notice)
def __process_update2(self, table, uuid, row_update):
+ """Returns Notice if a column changed, False otherwise."""
row = table.rows.get(uuid)
- changed = False
if "delete" in row_update:
if row:
del table.rows[uuid]
- self.notify(ROW_DELETE, row)
- changed = True
+ return Notice(ROW_DELETE, row)
else:
# XXX rate-limit
vlog.warn("cannot delete missing row %s from table"
@@ -681,29 +692,27 @@ class Idl(object):
changed = self.__row_update(table, row, row_update)
table.rows[uuid] = row
if changed:
- self.notify(ROW_CREATE, row)
+ return Notice(ROW_CREATE, row)
elif "modify" in row_update:
if not row:
raise error.Error('Modify non-existing row')
old_row = self.__apply_diff(table, row, row_update['modify'])
- self.notify(ROW_UPDATE, row, Row(self, table, uuid, old_row))
- changed = True
+ return Notice(ROW_UPDATE, row, Row(self, table, uuid, old_row))
else:
raise error.Error('<row-update> unknown operation',
row_update)
- return changed
+ return False
def __process_update(self, table, uuid, old, new):
- """Returns True if a column changed, False otherwise."""
+ """Returns Notice if a column changed, False otherwise."""
row = table.rows.get(uuid)
changed = False
if not new:
# Delete row.
if row:
del table.rows[uuid]
- changed = True
- self.notify(ROW_DELETE, row)
+ return Notice(ROW_DELETE, row)
else:
# XXX rate-limit
vlog.warn("cannot delete missing row %s from table %s"
@@ -723,7 +732,7 @@ class Idl(object):
if op == ROW_CREATE:
table.rows[uuid] = row
if changed:
- self.notify(ROW_CREATE, row)
+ return Notice(ROW_CREATE, row)
else:
op = ROW_UPDATE
if not row:
@@ -737,8 +746,8 @@ class Idl(object):
if op == ROW_CREATE:
table.rows[uuid] = row
if changed:
- self.notify(op, row, Row.from_json(self, table, uuid, old))
- return changed
+ return Notice(op, row, Row.from_json(self, table, uuid, old))
+ return False
def __check_server_db(self):
"""Returns True if this is a valid server database, False
otherwise."""
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 4b4791a7d..e00e67e94 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -1486,6 +1486,28 @@ m4_define([OVSDB_CHECK_IDL_NOTIFY],
[OVSDB_CHECK_IDL_PY([$1], [], [$2], [$3], [notify $4], [$5])
OVSDB_CHECK_IDL_SSL_PY([$1], [], [$2], [$3], [notify $4], [$5])])
+OVSDB_CHECK_IDL_NOTIFY([simple link idl verify notify],
+ [['track-notify' \
+ '["idltest",
+ {"op": "insert",
+ "table": "link1",
+ "row": {"i": 1, "k": ["named-uuid", "l1row"], "l2": ["set", [["named-uuid",
"l2row"]]]},
+ "uuid-name": "l1row"},
+ {"op": "insert",
+ "table": "link2",
+ "uuid-name": "l2row",
+ "row": {"i": 2, "l1": ["set", [["named-uuid", "l1row"]]]}}]']],
+[[000: empty
+000: event:create, row={uuid=<0>}, updates=None
+000: event:create, row={uuid=<1>}, updates=None
+001: {"error":null,"result":[{"uuid":["uuid","<2>"]},{"uuid":["uuid","<3>"]}]}
+002: event:create, row={i=1 uuid=<2> l2=[<3>]}, updates=None
+002: event:create, row={i=2 uuid=<3> l1=[<2>]}, updates=None
+002: i=1 k=1 ka=[] l2=2 uuid=<2>
+002: i=2 l1=1 uuid=<3>
+003: done
+]])
+
OVSDB_CHECK_IDL_NOTIFY([simple idl verify notify],
[['track-notify' \
'["idltest",
diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
index a19680274..9d3228f23 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())
+ if isinstance(value, (list, tuple)):
+ value = sorted((row_to_uuid(v) for v in value))
s += "%s=%s " % (column, value)
s = s.strip()
s = re.sub('""|,|u?\'', "", s)
@@ -172,9 +174,10 @@ def get_simple_printable_row_string(row, columns):
return s
-def get_simple_table_printable_row(row):
+def get_simple_table_printable_row(row, *additional_columns):
simple_columns = ["i", "r", "b", "s", "u", "ia",
"ra", "ba", "sa", "ua", "uuid"]
+ simple_columns.extend(additional_columns)
return get_simple_printable_row_string(row, simple_columns)
@@ -637,7 +640,8 @@ def do_idl(schema_file, remote, *commands):
def mock_notify(event, row, updates=None):
output = "%03d: " % step
output += "event:" + str(event) + ", row={"
- output += get_simple_table_printable_row(row) + "}, updates="
+ output += get_simple_table_printable_row(row,
+ 'l2', 'l1') + "}, updates="
if updates is None:
output += "None"
else:
