On Fri, Nov 26, 2021 at 5:38 AM Dumitru Ceara <[email protected]> wrote: > Nit: To be consistent with the other ifs below we should probably add > this comment: > > # Database contents changed.
Will do. > > + # If 'found' is false, clear table rows for new > > dump > > + if not msg.result[0]: > > + self.__clear() > > Can msg.result[0] be 'false'? I think so and we should call __clear() > then and I think we won't; or am I reading this wrong? if not msg.result[0] is equivalent (mostly) to if msg.result[0] == False, in which case we would call self.__clear(). > > @@ -420,9 +571,16 @@ class Idl(object): > > > > if cond == []: > > cond = [False] > > - if table.condition != cond: > > - table.condition = cond > > - table.cond_changed = True > > + > > + # Compare the new condition to the last known condition > > + if table.condition.latest != cond: > > + table.condition.init(cond) > > + self.cond_changed = True > > + p = ovs.poller.Poller() > > + p.immediate_wake() The poller lines here were wrong due to differences in how python-ovs and the C code do polling. Instead, I'll add some code to Idl.wait() to wake up if self.cond_changed. > > + return self.cond_seqno + 1 > > I'm not sure this is correct. In the C ovsdb-cs module we do: > > https://github.com/openvswitch/ovs/blob/7b8aeadd60c82f99a9d791a7df7c617254654f9d/lib/ovsdb-cs.c#L947 > > Which translates to expecting "self.cond_seqno + 2" if there's at least > one condition change request in flight for one of the monitor tables. Ah, good point. I'll fix this. Thanks for the review! Terry _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
