[ovs-dev] [PATCH monitor_cond V5 04/18] ovsdb: generate update notifications for monitor_cond session

[Ben Pfaff]

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 <lirans at 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.

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?

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.

I'll continue reviewing the series once I understand this point.

Thank you!