| Message ID | 20231116221636.3475471-1-twilson@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev] python: idl: Handle monitor_canceled | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/intel-ovs-compilation | success | test: success |
On 11/16/23 23:16, 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. Adding a > force_reconnect() upon receiving a "monitor_canceled" notification > resolves this issue. > > Signed-off-by: Terry Wilson <twilson@redhat.com> > --- Hi Terry, Thanks for the patch. > python/ovs/db/idl.py | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py > index 16ece0334..cfd81a1ec 100644 > --- a/python/ovs/db/idl.py > +++ b/python/ovs/db/idl.py > @@ -481,6 +481,10 @@ class Idl(object): > break > else: > self.__parse_update(msg.params[1], OVSDB_UPDATE) > + elif (msg.type == ovs.jsonrpc.Message.T_NOTIFY > + and msg.method == "monitor_canceled"): > + self.force_reconnect() > + break I didn't test this at all, I was just looking at the code, so I might be completely wrong but can't we avoid a full reconnect and instead just restart the fsm (self.restart_fsm())? The C version of the IDL/CS layer does something along those lines (it also cancels the other monitor): https://github.com/openvswitch/ovs/blob/7b514aba0e91c535024508624724a83a3df87b71/lib/ovsdb-cs.c#L1661-L1678 Also, force_reconnect() messes with the backoff a bit; I'm not completely sure we want that on monitor_cancel. > elif (msg.type == ovs.jsonrpc.Message.T_REPLY > and self._monitor_request_id is not None > and self._monitor_request_id == msg.id): Regards, Dumitru
Bleep bloop. Greetings Terry Wilson, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: The subject summary should end with a dot. Subject: python: idl: Handle monitor_canceled Lines checked: 35, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
diff --git a/python/ovs/db/idl.py b/python/ovs/db/idl.py index 16ece0334..cfd81a1ec 100644 --- a/python/ovs/db/idl.py +++ b/python/ovs/db/idl.py @@ -481,6 +481,10 @@ class Idl(object): break else: self.__parse_update(msg.params[1], OVSDB_UPDATE) + elif (msg.type == ovs.jsonrpc.Message.T_NOTIFY + and msg.method == "monitor_canceled"): + self.force_reconnect() + break elif (msg.type == ovs.jsonrpc.Message.T_REPLY and self._monitor_request_id is not None and self._monitor_request_id == msg.id):
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. Adding a force_reconnect() upon receiving a "monitor_canceled" notification resolves this issue. Signed-off-by: Terry Wilson <twilson@redhat.com> --- python/ovs/db/idl.py | 4 ++++ 1 file changed, 4 insertions(+)