On Wed, Mar 07, 2018 at 01:30:16PM +0800, Guoshuai Li wrote:
> From: Dong Jun <do...@dtdream.com>
> This patch can set inactivity probe for connection by command
> ovn-nbctl set-connection inactivity_probe=30000 ptcp:6641:
> ovn-sbctl set-connection inactivity_probe=30000 ptcp:6642:
> Signed-off-by: Guoshuai Li <l...@dtdream.com>

Thanks for working on making OVN better.  I think that this feature can
be useful.  I have a few additional comments.

I see a few casts to (int64_t).  I don't think that these have any value
and can be removed.

The user cannot tell how to use this feature from the documentation.
The actual syntax is inactivity_probe=<number>, but the documentation
doesn't say that, in multiple places.  For example:

> -      <dt><code>set-connection</code> <var>target</var>...</dt>
> +      <dt><code>set-connection</code> <code>inactivity_probe</code> 
> <var>target</var>...</dt>

I suggest that the documentation should also use [] to show that the
inactivity_probe is optional.

Here's another place with misleading documentation.  I won't point out
all of them:
> -  set-connection TARGET...   set the list of connections to TARGET...\n\
> +  set-connection [INACTIVITY_PROBE] TARGET...\n\
> +                             set the list of connections to TARGET...\n\

Thanks again!

dev mailing list

Reply via email to