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

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(a)redhat.com&gt; --- 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