From patchwork Thu Mar 4 16:18:35 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Terry Wilson X-Patchwork-Id: 1447427 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.136; helo=smtp3.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=gkv3FsBJ; dkim-atps=neutral Received: from smtp3.osuosl.org (smtp3.osuosl.org [140.211.166.136]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4Drx004MQpz9sW8 for ; Fri, 5 Mar 2021 03:19:28 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp3.osuosl.org (Postfix) with ESMTP id ECE696F54B; Thu, 4 Mar 2021 16:19:26 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp3.osuosl.org ([127.0.0.1]) by localhost (smtp3.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 5rvg2vOI407b; Thu, 4 Mar 2021 16:19:25 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by smtp3.osuosl.org (Postfix) with ESMTP id DDD4D605D3; Thu, 4 Mar 2021 16:19:24 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id BF442C000A; Thu, 4 Mar 2021 16:19:24 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp1.osuosl.org (smtp1.osuosl.org [140.211.166.138]) by lists.linuxfoundation.org (Postfix) with ESMTP id 8610AC0001 for ; Thu, 4 Mar 2021 16:19:23 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp1.osuosl.org (Postfix) with ESMTP id 5F69183279 for ; Thu, 4 Mar 2021 16:19:23 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Authentication-Results: smtp1.osuosl.org (amavisd-new); dkim=pass (1024-bit key) header.d=redhat.com Received: from smtp1.osuosl.org ([127.0.0.1]) by localhost (smtp1.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id llcfSS6PwLlF for ; Thu, 4 Mar 2021 16:19:22 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.8.0 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by smtp1.osuosl.org (Postfix) with ESMTPS id 5388D831D5 for ; Thu, 4 Mar 2021 16:19:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1614874761; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:content-type:content-type; bh=/RJMsRtgYSF6rHznn8za+PG13k0knfjUEJgdMEafxf8=; b=gkv3FsBJrf3TfV+FcgMo2jn6h0GgGnSaw3+Nebu8wqAxcm8okDPVnF74N1HUwz59QuEzXB 4SnAjJbB+ikmLsF4qwe3w/TNbtBA6eNhaU95346eWdHHKAO9lSBCV3XOR3u7YsgGkI0Fmp mFsIHO5UbAf/SCtS7sUAZzvdyLltH2A= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-426-Q14IxsQdP6i2ETACQIQeRQ-1; Thu, 04 Mar 2021 11:18:59 -0500 X-MC-Unique: Q14IxsQdP6i2ETACQIQeRQ-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E10BF108BD0E for ; Thu, 4 Mar 2021 16:18:38 +0000 (UTC) Received: from ubuntu1804.localdomain (ovpn-117-42.rdu2.redhat.com [10.10.117.42]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2424D19C48; Thu, 4 Mar 2021 16:18:37 +0000 (UTC) From: Terry Wilson To: dev@openvswitch.org Date: Thu, 4 Mar 2021 16:18:35 +0000 Message-Id: <20210304161835.10150-1-twilson@redhat.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=twilson@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH v5] python: Send notifications after the transaction ends X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" 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 Acked-by: Flavio Fernandes > Acked-by: Brian Haley Tested-by: Flavio Fernandes --- python/ovs/db/idl.py | 39 ++++++++++++++++++++++++--------------- tests/ovsdb-idl.at | 22 ++++++++++++++++++++++ tests/test-ovsdb.py | 7 +++++-- 3 files changed, 51 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(" 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(' 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(' 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..0ea759195 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,7 @@ 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: