On 12/19/23 00:31, Terry Wilson wrote: > Currently python-ovs claims to be "db change aware" but does not > parse the "monitor_canceled" notification. Transactions can continue > being made, but the monitor updates will not be sent. This handles > monitor_cancel similarly to how ovsdb-cs currently does. > > Signed-off-by: Terry Wilson <[email protected]> > ---
Hi Terry, Thanks for the v2! > python/ovs/db/idl.py | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > index 9fc2159b0..be6ae2ca4 100644 > --- a/python/ovs/db/idl.py > +++ b/python/ovs/db/idl.py > @@ -299,6 +299,7 @@ class Idl(object): > self._server_schema_request_id = None > self._server_monitor_request_id = None > self._db_change_aware_request_id = None > + self._monitor_cancel_request_id = None This makes me a bit uneasy. I don't think there's ever a case when the first 3 IDs above are simultaneously not-None. For the monitor_cancel request ID it's possible it's not-None while others are also not-None because we immediately restart the FSM after sending the cancel request. Note: it's also what the ovsdb-cs layer does today (except that the cs layer doesn't even validate that the ID we get in the reply matches the ID of the request). I think ideally the state machine should be a bit more robust and just wait for the cancel reply to be received before restarting the FSM. Anyway, end of rant, I guess all of the above can be fixed as a follow up patch if we ever find time for it. Above rant aside: Acked-by: Dumitru Ceara <[email protected]> Regards, Dumitru > self._server_db_name = '_Server' > self._server_db_table = 'Database' > self.server_tables = None > @@ -481,6 +482,10 @@ class Idl(object): > break > else: > self.__parse_update(msg.params[1], OVSDB_UPDATE) > + elif self.handle_monitor_canceled(msg): > + break > + elif self.handle_monitor_cancel_reply(msg): > + break > elif (msg.type == ovs.jsonrpc.Message.T_REPLY > and self._monitor_request_id is not None > and self._monitor_request_id == msg.id): > @@ -615,6 +620,33 @@ class Idl(object): > > return initial_change_seqno != self.change_seqno > > + def handle_monitor_canceled(self, msg): > + if msg.type != msg.T_NOTIFY: > + return False > + if msg.method != "monitor_canceled": > + return False > + > + if msg.params[0] == str(self.uuid): > + params = [str(self.server_monitor_uuid)] > + elif msg.params[0] == str(self.server_monitor_uuid): > + params = [str(self.uuid)] > + else: > + return False > + > + mc_msg = ovs.jsonrpc.Message.create_request("monitor_cancel", params) > + self._monitor_cancel_request_id = mc_msg.id > + self.send_request(mc_msg) > + self.restart_fsm() > + return True > + > + def handle_monitor_cancel_reply(self, msg): > + if msg.type != msg.T_REPLY: > + return False > + if msg.id != self._monitor_cancel_request_id: > + return False > + self._monitor_cancel_request_id = None > + return True > + > def compose_cond_change(self): > if not self.cond_changed: > return _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
