[libvirt] [PATCHv2] virsh: Make vshCommandOpt* report error

Michal Privoznik mprivozn at redhat.com
Fri Apr 4 13:50:39 UTC 2014


Currently, the virsh code is plenty of the following pattern:

  if (vshCommandOptUInt(..) < 0) {
      vshError(...);
      goto cleanup;
  }

It doesn't make much sense to repeat the code everywhere.
Moreover, some functions from the family already report
error some of them don't.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 tests/vcpupin                |   2 +-
 tools/virsh-domain-monitor.c |   7 +--
 tools/virsh-domain.c         | 102 +++++++++++++++----------------------------
 tools/virsh-host.c           |  25 +++--------
 tools/virsh-interface.c      |   4 +-
 tools/virsh-network.c        |   4 +-
 tools/virsh-volume.c         |  16 ++-----
 tools/virsh.c                |  99 +++++++++++++++++++++++++++++++----------
 tools/virsh.h                |  24 +++++-----
 9 files changed, 140 insertions(+), 143 deletions(-)

diff --git a/tests/vcpupin b/tests/vcpupin
index f1fb038..638f62b 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -34,7 +34,7 @@ fail=0
 $abs_top_builddir/tools/virsh --connect test:///default vcpupin test a 0,1 > out 2>&1
 test $? = 1 || fail=1
 cat <<\EOF > exp || fail=1
-error: vcpupin: Invalid or missing vCPU number.
+error: Unable to parse integer parameter to --vcpu
 
 EOF
 compare exp out || fail=1
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 18d551a..0da4f91 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -324,12 +324,9 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
     /* Providing a period will adjust the balloon driver collection period.
      * This is not really an unsigned long, but it
      */
