[libvirt] [PATCH 1/3] virsh: Improve error message on integer value parsing failure.

Andrea Bolognani abologna at redhat.com
Fri May 15 16:14:39 UTC 2015


Replace more than 30 ad-hoc error messages with a single, generic one
that contains the name of the option being processed and some hints
to help the user understand what could have gone wrong.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1207043
---
 tests/vcpupin                |   4 +-
 tools/virsh-domain-monitor.c |  10 +++--
 tools/virsh-domain.c         | 102 ++++++++++++++++++++++++++++++++-----------
 tools/virsh-host.c           |  44 ++++++++++++++-----
 tools/virsh-interface.c      |   4 +-
 tools/virsh-network.c        |   4 +-
 tools/virsh-volume.c         |  16 +++++--
 7 files changed, 135 insertions(+), 49 deletions(-)

diff --git a/tests/vcpupin b/tests/vcpupin
index cd09145..ab0d38f 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 vCPU number.
+error: Numeric value for <vcpu> option is malformed or out of range
 
 EOF
 compare exp out || fail=1
@@ -52,7 +52,7 @@ compare exp out || fail=1
 $abs_top_builddir/tools/virsh --connect test:///default vcpupin test -100 0,1 > out 2>&1
 test $? = 1 || fail=1
 cat <<\EOF > exp || fail=1
