On Thu, Mar 19, 2015 at 12:08:28AM -0700, Andy Zhou wrote:
> Currently, each ovsdb-monitor points to a single jsonrpc_monitor object.
> This means there is 1:1 relationship between them.
>
> In case multiple jsonrpc-monitors needs to monitor the same tables and
> the columns within them, then can share a single ovsdb-monitor, so the
> updates only needs to be maintained once.
>
> This patch, with a few following patches is to allow for N:1 mapping
> between jsonrpc-monitor and ovsdb-monitor.
>
> Maintain jsonrpc-monitors pointers in a linked-list is the first
> step towards allowing N:1 mapping. The ovsdb-monitor life cycle
> is now reference counted. An empty list means zero references.
>
> Signed-off-by: Andy Zhou <az...@nicira.com>
It's starting to get a little awkward that ovsdb_monitor and
ovsdb_jsonrpc_monitor each points to the other. It's nice, when one
can, to have one-way relationships, where only one end of a
relationship has to keep track of the other end. Is there a clean
way, for example, for the ovsdb_monitor to not have to keep track of
the ovsdb_jsonrpc_monitor but only the other way around? Not sure;
food for thought.
strict jsonrpc_monitor_node seems extra awkward since it's a very
degenerate structure. It's OK for iterating through the
jsonrpc_monitors but not really for anything else, especially in
ovsdb_monitor_remove_jsonrpc_monitor(). Will later patches add more
members to this struct? If not, then there might be a way to get rid
of it entirely.
It's nice to say the particular type in a list, e.g. instead of:
struct ovs_list jsonrpc_monitors; /* List front end jsonrpc monitors. */
something like:
struct ovs_list jsonrpc_monitors; /* Contains "jsonrpc_monitor_node"s. */
In ovsdb_monitor_remove_jsonrpc_monitor(), s/destory/destroy/:
+ /* If this is the last jsonrpc monitor, also destory
and or s/users/user/:
+ * ovsdb monitor, since there is no other users of
+ * the monitor. */
and I would replace this by just OVS_NOT_REACHED():
+ /* Should never reach here. jsonrpc_monitor should be on the list. */
+ ovs_assert(true);
In ovsdb_monitor_destroy_callback() there's an extra space in the
comment here, and s/destroied/destroyed/:
+ /* Delete all front end monitors. Removing the last front
+ * end monitor will also destroy the corresponding 'ovsdb_monitor'.
+ * ovsdb monitor will also be destroied. */
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
