[libvirt] [PATCH] virsh: fix keepalive error msg, man page update

[Martin Kletzander]

On Wed, Aug 27, 2014 at 03:15:35PM +0200, Erik Skultety wrote:
>resolves https://bugzilla.redhat.com/show_bug.cgi?id=1132305
>---
> tools/virsh.c   | 14 ++++++++++----
> tools/virsh.pod |  6 ++++--
> 2 files changed, 14 insertions(+), 6 deletions(-)
>
>diff --git a/tools/virsh.c b/tools/virsh.c
>index 30a84c1..f9b3991 100644
>--- a/tools/virsh.c
>+++ b/tools/virsh.c
>@@ -3472,8 +3472,11 @@ vshParseArgv(vshControl *ctl, int argc, char **argv)
>         case 'k':
>             if (virStrToLong_i(optarg, NULL, 0, &keepalive) < 0 ||
>                 keepalive < 0) {
>-                vshError(ctl, _("option %s requires a positive numeric argument"),
>-                         longindex == -1 ? "-k" : "--keepalive-interval");
>+                vshError(ctl,
>+                         _("option %s requires a positive integer argument "
>+                           "within range <0,%d>"),
>+                         longindex == -1 ? "-k" : "--keepalive-interval",
>+                         INT_MAX);

There is no reasonable use for any interval greater than, let's say,
100 seconds (and that's already pretty extreme).  INT_MAX seconds is
too much and reporting it may even confuse users.  Imagine that we
would have to report the limits for *all* options.  Why not just do 2
conditions:

 if (virStrToLong_i(optarg, NULL, 0, &keepalive) < 0) {
     vshError(ctl, _("Invalid value for option %s"),
              longindex == -1 ? "-k" : "--keepalive-interval");
     exit(EXIT_FAILURE);
 }
 if (keepalive < 0) {
     vshError(ctl, _("option %s requires a positive numeric argument"),
              longindex == -1 ? "-k" : "--keepalive-interval");
     exit(EXIT_FAILURE);
 }

>                 exit(EXIT_FAILURE);
>             }
>             ctl->keepalive_interval = keepalive;
>@@ -3481,8 +3484,11 @@ vshParseArgv(vshControl *ctl, int argc, char **argv)
>         case 'K':
>             if (virStrToLong_i(optarg, NULL, 0, &keepalive) < 0 ||
>                 keepalive < 0) {
>-                vshError(ctl, _("option %s requires a positive numeric argument"),
>-                         longindex == -1 ? "-K" : "--keepalive-count");
>+                vshError(ctl,
>+                         _("option %s requires a positive integer argument "
>+                           "within range <0,%d>"),
>+                         longindex == -1 ? "-K" : "--keepalive-count",
>+                         INT_MAX);
>                 exit(EXIT_FAILURE);

Similarly here.

>             }
>             ctl->keepalive_count = keepalive;
>diff --git a/tools/virsh.pod b/tools/virsh.pod
>index e0dfd13..9bee664 100644
>--- a/tools/virsh.pod
>+++ b/tools/virsh.pod
>@@ -77,13 +77,15 @@ given instead.
> =item B<-k>, B<--keepalive-interval> I<INTERVAL>
>
> Set an I<INTERVAL> (in seconds) for sending keepalive messages to
>-check whether connection to the server is still alive.  Setting the
>+check whether connection to the server is still alive. I<INTERVAL>
>+must be an integer value within range <0,INT_MAX>. Setting the

Same here, check another options that take "normal numbers", we don't
say that it needs to be between 0 and LLONG_MAX for example.

> interval to 0 disables client keepalive mechanism.
>
> =item B<-K>, B<--keepalive-count> I<COUNT>
>
> Set a number of times keepalive message can be sent without getting an
>-answer from the server without marking the connection dead.  There is
>+answer from the server without marking the connection dead. I<COUNT>
>+must be an integer value within range <0,INT_MAX>. There is

Same here.

Martin

> no effect to this setting in case the I<INTERVAL> is set to 0.
>
> =item B<-l>, B<--log> I<FILE>
>--
>1.9.3
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140827/2ae0e00b/attachment-0001.sig>