On Thu, Mar 08, 2018 at 07:24:47PM +0800, Guoshuai Li wrote:
> From: Dong Jun <[email protected]>
>
> This patch can set inactivity probe for connection by command
> ovn-nbctl set-connection inactivity_probe=30000 ptcp:6641:0.0.0.0
> ovn-sbctl set-connection inactivity_probe=30000 ptcp:6642:0.0.0.0
>
> Signed-off-by: Guoshuai Li <[email protected]>
Thanks for v2. Here are some suggestions for folding in on the next
version.
I have some additional thoughts here. I am probably thinking about it
all too hard.
First, ovn-nbctl (etc.) has a syntax for options that would ordinarily
be used, e.g.:
ovn-nbctl --inactivity-probe=MSECS set-connection TARGET...
The possible disadvantage of that is that it could not be used to set a
different inactivity probe value for different targets. However, I
don't know whether that is actually a valuable use case. If it is
valuable, though, there is the problem that cmd_get_connection()
currently won't show the inactivity probes in an order that could be fed
directly back to set-connection, since it might put the ones with the
defaults after the ones that are non-default. However, I'm not sure
that get-connection showing the inactivity probes is actually valuable
at all!
Anyway, I'd be inclined to use --inactivity-probe as an option, have it
apply to all connections, and then make get-connection not print the
inactivity probes at all (which matches what "ovs-vsctl get-controller"
does, and I don't recall anyone ever complaining about that before).
What do you think?
Thanks,
Ben.
--8<--------------------------cut here-------------------------->8--
diff --git a/ovn/utilities/ovn-nbctl.8.xml b/ovn/utilities/ovn-nbctl.8.xml
index 521d6c3b4520..cbf20f047028 100644
--- a/ovn/utilities/ovn-nbctl.8.xml
+++ b/ovn/utilities/ovn-nbctl.8.xml
@@ -866,11 +866,11 @@
Deletes the configured connection(s).
</dd>
- <dt><code>set-connection</code> [<code>inactivity_probe=time</code>]
<var>target</var>...</dt>
+ <dt><code>set-connection</code>
[<code>inactivity_probe=</code><var>msecs</var>] <var>target</var>...</dt>
<dd>
- Sets the configured manager target or targets. Specified Maximum number
- of milliseconds of idle connection inactivity probe time by
- <code>inactivity_probe=time</code>, A value of 0 disables inactivity
+ Sets the configured manager target or targets. Use
+ <code>inactivity_probe=</code><var>msecs</var> to override the default
+ idle connection inactivity probe time. Use 0 to disable inactivity
probes.
</dd>
</dl>
diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c
index 75ab1375bced..9d7b8591d4be 100644
--- a/ovn/utilities/ovn-nbctl.c
+++ b/ovn/utilities/ovn-nbctl.c
@@ -446,7 +446,7 @@ DHCP Options commands:\n\
Connection commands:\n\
get-connection print the connections\n\
del-connection delete the connections\n\
- set-connection [inactivity_probe=TIME] TARGET...\n\
+ set-connection [inactivity_probe=MSECS] TARGET...\n\
set the list of connections to TARGET...\n\
\n\
SSL commands:\n\
@@ -3509,12 +3509,10 @@ cmd_get_connection(struct ctl_context *ctx)
NBREC_CONNECTION_FOR_EACH(conn, ctx->idl) {
if (conn->n_inactivity_probe) {
- char *s;
- s = xasprintf("inactivity_probe=%"PRId64" %s",
- conn->inactivity_probe[0],
- conn->target);
- svec_add(&targets, s);
- free(s);
+ svec_add_nocopy(&targets,
+ xasprintf("inactivity_probe=%"PRId64" %s",
+ conn->inactivity_probe[0],
+ conn->target));
} else {
svec_add(&targets, conn->target);
}
@@ -4085,7 +4083,7 @@ static const struct ctl_command_syntax nbctl_commands[] =
{
{"get-connection", 0, 0, "", pre_connection, cmd_get_connection, NULL, "",
RO},
{"del-connection", 0, 0, "", pre_connection, cmd_del_connection, NULL, "",
RW},
{"set-connection", 1, INT_MAX,
- "[inactivity_probe=TIME] TARGET...",
+ "[inactivity_probe=MSECS] TARGET...",
pre_connection, cmd_set_connection,
NULL, "", RW},
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
