On Thu, Mar 19, 2015 at 12:08:34AM -0700, Andy Zhou wrote:
> Currently, each monitor table contains a single hmap 'changes' to
> track updates. This patch introduces a new data structure
> 'ovsdb_monitor_changes' that stores the updates 'rows' tagged by
> its first commit transaction id. Each 'ovsdb_monitor_changes' is
> refenece counted allowing multiple jsonrpc_monitors to share them.
>
> The next patch will allow each ovsdb monitor table to store a list
> of 'ovsdb_monitor_changes'. This patch stores only one, same as
> before.
>
> Signed-off-by: Andy Zhou <[email protected]>
Is there a reason to separate ovsdb_monitor_compose_update() from
ovsdb_monitor_renew_tracking_changes()? It looks to me like they are
always called together, in the same way, and only from one place
anyhow.
In ovsdb_monitor_table_track_changes(), this:
+ struct ovsdb_monitor_changes *changes;
+
+ changes = mt->changes;
+ if (changes) {
+ ovs_assert(false);
+ } else {
+ ovsdb_monitor_table_add_changes(mt, transaction);
+ }
looks to me like an awkward way to write:
ovs_assert(!mt->changes);
ovsdb_monitor_table_add_changes(mt, transaction);
No?
There's an extra ; after ovsdb_monitor_table_add_changes().
ovsdb_monitor_destroy() destroys the contents of mt->changes with a
call to ovsdb_monitor_changes_destroy_rows() but I don't see anything
that calls free(mt->changes). Leak?
Acked-by: Ben Pfaff <[email protected]>
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