-    if ((rv = vshCommandOptInt(cmd, "period", &period)) < 0) {
-        vshError(ctl, "%s",
-                 _("Unable to parse integer parameter."));
+    if ((rv = vshCommandOptInt(ctl, cmd, "period", &period)) < 0) {
         goto cleanup;
-    }
-    if (rv > 0) {
+    } else if (rv > 0) {
         if (period < 0) {
             vshError(ctl, _("Invalid collection period value '%d'"), period);
             goto cleanup;
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 73414f8..821ed62 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1119,8 +1119,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
     if (vshCommandOptStringReq(ctl, cmd, "device", &disk) < 0)
         goto cleanup;
 
-    if ((rv = vshCommandOptULongLong(cmd, "total-bytes-sec", &value)) < 0) {
-        goto interror;
+    if ((rv = vshCommandOptULongLong(ctl, cmd, "total-bytes-sec", &value)) < 0) {
+        goto cleanup;
     } else if (rv > 0) {
         if (virTypedParamsAddULLong(&params, &nparams, &maxparams,
                                     VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC,
@@ -1128,8 +1128,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
             goto save_error;
     }
 
-    if ((rv = vshCommandOptULongLong(cmd, "read-bytes-sec", &value)) < 0) {
-        goto interror;
+    if ((rv = vshCommandOptULongLong(ctl, cmd, "read-bytes-sec", &value)) < 0) {
+        goto cleanup;
     } else if (rv > 0) {
         if (virTypedParamsAddULLong(&params, &nparams, &maxparams,
                                     VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC,
@@ -1137,8 +1137,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
             goto save_error;
     }
 
-    if ((rv = vshCommandOptULongLong(cmd, "write-bytes-sec", &value)) < 0) {
-        goto interror;
+    if ((rv = vshCommandOptULongLong(ctl, cmd, "write-bytes-sec", &value)) < 0) {
+        goto cleanup;
     } else if (rv > 0) {
         if (virTypedParamsAddULLong(&params, &nparams, &maxparams,
                                     VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC,
@@ -1146,8 +1146,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
             goto save_error;
     }
 
-    if ((rv = vshCommandOptULongLong(cmd, "total-iops-sec", &value)) < 0) {
-        goto interror;
+    if ((rv = vshCommandOptULongLong(ctl, cmd, "total-iops-sec", &value)) < 0) {
+        goto cleanup;
     } else if (rv > 0) {
         if (virTypedParamsAddULLong(&params, &nparams, &maxparams,
                                     VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC,
@@ -1155,8 +1155,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
             goto save_error;
     }
 
-    if ((rv = vshCommandOptULongLong(cmd, "read-iops-sec", &value)) < 0) {
-        goto interror;
+    if ((rv = vshCommandOptULongLong(ctl, cmd, "read-iops-sec", &value)) < 0) {
+        goto cleanup;
     } else if (rv > 0) {
         if (virTypedParamsAddULLong(&params, &nparams, &maxparams,
                                     VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC,
@@ -1164,8 +1164,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
             goto save_error;
     }
 
-    if ((rv = vshCommandOptULongLong(cmd, "write-iops-sec", &value)) < 0) {
-        goto interror;
+    if ((rv = vshCommandOptULongLong(ctl, cmd, "write-iops-sec", &value)) < 0) {
+        goto cleanup;
     } else if (rv > 0) {
         if (virTypedParamsAddULLong(&params, &nparams, &maxparams,
                                     VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC,
@@ -1216,10 +1216,6 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
  error:
     vshError(ctl, "%s", _("Unable to change block I/O throttle"));
     goto cleanup;
-
- interror:
-    vshError(ctl, "%s", _("Unable to parse integer parameter"));
-    goto cleanup;
 }
 
 /*
@@ -1315,8 +1311,7 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return false;
 
-    if ((rv = vshCommandOptInt(cmd, "weight", &weight)) < 0) {
-        vshError(ctl, "%s", _("Unable to parse integer parameter"));
+    if ((rv = vshCommandOptInt(ctl, cmd, "weight", &weight)) < 0) {
         goto cleanup;
     } else if (rv > 0) {
         if (weight <= 0) {
@@ -1457,10 +1452,8 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
     if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
         goto cleanup;
 
-    if (vshCommandOptUL(cmd, "bandwidth", &bandwidth) < 0) {
-        vshError(ctl, "%s", _("bandwidth must be a number"));
+    if (vshCommandOptUL(ctl, cmd, "bandwidth", &bandwidth) < 0)
         goto cleanup;
-    }
 
     switch ((vshCmdBlockJobMode) mode) {
     case VSH_CMD_BLOCK_JOB_ABORT:
@@ -2215,10 +2208,8 @@ cmdBlockResize(vshControl *ctl, const vshCmd *cmd)
     if (vshCommandOptStringReq(ctl, cmd, "path", (const char **) &path) < 0)
         return false;
 
-    if (vshCommandOptScaledInt(cmd, "size", &size, 1024, ULLONG_MAX) < 0) {
-        vshError(ctl, "%s", _("Unable to parse integer"));
+    if (vshCommandOptScaledInt(ctl, cmd, "size", &size, 1024, ULLONG_MAX) < 0)
         return false;
-    }
 
     /* Prefer the older interface of KiB.  */
     if (size % 1024 == 0)
@@ -2805,10 +2796,8 @@ cmdDomPMSuspend(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
         return false;
 
-    if (vshCommandOptULongLong(cmd, "duration", &duration) < 0) {
-        vshError(ctl, _("Invalid duration argument"));
+    if (vshCommandOptULongLong(ctl, cmd, "duration", &duration) < 0)
         goto cleanup;
-    }
 
     if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0)
         goto cleanup;
@@ -4709,10 +4698,8 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
     if (vshCommandOptStringReq(ctl, cmd, "file", (const char **) &file) < 0)
         return false;
 
-    if (vshCommandOptUInt(cmd, "screen", &screen) < 0) {
-        vshError(ctl, "%s", _("invalid screen ID"));
+    if (vshCommandOptUInt(ctl, cmd, "screen", &screen) < 0)
         return false;
-    }
 
     if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
         return false;
@@ -5819,9 +5806,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
     query = !cpulist;
 
     /* In query mode, "vcpu" is optional */
-    if (vshCommandOptInt(cmd, "vcpu", &vcpu) < !query) {
-        vshError(ctl, "%s",
-                 _("vcpupin: Invalid or missing vCPU number."));
+    if (vshCommandOptInt(ctl, cmd, "vcpu", &vcpu) < !query) {
         virDomainFree(dom);
         return false;
     }
@@ -6063,7 +6048,7 @@ static bool
 cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
 {
     virDomainPtr dom;
-    int count = 0;
+    unsigned int count = 0;
     bool ret = false;
     bool maximum = vshCommandOptBool(cmd, "maximum");
     bool config = vshCommandOptBool(cmd, "config");
@@ -6089,10 +6074,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return false;
 
-    if (vshCommandOptInt(cmd, "count", &count) < 0 || count <= 0) {
-        vshError(ctl, "%s", _("Invalid number of virtual CPUs"));
+    if (vshCommandOptUInt(ctl, cmd, "count", &count) < 0)
         goto cleanup;
-    }
 
     if (flags == -1) {
         if (virDomainSetVcpus(dom, count) != 0)
@@ -6384,8 +6367,7 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd)
 
     show_total = vshCommandOptBool(cmd, "total");
 
-    if ((rv = vshCommandOptInt(cmd, "start", &cpu)) < 0) {
-        vshError(ctl, "%s", _("Unable to parse integer parameter for start"));
+    if ((rv = vshCommandOptInt(ctl, cmd, "start", &cpu)) < 0) {
         goto cleanup;
     } else if (rv > 0) {
         if (cpu < 0) {
@@ -6395,9 +6377,7 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd)
         show_per_cpu = true;
     }
 
-    if ((rv = vshCommandOptInt(cmd, "count", &show_count)) < 0) {
-        vshError(ctl, "%s",
-                 _("Unable to parse integer parameter for CPUs to show"));
+    if ((rv = vshCommandOptInt(ctl, cmd, "count", &show_count)) < 0) {
         goto cleanup;
     } else if (rv > 0) {
         if (show_count < 0) {
@@ -7157,10 +7137,8 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd)
     if (vshCommandOptString(cmd, "codeset", &codeset_option) <= 0)
         codeset_option = "linux";
 
-    if (vshCommandOptUInt(cmd, "holdtime", &holdtime) < 0) {
-        vshError(ctl, _("invalid value of --holdtime"));
+    if (vshCommandOptUInt(ctl, cmd, "holdtime", &holdtime) < 0)
         goto cleanup;
-    }
 
     codeset = virKeycodeSetTypeFromString(codeset_option);
     if (codeset < 0) {
@@ -7384,8 +7362,7 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd)
         max = 1024ull * ULONG_MAX;
     else
         max = ULONG_MAX;
-    if (vshCommandOptScaledInt(cmd, "size", &bytes, 1024, max) < 0) {
-        vshError(ctl, "%s", _("memory size has to be a number"));
+    if (vshCommandOptScaledInt(ctl, cmd, "size", &bytes, 1024, max) < 0) {
         virDomainFree(dom);
         return false;
     }
@@ -7481,8 +7458,7 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
         max = 1024ull * ULONG_MAX;
     else
         max = ULONG_MAX;
-    if (vshCommandOptScaledInt(cmd, "size", &bytes, 1024, max) < 0) {
-        vshError(ctl, "%s", _("memory size has to be a number"));
+    if (vshCommandOptScaledInt(ctl, cmd, "size", &bytes, 1024, max) < 0) {
         virDomainFree(dom);
         return false;
     }
@@ -8123,9 +8099,11 @@ cmdQemuAttach(vshControl *ctl, const vshCmd *cmd)
     bool ret = false;
     unsigned int flags = 0;
     unsigned int pid_value; /* API uses unsigned int, not pid_t */
+    int rv;
 
-    if (vshCommandOptUInt(cmd, "pid", &pid_value) <= 0) {
-        vshError(ctl, "%s", _("missing pid value"));
+    if ((rv = vshCommandOptUInt(ctl, cmd, "pid", &pid_value)) <= 0) {
+        if (rv == 0)
+            vshError(ctl, "%s", _("missing pid value"));
         goto cleanup;
     }
 
@@ -8220,9 +8198,8 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd)
     }
     guest_agent_cmd = virBufferContentAndReset(&buf);
 
-    judge = vshCommandOptInt(cmd, "timeout", &timeout);
+    judge = vshCommandOptInt(ctl, cmd, "timeout", &timeout);
     if (judge < 0) {
-        vshError(ctl, "%s", _("timeout number has to be a number"));
         goto cleanup;
     } else if (judge > 0) {
         judge = 1;
@@ -9089,17 +9066,14 @@ static bool
 cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd)
 {
     virDomainPtr dom = NULL;
-    long long downtime = 0;
+    unsigned long long downtime = 0;
     bool ret = false;
 
     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return false;
 
-    if (vshCommandOptLongLong(cmd, "downtime", &downtime) < 0 ||
-        downtime < 1) {
-        vshError(ctl, "%s", _("migrate: Invalid downtime"));
+    if (vshCommandOptULongLong(ctl, cmd, "downtime", &downtime) < 0)
         goto done;
-    }
 
     if (virDomainMigrateSetMaxDowntime(dom, downtime, 0))
         goto done;
@@ -9152,9 +9126,7 @@ cmdMigrateCompCache(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return false;
 
-    rc = vshCommandOptULongLong(cmd, "size", &size);
-    if (rc < 0) {
-        vshError(ctl, "%s", _("Unable to parse size parameter"));
+    if ((rc = vshCommandOptULongLong(ctl, cmd, "size", &size)) < 0) {
         goto cleanup;
     } else if (rc != 0) {
         if (virDomainMigrateSetCompressionCache(dom, size, 0) < 0)
@@ -9211,10 +9183,8 @@ cmdMigrateSetMaxSpeed(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return false;
 
-    if (vshCommandOptUL(cmd, "bandwidth", &bandwidth) < 0) {
-        vshError(ctl, "%s", _("migrate: Invalid bandwidth"));
+    if (vshCommandOptUL(ctl, cmd, "bandwidth", &bandwidth) < 0)
         goto done;
-    }
 
     if (virDomainMigrateSetMaxSpeed(dom, bandwidth, 0) < 0)
         goto done;
@@ -11371,10 +11341,8 @@ cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return ret;
 
-    if (vshCommandOptULongLong(cmd, "minimum", &minimum) < 0) {
-        vshError(ctl, _("Unable to parse integer parameter minimum"));
+    if (vshCommandOptULongLong(ctl, cmd, "minimum", &minimum) < 0)
         goto cleanup;
-    }
 
     if (vshCommandOptStringReq(ctl, cmd, "mountpoint", &mountPoint) < 0)
         goto cleanup;
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index cac6086..61be4c0 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -111,10 +111,8 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
 
     VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno);
 
-    if (cellno && vshCommandOptInt(cmd, "cellno", &cell) < 0) {
-        vshError(ctl, "%s", _("cell number has to be a number"));
+    if (cellno && vshCommandOptInt(ctl, cmd, "cellno", &cell) < 0)
         return false;
-    }
 
     if (all) {
         if (!(cap_xml = virConnectGetCapabilities(ctl->conn))) {
@@ -372,10 +370,8 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd)
     unsigned long long cpu_stats[VSH_CPU_LAST] = { 0 };
     bool present[VSH_CPU_LAST] = { false };
 
-    if (vshCommandOptInt(cmd, "cpu", &cpuNum) < 0) {
-        vshError(ctl, "%s", _("Invalid value of cpuNum"));
+    if (vshCommandOptInt(ctl, cmd, "cpu", &cpuNum) < 0)
         return false;
-    }
 
     if (virNodeGetCPUStats(ctl->conn, cpuNum, NULL, &nparams, 0) != 0) {
         vshError(ctl, "%s",
@@ -481,10 +477,8 @@ cmdNodeMemStats(vshControl *ctl, const vshCmd *cmd)
     virNodeMemoryStatsPtr params = NULL;
     bool ret = false;
 
-    if (vshCommandOptInt(cmd, "cell", &cellNum) < 0) {
-        vshError(ctl, "%s", _("Invalid value of cellNum"));
+    if (vshCommandOptInt(ctl, cmd, "cell", &cellNum) < 0)
         return false;
-    }
 
     /* get the number of memory parameters */
     if (virNodeGetMemoryStats(ctl->conn, cellNum, NULL, &nparams, 0) != 0) {
@@ -555,10 +549,8 @@ cmdNodeSuspend(vshControl *ctl, const vshCmd *cmd)
     if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0)
         return false;
 
-    if (vshCommandOptLongLong(cmd, "duration", &duration) < 0) {
-        vshError(ctl, _("Invalid duration argument"));
+    if (vshCommandOptLongLong(ctl, cmd, "duration", &duration) < 0)
         return false;
-    }
 
     if (STREQ(target, "mem"))
         suspendTarget = VIR_NODE_SUSPEND_TARGET_MEM;
@@ -862,8 +854,7 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
     int rc = -1;
     size_t i;
 
-    if ((rc = vshCommandOptUInt(cmd, "shm-pages-to-scan", &value)) < 0) {
-        vshError(ctl, "%s", _("invalid shm-pages-to-scan number"));
+    if ((rc = vshCommandOptUInt(ctl, cmd, "shm-pages-to-scan", &value)) < 0) {
         goto cleanup;
     } else if (rc > 0) {
         if (virTypedParamsAddUInt(&params, &nparams, &maxparams,
@@ -872,8 +863,7 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
             goto save_error;
     }
 
-    if ((rc = vshCommandOptUInt(cmd, "shm-sleep-millisecs", &value)) < 0) {
-        vshError(ctl, "%s", _("invalid shm-sleep-millisecs number"));
+    if ((rc = vshCommandOptUInt(ctl, cmd, "shm-sleep-millisecs", &value)) < 0) {
         goto cleanup;
     } else if (rc > 0) {
         if (virTypedParamsAddUInt(&params, &nparams, &maxparams,
@@ -882,8 +872,7 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
             goto save_error;
     }
 
-    if ((rc = vshCommandOptUInt(cmd, "shm-merge-across-nodes", &value)) < 0) {
-        vshError(ctl, "%s", _("invalid shm-merge-across-nodes number"));
+    if ((rc = vshCommandOptUInt(ctl, cmd, "shm-merge-across-nodes", &value)) < 0) {
         goto cleanup;
     } else if (rc > 0) {
         if (virTypedParamsAddUInt(&params, &nparams, &maxparams,
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index d4ec854..4dfd4ae 100644
--- a/tools/virsh-interface.c
+++ b/tools/virsh-interface.c
@@ -844,10 +844,8 @@ cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd)
     /* use "no-stp" because we want "stp" to default true */
     stp = !vshCommandOptBool(cmd, "no-stp");
 
-    if (vshCommandOptUInt(cmd, "delay", &delay) < 0) {
-        vshError(ctl, "%s", _("Unable to parse delay parameter"));
+    if (vshCommandOptUInt(ctl, cmd, "delay", &delay) < 0)
         goto cleanup;
-    }
 
     nostart = vshCommandOptBool(cmd, "no-start");
 
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 4b0df62..1f42b0d 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -943,10 +943,8 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd)
         goto cleanup;
     }
 
-    if (vshCommandOptInt(cmd, "parent-index", &parentIndex) < 0) {
-        vshError(ctl, "%s", _("malformed parent-index argument"));
+    if (vshCommandOptInt(ctl, cmd, "parent-index", &parentIndex) < 0)
         goto cleanup;
-    }
 
     /* The goal is to have a full xml element in the "xml"
      * string. This is provided in the --xml option, either directly
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 55bf6f0..24a2b14 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -658,15 +658,11 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
     const char *name = NULL;
     unsigned long long offset = 0, length = 0;
 
-    if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) {
-        vshError(ctl, _("Unable to parse integer"));
+    if (vshCommandOptULongLong(ctl, cmd, "offset", &offset) < 0)
         return false;
-    }
 
-    if (vshCommandOptULongLong(cmd, "length", &length) < 0) {
-        vshError(ctl, _("Unable to parse integer"));
+    if (vshCommandOptULongLong(ctl, cmd, "length", &length) < 0)
         return false;
-    }
 
     if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name))) {
         return false;
@@ -768,15 +764,11 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
     unsigned long long offset = 0, length = 0;
     bool created = false;
 
-    if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) {
-        vshError(ctl, _("Unable to parse integer"));
+    if (vshCommandOptULongLong(ctl, cmd, "offset", &offset) < 0)
         return false;
-    }
 
-    if (vshCommandOptULongLong(cmd, "length", &length) < 0) {
-        vshError(ctl, _("Unable to parse integer"));
+    if (vshCommandOptULongLong(ctl, cmd, "length", &length) < 0)
         return false;
-    }
 
     if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name)))
         return false;
diff --git a/tools/virsh.c b/tools/virsh.c
index 28af3c3..b2edea3 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1469,6 +1469,7 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
 
 /**
  * vshCommandOptInt:
+ * @ctl virsh control structure
  * @cmd command reference
  * @name option name
  * @value result
@@ -1480,7 +1481,10 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt,
  * <0 in all other cases (@value untouched)
  */
 int
-vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
+vshCommandOptInt(vshControl *ctl,
+                 const vshCmd *cmd,
+                 const char *name,
+                 int *value)
 {
     vshCmdOpt *arg;
     int ret;
@@ -1489,14 +1493,19 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
     if (ret <= 0)
         return ret;
 
-    if (virStrToLong_i(arg->data, NULL, 10, value) < 0)
+    if (virStrToLong_i(arg->data, NULL, 10, value) < 0) {
+        vshError(ctl,
+                 _("Unable to parse integer parameter to --%s"),
+                 name);
         return -1;
+    }
     return 1;
 }
 
 
 /**
  * vshCommandOptUInt:
+ * @ctl virsh control structure
  * @cmd command reference
  * @name option name
  * @value result
@@ -1505,7 +1514,10 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
  * See vshCommandOptInt()
  */
 int
-vshCommandOptUInt(const vshCmd *cmd, const char *name, unsigned int *value)
+vshCommandOptUInt(vshControl *ctl,
+                  const vshCmd *cmd,
+                  const char *name,
+                  unsigned int *value)
 {
     vshCmdOpt *arg;
     int ret;
@@ -1514,14 +1526,19 @@ vshCommandOptUInt(const vshCmd *cmd, const char *name, unsigned int *value)
     if (ret <= 0)
         return ret;
 
-    if (virStrToLong_ui(arg->data, NULL, 10, value) < 0)
+    if (virStrToLong_ui(arg->data, NULL, 10, value) < 0) {
+        vshError(ctl,
+                 _("Unable to parse integer parameter to --%s"),
+                 name);
         return -1;
+    }
     return 1;
 }
 
 
 /*
  * vshCommandOptUL:
+ * @ctl virsh control structure
  * @cmd command reference
  * @name option name
  * @value result
@@ -1530,7 +1547,10 @@ vshCommandOptUInt(const vshCmd *cmd, const char *name, unsigned int *value)
  * See vshCommandOptInt()
  */
 int
-vshCommandOptUL(const vshCmd *cmd, const char *name, unsigned long *value)
+vshCommandOptUL(vshControl *ctl,
+                const vshCmd *cmd,
+                const char *name,
+                unsigned long *value)
 {
     vshCmdOpt *arg;
     int ret;
@@ -1539,8 +1559,12 @@ vshCommandOptUL(const vshCmd *cmd, const char *name, unsigned long *value)
     if (ret <= 0)
         return ret;
 
-    if (virStrToLong_ul(arg->data, NULL, 10, value) < 0)
+    if (virStrToLong_ul(arg->data, NULL, 10, value) < 0) {
+        vshError(ctl,
+                 _("Unable to parse integer parameter to --%s"),
+                 name);
         return -1;
+    }
     return 1;
 }
 
@@ -1620,6 +1644,7 @@ vshCommandOptStringReq(vshControl *ctl,
 
 /**
  * vshCommandOptLongLong:
+ * @ctl virsh control structure
  * @cmd command reference
  * @name option name
  * @value result
@@ -1628,7 +1653,9 @@ vshCommandOptStringReq(vshControl *ctl,
  * See vshCommandOptInt()
  */
 int
-vshCommandOptLongLong(const vshCmd *cmd, const char *name,
+vshCommandOptLongLong(vshControl *ctl,
+                      const vshCmd *cmd,
+                      const char *name,
                       long long *value)
 {
     vshCmdOpt *arg;
@@ -1638,13 +1665,18 @@ vshCommandOptLongLong(const vshCmd *cmd, const char *name,
     if (ret <= 0)
         return ret;
 
-    if (virStrToLong_ll(arg->data, NULL, 10, value) < 0)
+    if (virStrToLong_ll(arg->data, NULL, 10, value) < 0) {
+        vshError(ctl,
+                 _("Unable to parse integer parameter to --%s"),
+                 name);
         return -1;
+    }
     return 1;
 }
 
 /**
  * vshCommandOptULongLong:
+ * @ctl virsh control structure
  * @cmd command reference
  * @name option name
  * @value result
@@ -1653,7 +1685,9 @@ vshCommandOptLongLong(const vshCmd *cmd, const char *name,
  * See vshCommandOptInt()
  */
 int
-vshCommandOptULongLong(const vshCmd *cmd, const char *name,
+vshCommandOptULongLong(vshControl *ctl,
+                       const vshCmd *cmd,
+                       const char *name,
                        unsigned long long *value)
 {
     vshCmdOpt *arg;
@@ -1663,14 +1697,19 @@ vshCommandOptULongLong(const vshCmd *cmd, const char *name,
     if (ret <= 0)
         return ret;
 
-    if (virStrToLong_ull(arg->data, NULL, 10, value) < 0)
+    if (virStrToLong_ull(arg->data, NULL, 10, value) < 0) {
+        vshError(ctl,
+                 _("Unable to parse integer parameter to --%s"),
+                 name);
         return -1;
+    }
     return 1;
 }
 
 
 /**
  * vshCommandOptScaledInt:
+ * @ctl virsh control structure
  * @cmd command reference
  * @name option name
  * @value result
@@ -1681,8 +1720,11 @@ vshCommandOptULongLong(const vshCmd *cmd, const char *name,
  * See vshCommandOptInt()
  */
 int
-vshCommandOptScaledInt(const vshCmd *cmd, const char *name,
-                       unsigned long long *value, int scale,
+vshCommandOptScaledInt(vshControl *ctl,
+                       const vshCmd *cmd,
+                       const char *name,
+                       unsigned long long *value,
+                       int scale,
                        unsigned long long max)
 {
     const char *str;
@@ -1692,9 +1734,18 @@ vshCommandOptScaledInt(const vshCmd *cmd, const char *name,
     ret = vshCommandOptString(cmd, name, &str);
     if (ret <= 0)
         return ret;
-    if (virStrToLong_ull(str, &end, 10, value) < 0 ||
-        virScaleInteger(value, end, scale, max) < 0)
+    if (virStrToLong_ull(str, &end, 10, value) < 0) {
+        vshError(ctl,
+                 _("Unable to parse integer parameter to --%s"),
+                 name);
         return -1;
+    }
+
+    if (virScaleInteger(value, end, scale, max) < 0) {
+        vshError(ctl, "%s",
+                 _("value falls out of range"));
+        return -1;
+    }
     return 1;
 }
 
@@ -1771,20 +1822,22 @@ vshCmdHasOption(vshControl *ctl, const vshCmd *cmd, const char *optname)
 int
 vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, int *timeout)
 {
-    int rv = vshCommandOptInt(cmd, "timeout", timeout);
+    int rv = vshCommandOptInt(ctl, cmd, "timeout", timeout);
 
-    if (rv < 0 || (rv > 0 && *timeout < 1)) {
+    if (rv <= 0)
+        return rv;
+
+    if (*timeout < 1) {
         vshError(ctl, "%s", _("invalid timeout"));
         return -1;
     }
-    if (rv > 0) {
-        /* Ensure that we can multiply by 1000 without overflowing. */
-        if (*timeout > INT_MAX / 1000) {
-            vshError(ctl, "%s", _("timeout is too big"));
-            return -1;
-        }
-        *timeout *= 1000;
+
+    /* Ensure that we can multiply by 1000 without overflowing. */
+    if (*timeout > INT_MAX / 1000) {
+        vshError(ctl, "%s", _("timeout is too big"));
+        return -1;
     }
+    *timeout *= 1000;
     return rv;
 }
 
diff --git a/tools/virsh.h b/tools/virsh.h
index 3e0251b..b217206 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -168,7 +168,7 @@ struct _vshCmdInfo {
 struct _vshCmdOptDef {
     const char *name;           /* the name of option, or NULL for list end */
     vshCmdOptType type;         /* option type */
-    unsigned int flags;         /* flags */
+    unsigned int flags;         /* bitwise OR of VSH_OFLAG_**/
     const char *help;           /* non-NULL help string; or for VSH_OT_ALIAS
                                  * the name of a later public option */
     vshCompleter completer;         /* option completer */
@@ -280,13 +280,14 @@ bool vshCmddefHelp(vshControl *ctl, const char *name);
 const vshCmdGrp *vshCmdGrpSearch(const char *grpname);
 bool vshCmdGrpHelp(vshControl *ctl, const char *name);
 
-int vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
+int vshCommandOptInt(vshControl *ctl, const vshCmd *cmd,
+                     const char *name, int *value)
     ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
-int vshCommandOptUInt(const vshCmd *cmd, const char *name,
-                      unsigned int *value)
+int vshCommandOptUInt(vshControl *ctl, const vshCmd *cmd,
+                      const char *name, unsigned int *value)
     ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
-int vshCommandOptUL(const vshCmd *cmd, const char *name,
-                    unsigned long *value)
+int vshCommandOptUL(vshControl *ctl, const vshCmd *cmd,
+                    const char *name, unsigned long *value)
     ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
 int vshCommandOptString(const vshCmd *cmd, const char *name,
                         const char **value)
@@ -295,13 +296,14 @@ int vshCommandOptStringReq(vshControl *ctl, const vshCmd *cmd,
                            const char *name, const char **value)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
     ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK;
-int vshCommandOptLongLong(const vshCmd *cmd, const char *name,
-                          long long *value)
+int vshCommandOptLongLong(vshControl *ctl, const vshCmd *cmd,
+                          const char *name, long long *value)
     ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
-int vshCommandOptULongLong(const vshCmd *cmd, const char *name,
-                           unsigned long long *value)
+int vshCommandOptULongLong(vshControl *ctl, const vshCmd *cmd,
+                           const char *name, unsigned long long *value)
     ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
-int vshCommandOptScaledInt(const vshCmd *cmd, const char *name,
+int vshCommandOptScaledInt(vshControl *ctl,
+                           const vshCmd *cmd, const char *name,
                            unsigned long long *value, int scale,
                            unsigned long long max)
     ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
-- 
1.9.0




More information about the libvir-list mailing list