[ovs-dev] [ovsdb speedup v3 18/19] ovsdb-monitor: allow multiple jsonrpc monitors to share a single ovsdb monitor

[Ben Pfaff]

On Thu, Apr 09, 2015 at 06:40:27PM -0700, Andy Zhou wrote:
> Store ovsdb monitor in global hmap. A newly created ovsdb monitor
> object will first search the global hmap for a possible match. If
> one is found, the existing ovsdb monitor is used instead.
> 
> With this patch, jsonrpc monitor and ovsdb monitor now have N:1 mapping.
> 
> Signed-off-by: Andy Zhou <azhou at nicira.com>
> Acked-by: Ben Pfaff <blp at nicira.com>
> 
> ---
> v1->v2:
>       use HMAP_INITIALIZER to initialize ovsdb_monitors
>       style fixes
> 
> v2->v3: no change

This fixes the bug in the previous commit.

I am not sure that ovsdb_monitor_initial_cb() entirely makes sense.  It
is a cut-and-paste copy of ovsdb_monitor_change_cb() with a few changes,
but ovsdb_monitor_change_cb() is written to conform to the calling
convention needed to be passed to ovsdb_txn_for_each_change().  Some of
that doesn't make sense for a function that is called directly, e.g. the
use of ovsdb_monitor_aux, the comment about telling the caller to skip
it (the caller doesn't even use the return value, the caller could pass
in the ovsdb_monitor_table directly, and so on.

I think that the "not" in the comment in ovsdb_monitor_add() should be
removed (also you could use list_is_singleton() here):
+    /* New_dbmon should not be associated with only one jsonrpc
+     * connections.  */
+    ovs_assert(list_size(&new_dbmon->jsonrpc_monitors) == 1);

Thanks,

Ben.