Ben Pfaff <b...@ovn.org> wrote on 22/03/2016 07:23:33 PM: > On Fri, Mar 04, 2016 at 08:08:59AM +0000, Liran Schour wrote: > > Hold session's conditions in ovsdb_monitor_session_condition. Pass it > > to ovsdb_monitor for generating "update2" notifications. > > Add functions that can generate "update2" notification for a > > "monitor_cond" session. > > JSON cache is enabled only for session's with true condition only. > > "monitor_cond" and "monitor_cond_change" are RFC 7047 extensions > > described by ovsdb-server(1) manpage. > > > > Signed-off-by: Liran Schour <lir...@il.ibm.com> > > I think that ovsdb_monitor_table_condition_set() has a memory leak in > the error case; it does not free the 'error' returned to it. >
Right. Will fix that. > I'm a little confused about ovsdb_monitor_session_condition. Why is > this a separate data structure? That is, why is there a separate shash > of conditions rather than incorporating a condition into > ovsdb_monitor_table? > A single ovsdb_monitor instance can be used by many jsonrpc sessions (as long as they share the same set of tables and columns). Ovsdb_monitor_condition_session is holding the condition state per each jsonrpc session. Each session, in ovsdb_monitor, can have a different set of conditions and it is being represented by ovsdb_monitor_session_condition with ovsdb_monitor_table_condition per table. The trivial option was to divide ovsdb_monitor into 2 different instances if we have 2 sessions with different sets of conditions. But, condition is changing during the lifetime of monitor session. And by holding sessions with different conditions under the same ovsdb_monitor we can reduce the computation needed to insert row changes to ovsdb_monitor. (insert once instead of twice) > If a separate data structure is really needed then possibly it would be > better to only include the non-"true" conditions in the shash. Then it > would be easy to determine how many non-"true" conditions there were > (shash_count(conditions)) and hence whether there were any at all. > Something like this can be done however I do not support this. Conditions might change and it is needed to keep the condition state even if it is true (implicit or explicit). > I'll continue reviewing the series once I understand this point. Hope that it is clearer now. Thanks. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