-error: vcpupin: Invalid vCPU number.
+error: Numeric value for <vcpu> option is malformed or out of range
 
 EOF
 compare exp out || fail=1
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 91c57e2..51276d3 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -341,8 +341,9 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
      * This is not really an unsigned long, but it
      */
     if ((rv = vshCommandOptInt(cmd, "period", &period)) < 0) {
-        vshError(ctl, "%s",
-                 _("Unable to parse integer parameter."));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "period");
         goto cleanup;
     }
     if (rv > 0) {
@@ -1439,8 +1440,9 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd)
 
     if (rv < 0) {
         /* invalid integer format */
-        vshError(ctl, "%s",
-                 _("Unable to parse integer parameter to --time."));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "time");
         goto cleanup;
     } else if (rv > 0) {
         /* valid integer to set */
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 20f8c75..10d01b6 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1552,7 +1552,9 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
         return false;
 
     if ((rv = vshCommandOptInt(cmd, "weight", &weight)) < 0) {
-        vshError(ctl, "%s", _("Unable to parse integer parameter"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "weight");
         goto cleanup;
     } else if (rv > 0) {
         if (weight <= 0) {
@@ -1691,7 +1693,9 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
         goto cleanup;
 
     if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) {
-        vshError(ctl, "%s", _("bandwidth must be a number"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "bandwidth");
         goto cleanup;
     }
 
@@ -2213,15 +2217,21 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
      * than trying to guess which value will work well across both
      * APIs with their different sizes and scales.  */
     if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) {
-        vshError(ctl, "%s", _("bandwidth must be a number"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "bandwidth");
         goto cleanup;
     }
     if (vshCommandOptUInt(cmd, "granularity", &granularity) < 0) {
-        vshError(ctl, "%s", _("granularity must be a number"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "granularity");
         goto cleanup;
     }
     if (vshCommandOptULongLong(cmd, "buf-size", &buf_size) < 0) {
-        vshError(ctl, "%s", _("buf-size must be a number"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "buf-size");
         goto cleanup;
     }
 
@@ -2791,7 +2801,9 @@ cmdBlockResize(vshControl *ctl, const vshCmd *cmd)
         return false;
 
     if (vshCommandOptScaledInt(cmd, "size", &size, 1024, ULLONG_MAX) < 0) {
-        vshError(ctl, "%s", _("Unable to parse integer"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "size");
         return false;
     }
 
@@ -3395,7 +3407,9 @@ cmdDomPMSuspend(vshControl *ctl, const vshCmd *cmd)
         return false;
 
     if (vshCommandOptULongLong(cmd, "duration", &duration) < 0) {
-        vshError(ctl, _("Invalid duration argument"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "duration");
         goto cleanup;
     }
 
@@ -5317,7 +5331,9 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd)
         return false;
 
     if (vshCommandOptUInt(cmd, "screen", &screen) < 0) {
-        vshError(ctl, "%s", _("invalid screen ID"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "screen");
         return false;
     }
 
@@ -6414,7 +6430,9 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
         VSH_EXCLUSIVE_OPTIONS_VAR(live, config);
 
     if ((got_vcpu = vshCommandOptUInt(cmd, "vcpu", &vcpu)) < 0) {
-        vshError(ctl, "%s", _("vcpupin: Invalid vCPU number."));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "vcpu");
         return false;
     }
 
@@ -6681,7 +6699,9 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
         return false;
 
     if (vshCommandOptInt(cmd, "count", &count) < 0 || count <= 0) {
-        vshError(ctl, "%s", _("Invalid number of virtual CPUs"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "count");
         goto cleanup;
     }
 
@@ -6859,7 +6879,9 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd)
         return false;
 
     if (vshCommandOptUInt(cmd, "iothread", &iothread_id) < 0) {
-        vshError(ctl, "%s", _("iothreadpin: Invalid IOThread number."));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "iothread");
         goto cleanup;
     }
 
@@ -6948,7 +6970,9 @@ cmdIOThreadAdd(vshControl *ctl, const vshCmd *cmd)
         return false;
 
     if (vshCommandOptInt(cmd, "id", &iothread_id) < 0) {
-        vshError(ctl, "%s", _("Unable to parse integer parameter"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "id");
         goto cleanup;
     }
     if (iothread_id <= 0) {
@@ -7028,7 +7052,9 @@ cmdIOThreadDel(vshControl *ctl, const vshCmd *cmd)
         return false;
 
     if (vshCommandOptInt(cmd, "id", &iothread_id) < 0) {
-        vshError(ctl, "%s", _("Unable to parse integer parameter"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "id");
         goto cleanup;
     }
     if (iothread_id <= 0) {
@@ -7315,7 +7341,9 @@ 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"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "start");
         goto cleanup;
     } else if (rv > 0) {
         if (cpu < 0) {
@@ -7326,8 +7354,9 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd)
     }
 
     if ((rv = vshCommandOptInt(cmd, "count", &show_count)) < 0) {
-        vshError(ctl, "%s",
-                 _("Unable to parse integer parameter for CPUs to show"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "count");
         goto cleanup;
     } else if (rv > 0) {
         if (show_count < 0) {
@@ -8115,7 +8144,9 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd)
         codeset_option = "linux";
 
     if (vshCommandOptUInt(cmd, "holdtime", &holdtime) < 0) {
-        vshError(ctl, _("invalid value of --holdtime"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "holdtime");
         goto cleanup;
     }
 
@@ -8342,7 +8373,9 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd)
     else
         max = ULONG_MAX;
     if (vshCommandOptScaledInt(cmd, "size", &bytes, 1024, max) < 0) {
-        vshError(ctl, "%s", _("memory size has to be a number"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "size");
         virDomainFree(dom);
         return false;
     }
@@ -8437,7 +8470,9 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
     else
         max = ULONG_MAX;
     if (vshCommandOptScaledInt(cmd, "size", &bytes, 1024, max) < 0) {
-        vshError(ctl, "%s", _("memory size has to be a number"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "size");
         virDomainFree(dom);
         return false;
     }
@@ -9071,7 +9106,9 @@ cmdQemuAttach(vshControl *ctl, const vshCmd *cmd)
     unsigned int pid_value; /* API uses unsigned int, not pid_t */
 
     if (vshCommandOptUInt(cmd, "pid", &pid_value) <= 0) {
-        vshError(ctl, "%s", _("missing pid value"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "pid");
         goto cleanup;
     }
 
@@ -9166,7 +9203,9 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd)
 
     judge = vshCommandOptInt(cmd, "timeout", &timeout);
     if (judge < 0) {
-        vshError(ctl, "%s", _("timeout number has to be a number"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "timeout");
         goto cleanup;
     } else if (judge > 0) {
         judge = 1;
@@ -10048,8 +10087,13 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
         return false;
 
-    if (vshCommandOptLongLong(cmd, "downtime", &downtime) < 0 ||
-        downtime < 1) {
+    if (vshCommandOptLongLong(cmd, "downtime", &downtime) < 0) {
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "downtime");
+        goto done;
+    }
+    if (downtime < 1) {
         vshError(ctl, "%s", _("migrate: Invalid downtime"));
         goto done;
     }
@@ -10107,7 +10151,9 @@ cmdMigrateCompCache(vshControl *ctl, const vshCmd *cmd)
 
     rc = vshCommandOptULongLong(cmd, "size", &size);
     if (rc < 0) {
-        vshError(ctl, "%s", _("Unable to parse size parameter"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "size");
         goto cleanup;
     } else if (rc != 0) {
         if (virDomainMigrateSetCompressionCache(dom, size, 0) < 0)
@@ -10165,7 +10211,9 @@ cmdMigrateSetMaxSpeed(vshControl *ctl, const vshCmd *cmd)
         return false;
 
     if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) {
-        vshError(ctl, "%s", _("migrate: Invalid bandwidth"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "bandwidth");
         goto done;
     }
 
@@ -12503,7 +12551,9 @@ cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd)
         return ret;
 
     if (vshCommandOptULongLong(cmd, "minimum", &minimum) < 0) {
-        vshError(ctl, _("Unable to parse integer parameter minimum"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "minimum");
         goto cleanup;
     }
 
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 20765e5..a72fd05 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -177,7 +177,9 @@ 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"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "cellno");
         return false;
     }
 
@@ -310,7 +312,9 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
     VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno);
 
     if (vshCommandOptScaledInt(cmd, "pagesize", &bytes, 1024, UINT_MAX) < 0) {
-        vshError(ctl, "%s", _("page size has to be a number"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "pagesize");
         goto cleanup;
     }
     kibibytes = VIR_DIV_UP(bytes, 1024);
@@ -388,7 +392,9 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
         }
 
         if (vshCommandOptInt(cmd, "cellno", &cell) < 0) {
-            vshError(ctl, "%s", _("Invalid cellno argument"));
+            vshError(ctl,
+                     _("Numeric value for <%s> option is malformed or out of range"),
+                     "cellno");
             goto cleanup;
         }
 
@@ -485,12 +491,16 @@ cmdAllocpages(vshControl *ctl, const vshCmd *cmd)
     VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno);
 
     if (cellno && vshCommandOptInt(cmd, "cellno", &startCell) < 0) {
-        vshError(ctl, "%s", _("cell number has to be a number"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "cellno");
         return false;
     }
 
     if (vshCommandOptScaledInt(cmd, "pagesize", &tmp, 1024, UINT_MAX) < 0) {
-        vshError(ctl, "%s", _("pagesize has to be a number"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "cellno");
         return false;
     }
     pageSizes[0] = VIR_DIV_UP(tmp, 1024);
@@ -755,7 +765,9 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd)
     bool present[VSH_CPU_LAST] = { false };
 
     if (vshCommandOptInt(cmd, "cpu", &cpuNum) < 0) {
-        vshError(ctl, "%s", _("Invalid value of cpuNum"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "cpu");
         return false;
     }
 
@@ -864,7 +876,9 @@ cmdNodeMemStats(vshControl *ctl, const vshCmd *cmd)
     bool ret = false;
 
     if (vshCommandOptInt(cmd, "cell", &cellNum) < 0) {
-        vshError(ctl, "%s", _("Invalid value of cellNum"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "cell");
         return false;
     }
 
@@ -938,7 +952,9 @@ cmdNodeSuspend(vshControl *ctl, const vshCmd *cmd)
         return false;
 
     if (vshCommandOptLongLong(cmd, "duration", &duration) < 0) {
-        vshError(ctl, _("Invalid duration argument"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "duration");
         return false;
     }
 
@@ -1245,7 +1261,9 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
     size_t i;
 
     if ((rc = vshCommandOptUInt(cmd, "shm-pages-to-scan", &value)) < 0) {
-        vshError(ctl, "%s", _("invalid shm-pages-to-scan number"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "shm-pages-to-scan");
         goto cleanup;
     } else if (rc > 0) {
         if (virTypedParamsAddUInt(&params, &nparams, &maxparams,
@@ -1255,7 +1273,9 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
     }
 
     if ((rc = vshCommandOptUInt(cmd, "shm-sleep-millisecs", &value)) < 0) {
-        vshError(ctl, "%s", _("invalid shm-sleep-millisecs number"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "shm-sleep-millisecs");
         goto cleanup;
     } else if (rc > 0) {
         if (virTypedParamsAddUInt(&params, &nparams, &maxparams,
@@ -1265,7 +1285,9 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
     }
 
     if ((rc = vshCommandOptUInt(cmd, "shm-merge-across-nodes", &value)) < 0) {
-        vshError(ctl, "%s", _("invalid shm-merge-across-nodes number"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "shm-merge-across-nodes");
         goto cleanup;
     } else if (rc > 0) {
         if (virTypedParamsAddUInt(&params, &nparams, &maxparams,
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index b9c60c5..7122bc5 100644
--- a/tools/virsh-interface.c
+++ b/tools/virsh-interface.c
@@ -844,7 +844,9 @@ cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd)
     stp = !vshCommandOptBool(cmd, "no-stp");
 
     if (vshCommandOptUInt(cmd, "delay", &delay) < 0) {
-        vshError(ctl, "%s", _("Unable to parse delay parameter"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "delay");
         goto cleanup;
     }
 
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index b859b49..a45367a 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -937,7 +937,9 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd)
     }
 
     if (vshCommandOptInt(cmd, "parent-index", &parentIndex) < 0) {
-        vshError(ctl, "%s", _("malformed parent-index argument"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "parent-index");
         goto cleanup;
     }
 
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index db94154..5d6cc6a 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -694,12 +694,16 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
     unsigned long long offset = 0, length = 0;
 
     if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) {
-        vshError(ctl, _("Unable to parse offset value"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "offset");
         return false;
     }
 
     if (vshCommandOptULongLongWrap(cmd, "length", &length) < 0) {
-        vshError(ctl, _("Unable to parse length value"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "length");
         return false;
     }
 
@@ -803,12 +807,16 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
     bool created = false;
 
     if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) {
-        vshError(ctl, _("Unable to parse offset value"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "offset");
         return false;
     }
 
     if (vshCommandOptULongLongWrap(cmd, "length", &length) < 0) {
-        vshError(ctl, _("Unable to parse length value"));
+        vshError(ctl,
+                 _("Numeric value for <%s> option is malformed or out of range"),
+                 "length");
         return false;
     }
 
-- 
2.1.0




More information about the libvir-list mailing list