[libvirt] [PATCH v4 0/6] virsh: Further improve handling of integer options

As suggested by Michal: now that we have a generic error message for failures related to the parsing of integer options, it makes sense to perform the corresponding check in a single spot instead of replicating it every time vshCommandOpt*() is used. Andrea Bolognani (6): tests: Add a bunch of new tests to virsh-optparse virsh: Use standard error messages in vshCommandOptTimeoutToMs() virsh: Improve vshCommandOptTimeoutToMs() virsh: Make vshCommandOptScaledInt() use vshCommandOpt() virsh: Pass vshControl to all vshCommandOpt*() calls virsh: Move error messages inside vshCommandOpt*() functions tests/vcpupin | 4 +- tests/virsh-optparse | 179 ++++++++++++++++++++++++++++++ tools/virsh-domain-monitor.c | 17 +-- tools/virsh-domain.c | 226 ++++++++++++-------------------------- tools/virsh-host.c | 67 +++--------- tools/virsh-interface.c | 6 +- tools/virsh-network.c | 10 +- tools/virsh-nodedev.c | 4 +- tools/virsh-snapshot.c | 2 +- tools/virsh-volume.c | 26 +---- tools/virsh.c | 252 ++++++++++++++++++++++++++----------------- tools/virsh.h | 66 ++++++------ 12 files changed, 461 insertions(+), 398 deletions(-) -- 2.1.0

The new tests deal with numeric options of three kinds: regular, scaled and timeouts. For each, both valid and invalid inputs are provided, hopefully covering all cases: this should allow us to avoid regressions when changing the relevant code in virsh. --- tests/virsh-optparse | 179 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 179 insertions(+) diff --git a/tests/virsh-optparse b/tests/virsh-optparse index 39e6cde..37a8d42 100755 --- a/tests/virsh-optparse +++ b/tests/virsh-optparse @@ -137,4 +137,183 @@ virsh -q -c $test_url qemu-monitor-command test a >out 2>err && fail=1 test -s out && fail=1 compare exp-err err || fail=1 +### Test a regular numeric option + +# Non-numeric value +cat <<\EOF > exp-err || framework_failure +error: Numeric value for <start> option is malformed or out of range +EOF +virsh -q -c $test_url cpu-stats test --start abc >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Numeric value with invalid suffix +cat <<\EOF > exp-err || framework_failure +error: Numeric value for <start> option is malformed or out of range +EOF +virsh -q -c $test_url cpu-stats test --start 42WB >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Numeric value with valid suffix. Suffixes are not supported for +# regular numeric options, so this value is rejected +cat <<\EOF > exp-err || framework_failure +error: Numeric value for <start> option is malformed or out of range +EOF +virsh -q -c $test_url cpu-stats test --start 42MB >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Numeric value bigger than INT_MAX +cat <<\EOF > exp-err || framework_failure +error: Numeric value for <start> option is malformed or out of range +EOF +virsh -q -c $test_url cpu-stats test --start 2147483648 >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Negative numeric value. The value is not valid for the command +# we're testing, but it has been parsed correctly +cat <<\EOF > exp-err || framework_failure +error: Invalid value for start CPU +EOF +virsh -q -c $test_url cpu-stats test --start -1 >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Zero. The test driver doesn't support the operation so the command +# fails, but the value has been parsed correctly +cat <<\EOF > exp-err || framework_failure +error: Failed to retrieve CPU statistics for domain 'test' +error: this function is not supported by the connection driver: virDomainGetCPUStats +EOF +virsh -q -c $test_url cpu-stats test --start 0 >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Valid numeric value. The test driver doesn't support the operation +# so the command fails, but the value has been parsed correctly +cat <<\EOF > exp-err || framework_failure +error: Failed to retrieve CPU statistics for domain 'test' +error: this function is not supported by the connection driver: virDomainGetCPUStats +EOF +virsh -q -c $test_url cpu-stats test --start 42 >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +### Test a scaled numeric option + +# Non-numeric value +cat <<\EOF > exp-err || framework_failure +error: Numeric value for <size> option is malformed or out of range +EOF +virsh -q -c $test_url setmaxmem test abc >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Numeric value with invalid suffix +cat <<\EOF > exp-err || framework_failure +error: Numeric value for <size> option is malformed or out of range +EOF +virsh -q -c $test_url setmaxmem test 42WB >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Numeric value with valid suffix +virsh -q -c $test_url setmaxmem test 42MB >out 2>err || fail=1 +test -s out && fail=1 +test -s err && fail=1 + +# Numeric value bigger than INT_MAX. No failure here because +# scaled numeric values are unsigned long long +virsh -q -c $test_url setmaxmem test 2147483648 >out 2>err || fail=1 +test -s out && fail=1 +test -s err && fail=1 + +# Negative numeric value +cat <<\EOF > exp-err || framework_failure +error: Numeric value for <size> option is malformed or out of range +EOF +virsh -q -c $test_url setmaxmem test -1 >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Zero. The value is not valid for the command we're testing, but +# it has been parsed correctly +cat <<\EOF > exp-err || framework_failure +error: Unable to change MaxMemorySize +error: memory in virDomainSetMaxMemory must not be zero +EOF +virsh -q -c $test_url setmaxmem test 0 >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Numeric value +virsh -q -c $test_url setmaxmem test 42 >out 2>err || fail=1 +test -s out && fail=1 +test -s err && fail=1 + +### Test the <timeout> option (numeric option converted to ms) + +# Non-numeric value +cat <<\EOF > exp-err || framework_failure +error: invalid timeout +EOF +virsh -q -c $test_url event --all --timeout abc >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Numeric value that's too big to be converted to ms and still +# fit inside an int +cat <<\EOF > exp-err || framework_failure +error: timeout is too big +EOF +virsh -q -c $test_url event --all --timeout 2147484 >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Numeric value with invalid suffix +cat <<\EOF > exp-err || framework_failure +error: invalid timeout +EOF +virsh -q -c $test_url event --all --timeout 42WB >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Numeric value with valid suffix. Suffixes are not supported for +# the <timeout> option, so this value is rejected +cat <<\EOF > exp-err || framework_failure +error: invalid timeout +EOF +virsh -q -c $test_url event --all --timeout 42MB >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Negative value +cat <<\EOF > exp-err || framework_failure +error: invalid timeout +EOF +virsh -q -c $test_url event --all --timeout -1 >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Zero. This is not a valid timeout, but the value is parsed +# correctly +cat <<\EOF > exp-err || framework_failure +error: invalid timeout +EOF +virsh -q -c $test_url event --all --timeout 0 >out 2>err && fail=1 +test -s out && fail=1 +compare exp-err err || fail=1 + +# Numeric value. No events will be received and the command will +# fail after a second, but the value has been parsed correctly +cat <<\EOF > exp-out || framework_failure +event loop timed out +events received: 0 +EOF +virsh -q -c $test_url event --all --timeout 1 >out 2>err && fail=1 +test -s err && fail=1 +compare exp-out out || fail=1 + (exit $fail); exit $fail -- 2.1.0

I missed this in the first time around, thanks Michal for noticing. --- tests/virsh-optparse | 12 ++++++------ tools/virsh.c | 8 ++++++-- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/virsh-optparse b/tests/virsh-optparse index 37a8d42..952fcc4 100755 --- a/tests/virsh-optparse +++ b/tests/virsh-optparse @@ -257,7 +257,7 @@ test -s err && fail=1 # Non-numeric value cat <<\EOF > exp-err || framework_failure -error: invalid timeout +error: Numeric value for <timeout> option is malformed or out of range EOF virsh -q -c $test_url event --all --timeout abc >out 2>err && fail=1 test -s out && fail=1 @@ -266,7 +266,7 @@ compare exp-err err || fail=1 # Numeric value that's too big to be converted to ms and still # fit inside an int cat <<\EOF > exp-err || framework_failure -error: timeout is too big +error: Numeric value for <timeout> option is malformed or out of range EOF virsh -q -c $test_url event --all --timeout 2147484 >out 2>err && fail=1 test -s out && fail=1 @@ -274,7 +274,7 @@ compare exp-err err || fail=1 # Numeric value with invalid suffix cat <<\EOF > exp-err || framework_failure -error: invalid timeout +error: Numeric value for <timeout> option is malformed or out of range EOF virsh -q -c $test_url event --all --timeout 42WB >out 2>err && fail=1 test -s out && fail=1 @@ -283,7 +283,7 @@ compare exp-err err || fail=1 # Numeric value with valid suffix. Suffixes are not supported for # the <timeout> option, so this value is rejected cat <<\EOF > exp-err || framework_failure -error: invalid timeout +error: Numeric value for <timeout> option is malformed or out of range EOF virsh -q -c $test_url event --all --timeout 42MB >out 2>err && fail=1 test -s out && fail=1 @@ -291,7 +291,7 @@ compare exp-err err || fail=1 # Negative value cat <<\EOF > exp-err || framework_failure -error: invalid timeout +error: Numeric value for <timeout> option is malformed or out of range EOF virsh -q -c $test_url event --all --timeout -1 >out 2>err && fail=1 test -s out && fail=1 @@ -300,7 +300,7 @@ compare exp-err err || fail=1 # Zero. This is not a valid timeout, but the value is parsed # correctly cat <<\EOF > exp-err || framework_failure -error: invalid timeout +error: Numeric value for <timeout> option is malformed or out of range EOF virsh -q -c $test_url event --all --timeout 0 >out 2>err && fail=1 test -s out && fail=1 diff --git a/tools/virsh.c b/tools/virsh.c index 1005ba8..d7964a1 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1869,13 +1869,17 @@ vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, int *timeout) int rv = vshCommandOptInt(cmd, "timeout", timeout); if (rv < 0 || (rv > 0 && *timeout < 1)) { - vshError(ctl, "%s", _("invalid timeout")); + vshError(ctl, + _("Numeric value for <%s> option is malformed or out of range"), + "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")); + vshError(ctl, + _("Numeric value for <%s> option is malformed or out of range"), + "timeout"); return -1; } *timeout *= 1000; -- 2.1.0

Use vshCommandOptUInt() instead of parsing the value as a signed integer and checking whether it's positive afterwards. Improve comments as well. --- tools/virsh.c | 45 +++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index d7964a1..55caa87 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1860,33 +1860,42 @@ vshCommandOptArgv(const vshCmd *cmd, const vshCmdOpt *opt) return NULL; } -/* Parse an optional --timeout parameter in seconds, but store the - * value of the timeout in milliseconds. Return -1 on error, 0 if - * no timeout was requested, and 1 if timeout was set. */ +/* + * vshCommandOptTimeoutToMs: + * @ctl virsh control structure + * @cmd command reference + * @timeout result + * + * Parse an optional --timeout parameter in seconds, but store the + * value of the timeout in milliseconds. + * See vshCommandOptInt() + */ int vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, int *timeout) { - int rv = vshCommandOptInt(cmd, "timeout", timeout); + int ret; + unsigned int utimeout; - if (rv < 0 || (rv > 0 && *timeout < 1)) { + if ((ret = vshCommandOptUInt(cmd, "timeout", &utimeout)) < 0) vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "timeout"); - return -1; - } - if (rv > 0) { - /* Ensure that we can multiply by 1000 without overflowing. */ - if (*timeout > INT_MAX / 1000) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "timeout"); - return -1; - } - *timeout *= 1000; + if (ret <= 0) + return ret; + + /* Ensure that the timeout is not zero and that we can convert + * it from seconds to milliseconds without overflowing. */ + if (utimeout == 0 || utimeout > INT_MAX / 1000) { + vshError(ctl, + _("Numeric value for <%s> option is malformed or out of range"), + "timeout"); + ret = -1; + } else { + *timeout = ((int) utimeout) * 1000; } - return rv; -} + return ret; +} static bool vshConnectionUsability(vshControl *ctl, virConnectPtr conn) -- 2.1.0

This aligns it to the other vshCommandOpt*() functions. --- tools/virsh.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 55caa87..cd2bfef 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1804,16 +1804,16 @@ vshCommandOptScaledInt(const vshCmd *cmd, const char *name, unsigned long long *value, int scale, unsigned long long max) { - const char *str; - int ret; + vshCmdOpt *arg; char *end; + int ret; - ret = vshCommandOptString(cmd, name, &str); - if (ret <= 0) + if ((ret = vshCommandOpt(cmd, name, &arg, true)) <= 0) return ret; - if (virStrToLong_ullp(str, &end, 10, value) < 0 || + if (virStrToLong_ullp(arg->data, &end, 10, value) < 0 || virScaleInteger(value, end, scale, max) < 0) return -1; + return 1; } -- 2.1.0

This will allow us to use vshError() to report errors from inside vshCommandOpt*(), instead of replicating the same logic and error messages all over the place. We also have more context inside the vshCommandOpt*() functions, for example the actual value used on the command line, which means we can produce more detailed error messages. vshCommandOptBool() is the exception here, because it's explicitly designed not to report any error. --- tools/virsh-domain-monitor.c | 10 ++-- tools/virsh-domain.c | 128 +++++++++++++++++++++---------------------- tools/virsh-host.c | 24 ++++---- tools/virsh-interface.c | 2 +- tools/virsh-network.c | 6 +- tools/virsh-nodedev.c | 4 +- tools/virsh-snapshot.c | 2 +- tools/virsh-volume.c | 10 ++-- tools/virsh.c | 97 ++++++++++++++++++-------------- tools/virsh.h | 66 +++++++++++----------- 10 files changed, 183 insertions(+), 166 deletions(-) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index a42c150..d8b217b 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -340,7 +340,7 @@ 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) { + if ((rv = vshCommandOptInt(ctl, cmd, "period", &period)) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "period"); @@ -1436,7 +1436,7 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - rv = vshCommandOptLongLong(cmd, "time", &seconds); + rv = vshCommandOptLongLong(ctl, cmd, "time", &seconds); if (rv < 0) { /* invalid integer format */ @@ -2165,7 +2165,7 @@ cmdDomstats(vshControl *ctl, const vshCmd *cmd) goto cleanup; ndoms = 1; - while ((opt = vshCommandOptArgv(cmd, opt))) { + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { if (!(dom = vshLookupDomainBy(ctl, opt->data, VSH_BYID | VSH_BYUUID | VSH_BYNAME))) goto cleanup; @@ -2244,9 +2244,9 @@ cmdDomIfAddr(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptString(cmd, "interface", &ifacestr) < 0) + if (vshCommandOptString(ctl, cmd, "interface", &ifacestr) < 0) goto cleanup; - if (vshCommandOptString(cmd, "source", &sourcestr) < 0) + if (vshCommandOptString(ctl, cmd, "source", &sourcestr) < 0) goto cleanup; if (sourcestr) { diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index fc4d1fc..79a9a9b 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1292,7 +1292,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "device", &disk) < 0) goto cleanup; - if ((rv = vshCommandOptULongLong(cmd, "total-bytes-sec", &value)) < 0) { + if ((rv = vshCommandOptULongLong(ctl, cmd, "total-bytes-sec", &value)) < 0) { goto interror; } else if (rv > 0) { if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, @@ -1301,7 +1301,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "read-bytes-sec", &value)) < 0) { + if ((rv = vshCommandOptULongLong(ctl, cmd, "read-bytes-sec", &value)) < 0) { goto interror; } else if (rv > 0) { if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, @@ -1310,7 +1310,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "write-bytes-sec", &value)) < 0) { + if ((rv = vshCommandOptULongLong(ctl, cmd, "write-bytes-sec", &value)) < 0) { goto interror; } else if (rv > 0) { if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, @@ -1319,7 +1319,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "total-bytes-sec-max", &value)) < 0) { + if ((rv = vshCommandOptULongLong(ctl, cmd, "total-bytes-sec-max", &value)) < 0) { goto interror; } else if (rv > 0) { if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, @@ -1328,7 +1328,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "read-bytes-sec-max", &value)) < 0) { + if ((rv = vshCommandOptULongLong(ctl, cmd, "read-bytes-sec-max", &value)) < 0) { goto interror; } else if (rv > 0) { if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, @@ -1337,7 +1337,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "write-bytes-sec-max", &value)) < 0) { + if ((rv = vshCommandOptULongLong(ctl, cmd, "write-bytes-sec-max", &value)) < 0) { goto interror; } else if (rv > 0) { if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, @@ -1346,7 +1346,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "total-iops-sec", &value)) < 0) { + if ((rv = vshCommandOptULongLong(ctl, cmd, "total-iops-sec", &value)) < 0) { goto interror; } else if (rv > 0) { if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, @@ -1355,7 +1355,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "read-iops-sec", &value)) < 0) { + if ((rv = vshCommandOptULongLong(ctl, cmd, "read-iops-sec", &value)) < 0) { goto interror; } else if (rv > 0) { if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, @@ -1364,7 +1364,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "write-iops-sec", &value)) < 0) { + if ((rv = vshCommandOptULongLong(ctl, cmd, "write-iops-sec", &value)) < 0) { goto interror; } else if (rv > 0) { if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, @@ -1373,7 +1373,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "write-iops-sec-max", &value)) < 0) { + if ((rv = vshCommandOptULongLong(ctl, cmd, "write-iops-sec-max", &value)) < 0) { goto interror; } else if (rv > 0) { if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, @@ -1382,7 +1382,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "read-iops-sec-max", &value)) < 0) { + if ((rv = vshCommandOptULongLong(ctl, cmd, "read-iops-sec-max", &value)) < 0) { goto interror; } else if (rv > 0) { if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, @@ -1391,7 +1391,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "total-iops-sec-max", &value)) < 0) { + if ((rv = vshCommandOptULongLong(ctl, cmd, "total-iops-sec-max", &value)) < 0) { goto interror; } else if (rv > 0) { if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, @@ -1400,7 +1400,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rv = vshCommandOptULongLong(cmd, "size-iops-sec", &value)) < 0) { + if ((rv = vshCommandOptULongLong(ctl, cmd, "size-iops-sec", &value)) < 0) { goto interror; } else if (rv > 0) { if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams, @@ -1551,7 +1551,7 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if ((rv = vshCommandOptInt(cmd, "weight", &weight)) < 0) { + if ((rv = vshCommandOptInt(ctl, cmd, "weight", &weight)) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "weight"); @@ -1566,7 +1566,7 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) goto save_error; } - rv = vshCommandOptString(cmd, "device-weights", &device_weight); + rv = vshCommandOptString(ctl, cmd, "device-weights", &device_weight); if (rv < 0) { vshError(ctl, "%s", _("Unable to parse string parameter")); goto cleanup; @@ -1577,7 +1577,7 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) goto save_error; } - rv = vshCommandOptString(cmd, "device-read-iops-sec", &device_riops); + rv = vshCommandOptString(ctl, cmd, "device-read-iops-sec", &device_riops); if (rv < 0) { vshError(ctl, "%s", _("Unable to parse string parameter")); goto cleanup; @@ -1588,7 +1588,7 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) goto save_error; } - rv = vshCommandOptString(cmd, "device-write-iops-sec", &device_wiops); + rv = vshCommandOptString(ctl, cmd, "device-write-iops-sec", &device_wiops); if (rv < 0) { vshError(ctl, "%s", _("Unable to parse string parameter")); goto cleanup; @@ -1599,7 +1599,7 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) goto save_error; } - rv = vshCommandOptString(cmd, "device-read-bytes-sec", &device_rbps); + rv = vshCommandOptString(ctl, cmd, "device-read-bytes-sec", &device_rbps); if (rv < 0) { vshError(ctl, "%s", _("Unable to parse string parameter")); goto cleanup; @@ -1610,7 +1610,7 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) goto save_error; } - rv = vshCommandOptString(cmd, "device-write-bytes-sec", &device_wbps); + rv = vshCommandOptString(ctl, cmd, "device-write-bytes-sec", &device_wbps); if (rv < 0) { vshError(ctl, "%s", _("Unable to parse string parameter")); goto cleanup; @@ -1692,7 +1692,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) goto cleanup; - if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) { + if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "bandwidth"); @@ -2161,11 +2161,11 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) return false; - if (vshCommandOptString(cmd, "dest", &dest) < 0) + if (vshCommandOptString(ctl, cmd, "dest", &dest) < 0) return false; - if (vshCommandOptString(cmd, "xml", &xml) < 0) + if (vshCommandOptString(ctl, cmd, "xml", &xml) < 0) return false; - if (vshCommandOptString(cmd, "format", &format) < 0) + if (vshCommandOptString(ctl, cmd, "format", &format) < 0) return false; VSH_EXCLUSIVE_OPTIONS_VAR(dest, xml); @@ -2216,19 +2216,19 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd) * MiB/s, and either reject negative input or treat it as 0 rather * than trying to guess which value will work well across both * APIs with their different sizes and scales. */ - if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) { + if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "bandwidth"); goto cleanup; } - if (vshCommandOptUInt(cmd, "granularity", &granularity) < 0) { + if (vshCommandOptUInt(ctl, cmd, "granularity", &granularity) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "granularity"); goto cleanup; } - if (vshCommandOptULongLong(cmd, "buf-size", &buf_size) < 0) { + if (vshCommandOptULongLong(ctl, cmd, "buf-size", &buf_size) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "buf-size"); @@ -2800,7 +2800,7 @@ 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) { + if (vshCommandOptScaledInt(ctl, cmd, "size", &size, 1024, ULLONG_MAX) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "size"); @@ -3406,7 +3406,7 @@ cmdDomPMSuspend(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (vshCommandOptULongLong(cmd, "duration", &duration) < 0) { + if (vshCommandOptULongLong(ctl, cmd, "duration", &duration) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "duration"); @@ -3587,7 +3587,7 @@ cmdUndefine(vshControl *ctl, const vshCmd *cmd) size_t i; size_t j; - ignore_value(vshCommandOptString(cmd, "storage", &vol_string)); + ignore_value(vshCommandOptString(ctl, cmd, "storage", &vol_string)); if (!(vol_string || remove_all_storage) && wipe_storage) { vshError(ctl, @@ -3966,7 +3966,7 @@ cmdStartGetFDs(vshControl *ctl, *nfdsret = 0; *fdsret = NULL; - if (vshCommandOptString(cmd, "pass-fds", &fdopt) <= 0) + if (vshCommandOptString(ctl, cmd, "pass-fds", &fdopt) <= 0) return 0; if (!(fdlist = virStringSplit(fdopt, ",", -1))) { @@ -4834,7 +4834,7 @@ cmdSchedInfoUpdate(vshControl *ctl, const vshCmd *cmd, int ret = -1; int rv; - while ((opt = vshCommandOptArgv(cmd, opt))) { + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { set_field = vshStrdup(ctl, opt->data); if (!(set_val = strchr(set_field, '='))) { vshError(ctl, "%s", _("Invalid syntax for --set, " @@ -5161,7 +5161,7 @@ doDump(void *opaque) goto out; } - if (vshCommandOptString(cmd, "format", &format)) { + if (vshCommandOptString(ctl, cmd, "format", &format)) { if (STREQ(format, "kdump-zlib")) { dumpformat = VIR_DOMAIN_CORE_DUMP_FORMAT_KDUMP_ZLIB; } else if (STREQ(format, "kdump-lzo")) { @@ -5330,7 +5330,7 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "file", (const char **) &file) < 0) return false; - if (vshCommandOptUInt(cmd, "screen", &screen) < 0) { + if (vshCommandOptUInt(ctl, cmd, "screen", &screen) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "screen"); @@ -6497,7 +6497,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) if (!cpulist) VSH_EXCLUSIVE_OPTIONS_VAR(live, config); - if ((got_vcpu = vshCommandOptUInt(cmd, "vcpu", &vcpu)) < 0) { + if ((got_vcpu = vshCommandOptUInt(ctl, cmd, "vcpu", &vcpu)) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "vcpu"); @@ -6766,7 +6766,7 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptInt(cmd, "count", &count) < 0 || count <= 0) { + if (vshCommandOptInt(ctl, cmd, "count", &count) < 0 || count <= 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "count"); @@ -6946,14 +6946,14 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptUInt(cmd, "iothread", &iothread_id) < 0) { + if (vshCommandOptUInt(ctl, cmd, "iothread", &iothread_id) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "iothread"); goto cleanup; } - if (vshCommandOptString(cmd, "cpulist", &cpulist) < 0) { + if (vshCommandOptString(ctl, cmd, "cpulist", &cpulist) < 0) { vshError(ctl, "%s", _("iothreadpin: invalid cpulist.")); goto cleanup; } @@ -7037,7 +7037,7 @@ cmdIOThreadAdd(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptInt(cmd, "id", &iothread_id) < 0) { + if (vshCommandOptInt(ctl, cmd, "id", &iothread_id) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "id"); @@ -7119,7 +7119,7 @@ cmdIOThreadDel(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptInt(cmd, "id", &iothread_id) < 0) { + if (vshCommandOptInt(ctl, cmd, "id", &iothread_id) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "id"); @@ -7408,7 +7408,7 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) show_total = vshCommandOptBool(cmd, "total"); - if ((rv = vshCommandOptInt(cmd, "start", &cpu)) < 0) { + if ((rv = vshCommandOptInt(ctl, cmd, "start", &cpu)) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "start"); @@ -7421,7 +7421,7 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) show_per_cpu = true; } - if ((rv = vshCommandOptInt(cmd, "count", &show_count)) < 0) { + if ((rv = vshCommandOptInt(ctl, cmd, "count", &show_count)) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "count"); @@ -7852,7 +7852,7 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) if ((state = vshDomainState(ctl, dom, NULL)) < 0) goto cleanup; - while ((opt = vshCommandOptArgv(cmd, opt))) { + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { if (pad) virBufferAddChar(&buf, ' '); pad = true; @@ -8208,10 +8208,10 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptString(cmd, "codeset", &codeset_option) <= 0) + if (vshCommandOptString(ctl, cmd, "codeset", &codeset_option) <= 0) codeset_option = "linux"; - if (vshCommandOptUInt(cmd, "holdtime", &holdtime) < 0) { + if (vshCommandOptUInt(ctl, cmd, "holdtime", &holdtime) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "holdtime"); @@ -8224,7 +8224,7 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - while ((opt = vshCommandOptArgv(cmd, opt))) { + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { if (count == VIR_DOMAIN_SEND_KEY_MAX_KEYS) { vshError(ctl, _("too many keycodes")); goto cleanup; @@ -8337,7 +8337,7 @@ cmdSendProcessSignal(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptLongLong(cmd, "pid", &pid_value) < 0) { + if (vshCommandOptLongLong(ctl, cmd, "pid", &pid_value) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "pid"); @@ -8438,7 +8438,7 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd) max = 1024ull * ULONG_MAX; else max = ULONG_MAX; - if (vshCommandOptScaledInt(cmd, "size", &bytes, 1024, max) < 0) { + if (vshCommandOptScaledInt(ctl, cmd, "size", &bytes, 1024, max) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "size"); @@ -8535,7 +8535,7 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) max = 1024ull * ULONG_MAX; else max = ULONG_MAX; - if (vshCommandOptScaledInt(cmd, "size", &bytes, 1024, max) < 0) { + if (vshCommandOptScaledInt(ctl, cmd, "size", &bytes, 1024, max) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "size"); @@ -8630,14 +8630,14 @@ static const vshCmdOptDef opts_memtune[] = { * <0 in all other cases */ static int -vshMemtuneGetSize(const vshCmd *cmd, const char *name, long long *value) +vshMemtuneGetSize(vshControl *ctl, const vshCmd *cmd, const char *name, long long *value) { int ret; unsigned long long tmp; const char *str; char *end; - ret = vshCommandOptString(cmd, name, &str); + ret = vshCommandOptString(ctl, cmd, name, &str); if (ret <= 0) return ret; if (virStrToLong_ll(str, &end, 10, value) < 0) @@ -8681,7 +8681,7 @@ cmdMemtune(vshControl *ctl, const vshCmd *cmd) return false; #define PARSE_MEMTUNE_PARAM(NAME, FIELD) \ - if ((rc = vshMemtuneGetSize(cmd, NAME, &tmpVal)) < 0) { \ + if ((rc = vshMemtuneGetSize(ctl, cmd, NAME, &tmpVal)) < 0) { \ vshError(ctl, _("Unable to parse integer parameter %s"), NAME); \ goto cleanup; \ } \ @@ -8952,7 +8952,7 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) if (dom == NULL) goto cleanup; - while ((opt = vshCommandOptArgv(cmd, opt))) { + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { if (pad) virBufferAddChar(&buf, ' '); pad = true; @@ -9100,7 +9100,7 @@ cmdQemuMonitorEvent(vshControl *ctl, const vshCmd *cmd) data.count = 0; if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0) return false; - if (vshCommandOptString(cmd, "event", &event) < 0) + if (vshCommandOptString(ctl, cmd, "event", &event) < 0) return false; if (vshCommandOptBool(cmd, "domain")) @@ -9171,7 +9171,7 @@ cmdQemuAttach(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; unsigned int pid_value; /* API uses unsigned int, not pid_t */ - if (vshCommandOptUInt(cmd, "pid", &pid_value) <= 0) { + if (vshCommandOptUInt(ctl, cmd, "pid", &pid_value) <= 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "pid"); @@ -9255,7 +9255,7 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd) if (dom == NULL) goto cleanup; - while ((opt = vshCommandOptArgv(cmd, opt))) { + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { if (pad) virBufferAddChar(&buf, ' '); pad = true; @@ -9267,7 +9267,7 @@ 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, _("Numeric value for <%s> option is malformed or out of range"), @@ -9377,7 +9377,7 @@ cmdLxcEnterNamespace(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "noseclabel")) setlabel = false; - while ((opt = vshCommandOptArgv(cmd, opt))) { + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { if (VIR_EXPAND_N(cmdargv, ncmdargv, 1) < 0) { vshError(ctl, _("%s: %d: failed to allocate argv"), __FILE__, __LINE__); @@ -10153,7 +10153,7 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptLongLong(cmd, "downtime", &downtime) < 0) { + if (vshCommandOptLongLong(ctl, cmd, "downtime", &downtime) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "downtime"); @@ -10215,7 +10215,7 @@ cmdMigrateCompCache(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - rc = vshCommandOptULongLong(cmd, "size", &size); + rc = vshCommandOptULongLong(ctl, cmd, "size", &size); if (rc < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), @@ -10276,7 +10276,7 @@ cmdMigrateSetMaxSpeed(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) { + if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "bandwidth"); @@ -12317,7 +12317,7 @@ cmdEvent(vshControl *ctl, const vshCmd *cmd) return true; } - if (vshCommandOptString(cmd, "event", &eventName) < 0) + if (vshCommandOptString(ctl, cmd, "event", &eventName) < 0) return false; if (eventName) { for (event = 0; event < VIR_DOMAIN_EVENT_ID_LAST; event++) @@ -12616,7 +12616,7 @@ cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return ret; - if (vshCommandOptULongLong(cmd, "minimum", &minimum) < 0) { + if (vshCommandOptULongLong(ctl, cmd, "minimum", &minimum) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "minimum"); @@ -12672,7 +12672,7 @@ cmdDomFSFreeze(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - while ((opt = vshCommandOptArgv(cmd, opt))) { + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { if (VIR_EXPAND_N(mountpoints, nmountpoints, 1) < 0) { vshError(ctl, _("%s: %d: failed to allocate mountpoints"), __FILE__, __LINE__); @@ -12729,7 +12729,7 @@ cmdDomFSThaw(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - while ((opt = vshCommandOptArgv(cmd, opt))) { + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { if (VIR_EXPAND_N(mountpoints, nmountpoints, 1) < 0) { vshError(ctl, _("%s: %d: failed to allocate mountpoints"), __FILE__, __LINE__); diff --git a/tools/virsh-host.c b/tools/virsh-host.c index a72fd05..b90782f 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -176,7 +176,7 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno); - if (cellno && vshCommandOptInt(cmd, "cellno", &cell) < 0) { + if (cellno && vshCommandOptInt(ctl, cmd, "cellno", &cell) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "cellno"); @@ -311,7 +311,7 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno); - if (vshCommandOptScaledInt(cmd, "pagesize", &bytes, 1024, UINT_MAX) < 0) { + if (vshCommandOptScaledInt(ctl, cmd, "pagesize", &bytes, 1024, UINT_MAX) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "pagesize"); @@ -391,7 +391,7 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (vshCommandOptInt(cmd, "cellno", &cell) < 0) { + if (vshCommandOptInt(ctl, cmd, "cellno", &cell) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "cellno"); @@ -490,14 +490,14 @@ cmdAllocpages(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno); - if (cellno && vshCommandOptInt(cmd, "cellno", &startCell) < 0) { + if (cellno && vshCommandOptInt(ctl, cmd, "cellno", &startCell) < 0) { 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) { + if (vshCommandOptScaledInt(ctl, cmd, "pagesize", &tmp, 1024, UINT_MAX) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "cellno"); @@ -505,7 +505,7 @@ cmdAllocpages(vshControl *ctl, const vshCmd *cmd) } pageSizes[0] = VIR_DIV_UP(tmp, 1024); - if (vshCommandOptULongLong(cmd, "pagecount", &pageCounts[0]) < 0) { + if (vshCommandOptULongLong(ctl, cmd, "pagecount", &pageCounts[0]) < 0) { vshError(ctl, "%s", _("pagecount has to be a number")); return false; } @@ -764,7 +764,7 @@ 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) { + if (vshCommandOptInt(ctl, cmd, "cpu", &cpuNum) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "cpu"); @@ -875,7 +875,7 @@ cmdNodeMemStats(vshControl *ctl, const vshCmd *cmd) virNodeMemoryStatsPtr params = NULL; bool ret = false; - if (vshCommandOptInt(cmd, "cell", &cellNum) < 0) { + if (vshCommandOptInt(ctl, cmd, "cell", &cellNum) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "cell"); @@ -951,7 +951,7 @@ cmdNodeSuspend(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0) return false; - if (vshCommandOptLongLong(cmd, "duration", &duration) < 0) { + if (vshCommandOptLongLong(ctl, cmd, "duration", &duration) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "duration"); @@ -1260,7 +1260,7 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd) int rc = -1; size_t i; - if ((rc = vshCommandOptUInt(cmd, "shm-pages-to-scan", &value)) < 0) { + if ((rc = vshCommandOptUInt(ctl, cmd, "shm-pages-to-scan", &value)) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "shm-pages-to-scan"); @@ -1272,7 +1272,7 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rc = vshCommandOptUInt(cmd, "shm-sleep-millisecs", &value)) < 0) { + if ((rc = vshCommandOptUInt(ctl, cmd, "shm-sleep-millisecs", &value)) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "shm-sleep-millisecs"); @@ -1284,7 +1284,7 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd) goto save_error; } - if ((rc = vshCommandOptUInt(cmd, "shm-merge-across-nodes", &value)) < 0) { + if ((rc = vshCommandOptUInt(ctl, cmd, "shm-merge-across-nodes", &value)) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "shm-merge-across-nodes"); diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 7122bc5..a6ae3b3 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -843,7 +843,7 @@ 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) { + if (vshCommandOptUInt(ctl, cmd, "delay", &delay) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "delay"); diff --git a/tools/virsh-network.c b/tools/virsh-network.c index a45367a..24250dd 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -936,7 +936,7 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (vshCommandOptInt(cmd, "parent-index", &parentIndex) < 0) { + if (vshCommandOptInt(ctl, cmd, "parent-index", &parentIndex) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "parent-index"); @@ -1227,7 +1227,7 @@ cmdNetworkEvent(vshControl *ctl, const vshCmd *cmd) return true; } - if (vshCommandOptString(cmd, "event", &eventName) < 0) + if (vshCommandOptString(ctl, cmd, "event", &eventName) < 0) return false; if (!eventName) { vshError(ctl, "%s", _("either --list or event type is required")); @@ -1337,7 +1337,7 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; virNetworkPtr network = NULL; - if (vshCommandOptString(cmd, "mac", &mac) < 0) + if (vshCommandOptString(ctl, cmd, "mac", &mac) < 0) return false; if (!(network = vshCommandOptNetwork(ctl, cmd, &name))) diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 0d7315c..8c578d1 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -395,7 +395,7 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) vshNodeDeviceListPtr list = NULL; int cap_type = -1; - ignore_value(vshCommandOptString(cmd, "cap", &cap_str)); + ignore_value(vshCommandOptString(ctl, cmd, "cap", &cap_str)); if (cap_str) { if (tree) { @@ -610,7 +610,7 @@ cmdNodeDeviceDetach(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "device", &name) < 0) return false; - ignore_value(vshCommandOptString(cmd, "driver", &driverName)); + ignore_value(vshCommandOptString(ctl, cmd, "driver", &driverName)); if (!(device = virNodeDeviceLookupByName(ctl->conn, name))) { vshError(ctl, _("Could not find matching device '%s'"), name); diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 459a8a8..de84175 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -440,7 +440,7 @@ cmdSnapshotCreateAs(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "diskspec")) { virBufferAddLit(&buf, "<disks>\n"); virBufferAdjustIndent(&buf, 2); - while ((opt = vshCommandOptArgv(cmd, opt))) { + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { if (vshParseSnapshotDiskspec(ctl, &buf, opt->data) < 0) goto cleanup; } diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 989ce87..9df1d81 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -219,7 +219,7 @@ cmdVolCreateAs(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (vshCommandOptString(cmd, "allocation", &allocationStr) > 0 && + if (vshCommandOptString(ctl, cmd, "allocation", &allocationStr) > 0 && vshVolSize(allocationStr, &allocation) < 0) { vshError(ctl, _("Malformed size %s"), allocationStr); goto cleanup; @@ -693,14 +693,14 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd) const char *name = NULL; unsigned long long offset = 0, length = 0; - if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) { + if (vshCommandOptULongLong(ctl, cmd, "offset", &offset) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "offset"); return false; } - if (vshCommandOptULongLongWrap(cmd, "length", &length) < 0) { + if (vshCommandOptULongLongWrap(ctl, cmd, "length", &length) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "length"); @@ -806,14 +806,14 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd) unsigned long long offset = 0, length = 0; bool created = false; - if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) { + if (vshCommandOptULongLong(ctl, cmd, "offset", &offset) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "offset"); return false; } - if (vshCommandOptULongLongWrap(cmd, "length", &length) < 0) { + if (vshCommandOptULongLongWrap(ctl, cmd, "length", &length) < 0) { vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "length"); diff --git a/tools/virsh.c b/tools/virsh.c index cd2bfef..bd65562 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -607,7 +607,7 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd) { const char *name = NULL; - if (vshCommandOptString(cmd, "command", &name) <= 0) { + if (vshCommandOptString(ctl, cmd, "command", &name) <= 0) { const vshCmdGrp *grp; const vshCmdDef *def; @@ -875,7 +875,7 @@ cmdCd(vshControl *ctl, const vshCmd *cmd) return false; } - if (vshCommandOptString(cmd, "dir", &dir) <= 0) + if (vshCommandOptString(ctl, cmd, "dir", &dir) <= 0) dir = dir_malloced = virGetUserDirectory(); if (!dir) dir = "/"; @@ -978,7 +978,7 @@ cmdEcho(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptBool(cmd, "xml")) xml = true; - while ((opt = vshCommandOptArgv(cmd, opt))) { + while ((opt = vshCommandOptArgv(ctl, cmd, opt))) { char *str; virBuffer xmlbuf = VIR_BUFFER_INITIALIZER; @@ -1471,7 +1471,8 @@ vshCommandFree(vshCmd *cmd) * issued if a value is returned. */ static int -vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt, +vshCommandOpt(const vshCmd *cmd, + const char *name, vshCmdOpt **opt, bool needData) { vshCmdOpt *candidate = cmd->opts; @@ -1504,6 +1505,7 @@ vshCommandOpt(const vshCmd *cmd, const char *name, vshCmdOpt **opt, /** * vshCommandOptInt: + * @ctl virsh control structure * @cmd command reference * @name option name * @value result @@ -1515,7 +1517,8 @@ 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 ATTRIBUTE_UNUSED, const vshCmd *cmd, + const char *name, int *value) { vshCmdOpt *arg; int ret; @@ -1530,9 +1533,8 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, int *value) } static int -vshCommandOptUIntInternal(const vshCmd *cmd, - const char *name, - unsigned int *value, +vshCommandOptUIntInternal(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, + const char *name, unsigned int *value, bool wrap) { vshCmdOpt *arg; @@ -1554,6 +1556,7 @@ vshCommandOptUIntInternal(const vshCmd *cmd, /** * vshCommandOptUInt: + * @ctl virsh control structure * @cmd command reference * @name option name * @value result @@ -1562,13 +1565,15 @@ vshCommandOptUIntInternal(const vshCmd *cmd, * 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) { - return vshCommandOptUIntInternal(cmd, name, value, false); + return vshCommandOptUIntInternal(ctl, cmd, name, value, false); } /** * vshCommandOptUIntWrap: + * @ctl virsh control structure * @cmd command reference * @name option name * @value result @@ -1577,15 +1582,15 @@ vshCommandOptUInt(const vshCmd *cmd, const char *name, unsigned int *value) * See vshCommandOptInt() */ int -vshCommandOptUIntWrap(const vshCmd *cmd, const char *name, unsigned int *value) +vshCommandOptUIntWrap(vshControl *ctl, const vshCmd *cmd, + const char *name, unsigned int *value) { - return vshCommandOptUIntInternal(cmd, name, value, true); + return vshCommandOptUIntInternal(ctl, cmd, name, value, true); } static int -vshCommandOptULInternal(const vshCmd *cmd, - const char *name, - unsigned long *value, +vshCommandOptULInternal(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, + const char *name, unsigned long *value, bool wrap) { vshCmdOpt *arg; @@ -1607,6 +1612,7 @@ vshCommandOptULInternal(const vshCmd *cmd, /* * vshCommandOptUL: + * @ctl virsh control structure * @cmd command reference * @name option name * @value result @@ -1615,13 +1621,15 @@ vshCommandOptULInternal(const vshCmd *cmd, * 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) { - return vshCommandOptULInternal(cmd, name, value, false); + return vshCommandOptULInternal(ctl, cmd, name, value, false); } /** * vshCommandOptULWrap: + * @ctl virsh control structure * @cmd command reference * @name option name * @value result @@ -1630,13 +1638,15 @@ vshCommandOptUL(const vshCmd *cmd, const char *name, unsigned long *value) * See vshCommandOptInt() */ int -vshCommandOptULWrap(const vshCmd *cmd, const char *name, unsigned long *value) +vshCommandOptULWrap(vshControl *ctl, const vshCmd *cmd, + const char *name, unsigned long *value) { - return vshCommandOptULInternal(cmd, name, value, true); + return vshCommandOptULInternal(ctl, cmd, name, value, true); } /** * vshCommandOptString: + * @ctl virsh control structure * @cmd command reference * @name option name * @value result @@ -1648,7 +1658,8 @@ vshCommandOptULWrap(const vshCmd *cmd, const char *name, unsigned long *value) * <0 in all other cases (@value untouched) */ int -vshCommandOptString(const vshCmd *cmd, const char *name, const char **value) +vshCommandOptString(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, + const char *name, const char **value) { vshCmdOpt *arg; int ret; @@ -1677,10 +1688,8 @@ vshCommandOptString(const vshCmd *cmd, const char *name, const char **value) * returned and error message printed. */ int -vshCommandOptStringReq(vshControl *ctl, - const vshCmd *cmd, - const char *name, - const char **value) +vshCommandOptStringReq(vshControl *ctl, const vshCmd *cmd, + const char *name, const char **value) { vshCmdOpt *arg; int ret; @@ -1710,6 +1719,7 @@ vshCommandOptStringReq(vshControl *ctl, /** * vshCommandOptLongLong: + * @ctl virsh control structure * @cmd command reference * @name option name * @value result @@ -1718,8 +1728,8 @@ vshCommandOptStringReq(vshControl *ctl, * See vshCommandOptInt() */ int -vshCommandOptLongLong(const vshCmd *cmd, const char *name, - long long *value) +vshCommandOptLongLong(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, + const char *name, long long *value) { vshCmdOpt *arg; int ret; @@ -1734,9 +1744,8 @@ vshCommandOptLongLong(const vshCmd *cmd, const char *name, } static int -vshCommandOptULongLongInternal(const vshCmd *cmd, - const char *name, - unsigned long long *value, +vshCommandOptULongLongInternal(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, + const char *name, unsigned long long *value, bool wrap) { vshCmdOpt *arg; @@ -1758,6 +1767,7 @@ vshCommandOptULongLongInternal(const vshCmd *cmd, /** * vshCommandOptULongLong: + * @ctl virsh control structure * @cmd command reference * @name option name * @value result @@ -1766,14 +1776,15 @@ vshCommandOptULongLongInternal(const vshCmd *cmd, * See vshCommandOptInt() */ int -vshCommandOptULongLong(const vshCmd *cmd, const char *name, - unsigned long long *value) +vshCommandOptULongLong(vshControl *ctl, const vshCmd *cmd, + const char *name, unsigned long long *value) { - return vshCommandOptULongLongInternal(cmd, name, value, false); + return vshCommandOptULongLongInternal(ctl, cmd, name, value, false); } /** * vshCommandOptULongLongWrap: + * @ctl virsh control structure * @cmd command reference * @name option name * @value result @@ -1782,14 +1793,15 @@ vshCommandOptULongLong(const vshCmd *cmd, const char *name, * See vshCommandOptInt() */ int -vshCommandOptULongLongWrap(const vshCmd *cmd, const char *name, - unsigned long long *value) +vshCommandOptULongLongWrap(vshControl *ctl, const vshCmd *cmd, + const char *name, unsigned long long *value) { - return vshCommandOptULongLongInternal(cmd, name, value, true); + return vshCommandOptULongLongInternal(ctl, cmd, name, value, true); } /** * vshCommandOptScaledInt: + * @ctl virsh control structure * @cmd command reference * @name option name * @value result @@ -1800,9 +1812,9 @@ vshCommandOptULongLongWrap(const vshCmd *cmd, const char *name, * See vshCommandOptInt() */ int -vshCommandOptScaledInt(const vshCmd *cmd, const char *name, - unsigned long long *value, int scale, - unsigned long long max) +vshCommandOptScaledInt(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, + const char *name, unsigned long long *value, + int scale, unsigned long long max) { vshCmdOpt *arg; char *end; @@ -1838,6 +1850,7 @@ vshCommandOptBool(const vshCmd *cmd, const char *name) /** * vshCommandOptArgv: + * @ctl virsh control structure * @cmd command reference * @opt starting point for the search * @@ -1848,7 +1861,8 @@ vshCommandOptBool(const vshCmd *cmd, const char *name) * list of supported options in CMD->def->opts. */ const vshCmdOpt * -vshCommandOptArgv(const vshCmd *cmd, const vshCmdOpt *opt) +vshCommandOptArgv(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, + const vshCmdOpt *opt) { opt = opt ? opt->next : cmd->opts; @@ -1871,12 +1885,13 @@ vshCommandOptArgv(const vshCmd *cmd, const vshCmdOpt *opt) * See vshCommandOptInt() */ int -vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, int *timeout) +vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, + int *timeout) { int ret; unsigned int utimeout; - if ((ret = vshCommandOptUInt(cmd, "timeout", &utimeout)) < 0) + if ((ret = vshCommandOptUInt(ctl, cmd, "timeout", &utimeout)) < 0) vshError(ctl, _("Numeric value for <%s> option is malformed or out of range"), "timeout"); diff --git a/tools/virsh.h b/tools/virsh.h index 32b1c91..8e00792 100644 --- a/tools/virsh.h +++ b/tools/virsh.h @@ -279,44 +279,46 @@ 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) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; -int vshCommandOptUInt(const vshCmd *cmd, const char *name, - unsigned int *value) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; -int vshCommandOptUIntWrap(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) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; -int vshCommandOptULWrap(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) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; +int vshCommandOptInt(vshControl *ctl, const vshCmd *cmd, + const char *name, int *value) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; +int vshCommandOptUInt(vshControl *ctl, const vshCmd *cmd, + const char *name, unsigned int *value) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; +int vshCommandOptUIntWrap(vshControl *ctl, const vshCmd *cmd, + const char *name, unsigned int *value) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; +int vshCommandOptUL(vshControl *ctl, const vshCmd *cmd, + const char *name, unsigned long *value) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; +int vshCommandOptULWrap(vshControl *ctl, const vshCmd *cmd, + const char *name, unsigned long *value) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; +int vshCommandOptString(vshControl *ctl, const vshCmd *cmd, + const char *name, const char **value) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; 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) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; -int vshCommandOptULongLong(const vshCmd *cmd, const char *name, - unsigned long long *value) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; -int vshCommandOptULongLongWrap(const vshCmd *cmd, const char *name, - unsigned long long *value) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; -int vshCommandOptScaledInt(const vshCmd *cmd, const char *name, - unsigned long long *value, int scale, - unsigned long long max) - ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; +int vshCommandOptLongLong(vshControl *ctl, const vshCmd *cmd, + const char *name, long long *value) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; +int vshCommandOptULongLong(vshControl *ctl, const vshCmd *cmd, + const char *name, unsigned long long *value) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; +int vshCommandOptULongLongWrap(vshControl *ctl, const vshCmd *cmd, + const char *name, unsigned long long *value) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; +int vshCommandOptScaledInt(vshControl *ctl, const vshCmd *cmd, + const char *name, unsigned long long *value, + int scale, unsigned long long max) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_RETURN_CHECK; bool vshCommandOptBool(const vshCmd *cmd, const char *name); -const vshCmdOpt *vshCommandOptArgv(const vshCmd *cmd, +const vshCmdOpt *vshCommandOptArgv(vshControl *ctl, const vshCmd *cmd, const vshCmdOpt *opt); -int vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, int *timeout); +int vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, + int *timeout); /* Filter flags for various vshCommandOpt*By() functions */ typedef enum { -- 2.1.0

--- tests/vcpupin | 4 +- tests/virsh-optparse | 26 ++++----- tools/virsh-domain-monitor.c | 9 +-- tools/virsh-domain.c | 134 +++++++------------------------------------ tools/virsh-host.c | 61 +++----------------- tools/virsh-interface.c | 6 +- tools/virsh-network.c | 6 +- tools/virsh-volume.c | 24 ++------ tools/virsh.c | 124 +++++++++++++++++++++++---------------- 9 files changed, 124 insertions(+), 270 deletions(-) diff --git a/tests/vcpupin b/tests/vcpupin index ab0d38f..b6b8b31 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: Numeric value for <vcpu> option is malformed or out of range +error: Numeric value 'a' 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: Numeric value for <vcpu> option is malformed or out of range +error: Numeric value '-100' for <vcpu> option is malformed or out of range EOF compare exp out || fail=1 diff --git a/tests/virsh-optparse b/tests/virsh-optparse index 952fcc4..e032094 100755 --- a/tests/virsh-optparse +++ b/tests/virsh-optparse @@ -141,7 +141,7 @@ compare exp-err err || fail=1 # Non-numeric value cat <<\EOF > exp-err || framework_failure -error: Numeric value for <start> option is malformed or out of range +error: Numeric value 'abc' for <start> option is malformed or out of range EOF virsh -q -c $test_url cpu-stats test --start abc >out 2>err && fail=1 test -s out && fail=1 @@ -149,7 +149,7 @@ compare exp-err err || fail=1 # Numeric value with invalid suffix cat <<\EOF > exp-err || framework_failure -error: Numeric value for <start> option is malformed or out of range +error: Numeric value '42WB' for <start> option is malformed or out of range EOF virsh -q -c $test_url cpu-stats test --start 42WB >out 2>err && fail=1 test -s out && fail=1 @@ -158,7 +158,7 @@ compare exp-err err || fail=1 # Numeric value with valid suffix. Suffixes are not supported for # regular numeric options, so this value is rejected cat <<\EOF > exp-err || framework_failure -error: Numeric value for <start> option is malformed or out of range +error: Numeric value '42MB' for <start> option is malformed or out of range EOF virsh -q -c $test_url cpu-stats test --start 42MB >out 2>err && fail=1 test -s out && fail=1 @@ -166,7 +166,7 @@ compare exp-err err || fail=1 # Numeric value bigger than INT_MAX cat <<\EOF > exp-err || framework_failure -error: Numeric value for <start> option is malformed or out of range +error: Numeric value '2147483648' for <start> option is malformed or out of range EOF virsh -q -c $test_url cpu-stats test --start 2147483648 >out 2>err && fail=1 test -s out && fail=1 @@ -205,7 +205,7 @@ compare exp-err err || fail=1 # Non-numeric value cat <<\EOF > exp-err || framework_failure -error: Numeric value for <size> option is malformed or out of range +error: Numeric value 'abc' for <size> option is malformed or out of range EOF virsh -q -c $test_url setmaxmem test abc >out 2>err && fail=1 test -s out && fail=1 @@ -213,7 +213,7 @@ compare exp-err err || fail=1 # Numeric value with invalid suffix cat <<\EOF > exp-err || framework_failure -error: Numeric value for <size> option is malformed or out of range +error: Numeric value '42WB' for <size> option is malformed or out of range EOF virsh -q -c $test_url setmaxmem test 42WB >out 2>err && fail=1 test -s out && fail=1 @@ -232,7 +232,7 @@ test -s err && fail=1 # Negative numeric value cat <<\EOF > exp-err || framework_failure -error: Numeric value for <size> option is malformed or out of range +error: Numeric value '-1' for <size> option is malformed or out of range EOF virsh -q -c $test_url setmaxmem test -1 >out 2>err && fail=1 test -s out && fail=1 @@ -257,7 +257,7 @@ test -s err && fail=1 # Non-numeric value cat <<\EOF > exp-err || framework_failure -error: Numeric value for <timeout> option is malformed or out of range +error: Numeric value 'abc' for <timeout> option is malformed or out of range EOF virsh -q -c $test_url event --all --timeout abc >out 2>err && fail=1 test -s out && fail=1 @@ -266,7 +266,7 @@ compare exp-err err || fail=1 # Numeric value that's too big to be converted to ms and still # fit inside an int cat <<\EOF > exp-err || framework_failure -error: Numeric value for <timeout> option is malformed or out of range +error: Numeric value '2147484' for <timeout> option is malformed or out of range EOF virsh -q -c $test_url event --all --timeout 2147484 >out 2>err && fail=1 test -s out && fail=1 @@ -274,7 +274,7 @@ compare exp-err err || fail=1 # Numeric value with invalid suffix cat <<\EOF > exp-err || framework_failure -error: Numeric value for <timeout> option is malformed or out of range +error: Numeric value '42WB' for <timeout> option is malformed or out of range EOF virsh -q -c $test_url event --all --timeout 42WB >out 2>err && fail=1 test -s out && fail=1 @@ -283,7 +283,7 @@ compare exp-err err || fail=1 # Numeric value with valid suffix. Suffixes are not supported for # the <timeout> option, so this value is rejected cat <<\EOF > exp-err || framework_failure -error: Numeric value for <timeout> option is malformed or out of range +error: Numeric value '42MB' for <timeout> option is malformed or out of range EOF virsh -q -c $test_url event --all --timeout 42MB >out 2>err && fail=1 test -s out && fail=1 @@ -291,7 +291,7 @@ compare exp-err err || fail=1 # Negative value cat <<\EOF > exp-err || framework_failure -error: Numeric value for <timeout> option is malformed or out of range +error: Numeric value '-1' for <timeout> option is malformed or out of range EOF virsh -q -c $test_url event --all --timeout -1 >out 2>err && fail=1 test -s out && fail=1 @@ -300,7 +300,7 @@ compare exp-err err || fail=1 # Zero. This is not a valid timeout, but the value is parsed # correctly cat <<\EOF > exp-err || framework_failure -error: Numeric value for <timeout> option is malformed or out of range +error: Numeric value '0' for <timeout> option is malformed or out of range EOF virsh -q -c $test_url event --all --timeout 0 >out 2>err && fail=1 test -s out && fail=1 diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index d8b217b..1d4dc25 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -340,12 +340,8 @@ 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(ctl, cmd, "period", &period)) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "period"); + if ((rv = vshCommandOptInt(ctl, cmd, "period", &period)) < 0) goto cleanup; - } if (rv > 0) { if (period < 0) { vshError(ctl, _("Invalid collection period value '%d'"), period); @@ -1440,9 +1436,6 @@ cmdDomTime(vshControl *ctl, const vshCmd *cmd) if (rv < 0) { /* invalid integer format */ - 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 79a9a9b..4970948 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -1552,9 +1552,6 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd) return false; if ((rv = vshCommandOptInt(ctl, cmd, "weight", &weight)) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "weight"); goto cleanup; } else if (rv > 0) { if (weight <= 0) { @@ -1692,12 +1689,8 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd, if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0) goto cleanup; - if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "bandwidth"); + if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0) goto cleanup; - } switch (mode) { case VSH_CMD_BLOCK_JOB_ABORT: @@ -2216,24 +2209,12 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd) * MiB/s, and either reject negative input or treat it as 0 rather * than trying to guess which value will work well across both * APIs with their different sizes and scales. */ - if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "bandwidth"); + if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0) goto cleanup; - } - if (vshCommandOptUInt(ctl, cmd, "granularity", &granularity) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "granularity"); + if (vshCommandOptUInt(ctl, cmd, "granularity", &granularity) < 0) goto cleanup; - } - if (vshCommandOptULongLong(ctl, cmd, "buf-size", &buf_size) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "buf-size"); + if (vshCommandOptULongLong(ctl, cmd, "buf-size", &buf_size) < 0) goto cleanup; - } if (xml) { if (virFileReadAll(xml, VSH_MAX_XML_FILE, &xmlstr) < 0) { @@ -2800,12 +2781,8 @@ cmdBlockResize(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "path", (const char **) &path) < 0) return false; - if (vshCommandOptScaledInt(ctl, cmd, "size", &size, 1024, ULLONG_MAX) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "size"); + if (vshCommandOptScaledInt(ctl, cmd, "size", &size, 1024, ULLONG_MAX) < 0) return false; - } /* Prefer the older interface of KiB. */ if (size % 1024 == 0) @@ -3406,12 +3383,8 @@ cmdDomPMSuspend(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; - if (vshCommandOptULongLong(ctl, cmd, "duration", &duration) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "duration"); + if (vshCommandOptULongLong(ctl, cmd, "duration", &duration) < 0) goto cleanup; - } if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0) goto cleanup; @@ -5330,12 +5303,8 @@ cmdScreenshot(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "file", (const char **) &file) < 0) return false; - if (vshCommandOptUInt(ctl, cmd, "screen", &screen) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "screen"); + if (vshCommandOptUInt(ctl, cmd, "screen", &screen) < 0) return false; - } if (!(dom = vshCommandOptDomain(ctl, cmd, &name))) return false; @@ -6497,12 +6466,8 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) if (!cpulist) VSH_EXCLUSIVE_OPTIONS_VAR(live, config); - if ((got_vcpu = vshCommandOptUInt(ctl, cmd, "vcpu", &vcpu)) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "vcpu"); + if ((got_vcpu = vshCommandOptUInt(ctl, cmd, "vcpu", &vcpu)) < 0) return false; - } /* In pin mode, "vcpu" is necessary */ if (cpulist && got_vcpu == 0) { @@ -6766,12 +6731,8 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptInt(ctl, cmd, "count", &count) < 0 || count <= 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "count"); + if (vshCommandOptInt(ctl, cmd, "count", &count) < 0 || count <= 0) goto cleanup; - } /* none of the options were specified */ if (!current && flags == 0) { @@ -6946,12 +6907,8 @@ cmdIOThreadPin(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptUInt(ctl, cmd, "iothread", &iothread_id) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "iothread"); + if (vshCommandOptUInt(ctl, cmd, "iothread", &iothread_id) < 0) goto cleanup; - } if (vshCommandOptString(ctl, cmd, "cpulist", &cpulist) < 0) { vshError(ctl, "%s", _("iothreadpin: invalid cpulist.")); @@ -7037,12 +6994,8 @@ cmdIOThreadAdd(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptInt(ctl, cmd, "id", &iothread_id) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "id"); + if (vshCommandOptInt(ctl, cmd, "id", &iothread_id) < 0) goto cleanup; - } if (iothread_id <= 0) { vshError(ctl, _("Invalid IOThread id value: '%d'"), iothread_id); goto cleanup; @@ -7119,12 +7072,8 @@ cmdIOThreadDel(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptInt(ctl, cmd, "id", &iothread_id) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "id"); + if (vshCommandOptInt(ctl, cmd, "id", &iothread_id) < 0) goto cleanup; - } if (iothread_id <= 0) { vshError(ctl, _("Invalid IOThread id value: '%d'"), iothread_id); goto cleanup; @@ -7409,9 +7358,6 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) show_total = vshCommandOptBool(cmd, "total"); if ((rv = vshCommandOptInt(ctl, cmd, "start", &cpu)) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "start"); goto cleanup; } else if (rv > 0) { if (cpu < 0) { @@ -7422,9 +7368,6 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd) } if ((rv = vshCommandOptInt(ctl, cmd, "count", &show_count)) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "count"); goto cleanup; } else if (rv > 0) { if (show_count < 0) { @@ -8211,12 +8154,8 @@ cmdSendKey(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(ctl, cmd, "codeset", &codeset_option) <= 0) codeset_option = "linux"; - if (vshCommandOptUInt(ctl, cmd, "holdtime", &holdtime) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "holdtime"); + if (vshCommandOptUInt(ctl, cmd, "holdtime", &holdtime) < 0) goto cleanup; - } codeset = virKeycodeSetTypeFromString(codeset_option); if (codeset < 0) { @@ -8337,12 +8276,8 @@ cmdSendProcessSignal(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptLongLong(ctl, cmd, "pid", &pid_value) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "pid"); + if (vshCommandOptLongLong(ctl, cmd, "pid", &pid_value) < 0) goto cleanup; - } if (vshCommandOptStringReq(ctl, cmd, "signame", &signame) < 0) goto cleanup; @@ -8439,9 +8374,6 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd) else max = ULONG_MAX; if (vshCommandOptScaledInt(ctl, cmd, "size", &bytes, 1024, max) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "size"); virDomainFree(dom); return false; } @@ -8536,9 +8468,6 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd) else max = ULONG_MAX; if (vshCommandOptScaledInt(ctl, cmd, "size", &bytes, 1024, max) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "size"); virDomainFree(dom); return false; } @@ -9171,12 +9100,8 @@ cmdQemuAttach(vshControl *ctl, const vshCmd *cmd) unsigned int flags = 0; unsigned int pid_value; /* API uses unsigned int, not pid_t */ - if (vshCommandOptUInt(ctl, cmd, "pid", &pid_value) <= 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "pid"); + if (vshCommandOptUInt(ctl, cmd, "pid", &pid_value) <= 0) goto cleanup; - } if (!(dom = virDomainQemuAttach(ctl->conn, pid_value, flags))) { vshError(ctl, _("Failed to attach to pid %u"), pid_value); @@ -9268,14 +9193,10 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd) guest_agent_cmd = virBufferContentAndReset(&buf); judge = vshCommandOptInt(ctl, cmd, "timeout", &timeout); - if (judge < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "timeout"); + if (judge < 0) goto cleanup; - } else if (judge > 0) { + else if (judge > 0) judge = 1; - } if (judge && timeout < 1) { vshError(ctl, "%s", _("timeout must be positive")); goto cleanup; @@ -10153,12 +10074,8 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptLongLong(ctl, cmd, "downtime", &downtime) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "downtime"); + if (vshCommandOptLongLong(ctl, cmd, "downtime", &downtime) < 0) goto done; - } if (downtime < 1) { vshError(ctl, "%s", _("migrate: Invalid downtime")); goto done; @@ -10217,9 +10134,6 @@ cmdMigrateCompCache(vshControl *ctl, const vshCmd *cmd) rc = vshCommandOptULongLong(ctl, cmd, "size", &size); if (rc < 0) { - 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) @@ -10276,12 +10190,8 @@ cmdMigrateSetMaxSpeed(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return false; - if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "bandwidth"); + if (vshCommandOptULWrap(ctl, cmd, "bandwidth", &bandwidth) < 0) goto done; - } if (virDomainMigrateSetMaxSpeed(dom, bandwidth, 0) < 0) goto done; @@ -12616,12 +12526,8 @@ cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd) if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) return ret; - if (vshCommandOptULongLong(ctl, cmd, "minimum", &minimum) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "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 b90782f..66f7fd9 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -176,12 +176,8 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno); - if (cellno && vshCommandOptInt(ctl, cmd, "cellno", &cell) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "cellno"); + if (cellno && vshCommandOptInt(ctl, cmd, "cellno", &cell) < 0) return false; - } if (all) { if (!(cap_xml = virConnectGetCapabilities(ctl->conn))) { @@ -311,12 +307,8 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno); - if (vshCommandOptScaledInt(ctl, cmd, "pagesize", &bytes, 1024, UINT_MAX) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "pagesize"); + if (vshCommandOptScaledInt(ctl, cmd, "pagesize", &bytes, 1024, UINT_MAX) < 0) goto cleanup; - } kibibytes = VIR_DIV_UP(bytes, 1024); if (all) { @@ -391,12 +383,8 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (vshCommandOptInt(ctl, cmd, "cellno", &cell) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "cellno"); + if (vshCommandOptInt(ctl, cmd, "cellno", &cell) < 0) goto cleanup; - } if (cell < -1) { vshError(ctl, "%s", @@ -490,25 +478,15 @@ cmdAllocpages(vshControl *ctl, const vshCmd *cmd) VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno); - if (cellno && vshCommandOptInt(ctl, cmd, "cellno", &startCell) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "cellno"); + if (cellno && vshCommandOptInt(ctl, cmd, "cellno", &startCell) < 0) return false; - } - if (vshCommandOptScaledInt(ctl, cmd, "pagesize", &tmp, 1024, UINT_MAX) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "cellno"); + if (vshCommandOptScaledInt(ctl, cmd, "pagesize", &tmp, 1024, UINT_MAX) < 0) return false; - } pageSizes[0] = VIR_DIV_UP(tmp, 1024); - if (vshCommandOptULongLong(ctl, cmd, "pagecount", &pageCounts[0]) < 0) { - vshError(ctl, "%s", _("pagecount has to be a number")); + if (vshCommandOptULongLong(ctl, cmd, "pagecount", &pageCounts[0]) < 0) return false; - } flags |= add ? VIR_NODE_ALLOC_PAGES_ADD : VIR_NODE_ALLOC_PAGES_SET; @@ -764,12 +742,8 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd) unsigned long long cpu_stats[VSH_CPU_LAST] = { 0 }; bool present[VSH_CPU_LAST] = { false }; - if (vshCommandOptInt(ctl, cmd, "cpu", &cpuNum) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "cpu"); + if (vshCommandOptInt(ctl, cmd, "cpu", &cpuNum) < 0) return false; - } if (virNodeGetCPUStats(ctl->conn, cpuNum, NULL, &nparams, 0) != 0) { vshError(ctl, "%s", @@ -875,12 +849,8 @@ cmdNodeMemStats(vshControl *ctl, const vshCmd *cmd) virNodeMemoryStatsPtr params = NULL; bool ret = false; - if (vshCommandOptInt(ctl, cmd, "cell", &cellNum) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "cell"); + if (vshCommandOptInt(ctl, cmd, "cell", &cellNum) < 0) return false; - } /* get the number of memory parameters */ if (virNodeGetMemoryStats(ctl->conn, cellNum, NULL, &nparams, 0) != 0) { @@ -951,12 +921,8 @@ cmdNodeSuspend(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptStringReq(ctl, cmd, "target", &target) < 0) return false; - if (vshCommandOptLongLong(ctl, cmd, "duration", &duration) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "duration"); + if (vshCommandOptLongLong(ctl, cmd, "duration", &duration) < 0) return false; - } if (STREQ(target, "mem")) { suspendTarget = VIR_NODE_SUSPEND_TARGET_MEM; @@ -1261,9 +1227,6 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd) size_t i; if ((rc = vshCommandOptUInt(ctl, cmd, "shm-pages-to-scan", &value)) < 0) { - 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(¶ms, &nparams, &maxparams, @@ -1273,9 +1236,6 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd) } if ((rc = vshCommandOptUInt(ctl, cmd, "shm-sleep-millisecs", &value)) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "shm-sleep-millisecs"); goto cleanup; } else if (rc > 0) { if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams, @@ -1285,9 +1245,6 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd) } if ((rc = vshCommandOptUInt(ctl, cmd, "shm-merge-across-nodes", &value)) < 0) { - 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(¶ms, &nparams, &maxparams, diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index a6ae3b3..6ad5345 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -843,12 +843,8 @@ cmdInterfaceBridge(vshControl *ctl, const vshCmd *cmd) /* use "no-stp" because we want "stp" to default true */ stp = !vshCommandOptBool(cmd, "no-stp"); - if (vshCommandOptUInt(ctl, cmd, "delay", &delay) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "delay"); + 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 24250dd..45f4840 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -936,12 +936,8 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd) goto cleanup; } - if (vshCommandOptInt(ctl, cmd, "parent-index", &parentIndex) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "parent-index"); + 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 9df1d81..bb1bfc2 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -693,19 +693,11 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd) const char *name = NULL; unsigned long long offset = 0, length = 0; - if (vshCommandOptULongLong(ctl, cmd, "offset", &offset) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "offset"); + if (vshCommandOptULongLong(ctl, cmd, "offset", &offset) < 0) return false; - } - if (vshCommandOptULongLongWrap(ctl, cmd, "length", &length) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "length"); + if (vshCommandOptULongLongWrap(ctl, cmd, "length", &length) < 0) return false; - } if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name))) return false; @@ -806,19 +798,11 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd) unsigned long long offset = 0, length = 0; bool created = false; - if (vshCommandOptULongLong(ctl, cmd, "offset", &offset) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "offset"); + if (vshCommandOptULongLong(ctl, cmd, "offset", &offset) < 0) return false; - } - if (vshCommandOptULongLongWrap(ctl, cmd, "length", &length) < 0) { - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "length"); + if (vshCommandOptULongLongWrap(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 bd65562..2dffc66 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1510,30 +1510,36 @@ vshCommandOpt(const vshCmd *cmd, * @name option name * @value result * - * Convert option to int + * Convert option to int. + * On error, a message is displayed. + * * Return value: * >0 if option found and valid (@value updated) * 0 if option not found and not required (@value untouched) * <0 in all other cases (@value untouched) */ int -vshCommandOptInt(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, +vshCommandOptInt(vshControl *ctl, const vshCmd *cmd, const char *name, int *value) { vshCmdOpt *arg; int ret; - ret = vshCommandOpt(cmd, name, &arg, true); - if (ret <= 0) + if ((ret = vshCommandOpt(cmd, name, &arg, true)) <= 0) return ret; - if (virStrToLong_i(arg->data, NULL, 10, value) < 0) - return -1; - return 1; + if ((ret = virStrToLong_i(arg->data, NULL, 10, value)) < 0) + vshError(ctl, + _("Numeric value '%s' for <%s> option is malformed or out of range"), + arg->data, name); + else + ret = 1; + + return ret; } static int -vshCommandOptUIntInternal(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, +vshCommandOptUIntInternal(vshControl *ctl, const vshCmd *cmd, const char *name, unsigned int *value, bool wrap) { @@ -1543,15 +1549,18 @@ vshCommandOptUIntInternal(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, if ((ret = vshCommandOpt(cmd, name, &arg, true)) <= 0) return ret; - if (wrap) { - if (virStrToLong_ui(arg->data, NULL, 10, value) < 0) - return -1; - } else { - if (virStrToLong_uip(arg->data, NULL, 10, value) < 0) - return -1; - } + if (wrap) + ret = virStrToLong_ui(arg->data, NULL, 10, value); + else + ret = virStrToLong_uip(arg->data, NULL, 10, value); + if (ret < 0) + vshError(ctl, + _("Numeric value '%s' for <%s> option is malformed or out of range"), + arg->data, name); + else + ret = 1; - return 1; + return ret; } /** @@ -1589,7 +1598,7 @@ vshCommandOptUIntWrap(vshControl *ctl, const vshCmd *cmd, } static int -vshCommandOptULInternal(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, +vshCommandOptULInternal(vshControl *ctl, const vshCmd *cmd, const char *name, unsigned long *value, bool wrap) { @@ -1599,15 +1608,18 @@ vshCommandOptULInternal(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, if ((ret = vshCommandOpt(cmd, name, &arg, true)) <= 0) return ret; - if (wrap) { - if (virStrToLong_ul(arg->data, NULL, 10, value) < 0) - return -1; - } else { - if (virStrToLong_ulp(arg->data, NULL, 10, value) < 0) - return -1; - } + if (wrap) + ret = virStrToLong_ul(arg->data, NULL, 10, value); + else + ret = virStrToLong_ulp(arg->data, NULL, 10, value); + if (ret < 0) + vshError(ctl, + _("Numeric value '%s' for <%s> option is malformed or out of range"), + arg->data, name); + else + ret = 1; - return 1; + return ret; } /* @@ -1664,8 +1676,7 @@ vshCommandOptString(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, vshCmdOpt *arg; int ret; - ret = vshCommandOpt(cmd, name, &arg, true); - if (ret <= 0) + if ((ret = vshCommandOpt(cmd, name, &arg, true)) <= 0) return ret; if (!*arg->data && !(arg->def->flags & VSH_OFLAG_EMPTY_OK)) @@ -1728,23 +1739,27 @@ vshCommandOptStringReq(vshControl *ctl, const vshCmd *cmd, * See vshCommandOptInt() */ int -vshCommandOptLongLong(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, +vshCommandOptLongLong(vshControl *ctl, const vshCmd *cmd, const char *name, long long *value) { vshCmdOpt *arg; int ret; - ret = vshCommandOpt(cmd, name, &arg, true); - if (ret <= 0) + if ((ret = vshCommandOpt(cmd, name, &arg, true)) <= 0) return ret; - if (virStrToLong_ll(arg->data, NULL, 10, value) < 0) - return -1; - return 1; + if ((ret = virStrToLong_ll(arg->data, NULL, 10, value)) < 0) + vshError(ctl, + _("Numeric value '%s' for <%s> option is malformed or out of range"), + arg->data, name); + else + ret = 1; + + return ret; } static int -vshCommandOptULongLongInternal(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, +vshCommandOptULongLongInternal(vshControl *ctl, const vshCmd *cmd, const char *name, unsigned long long *value, bool wrap) { @@ -1754,15 +1769,18 @@ vshCommandOptULongLongInternal(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *c if ((ret = vshCommandOpt(cmd, name, &arg, true)) <= 0) return ret; - if (wrap) { - if (virStrToLong_ull(arg->data, NULL, 10, value) < 0) - return -1; - } else { - if (virStrToLong_ullp(arg->data, NULL, 10, value) < 0) - return -1; - } + if (wrap) + ret = virStrToLong_ull(arg->data, NULL, 10, value); + else + ret = virStrToLong_ullp(arg->data, NULL, 10, value); + if (ret < 0) + vshError(ctl, + _("Numeric value '%s' for <%s> option is malformed or out of range"), + arg->data, name); + else + ret = 1; - return 1; + return ret; } /** @@ -1812,7 +1830,7 @@ vshCommandOptULongLongWrap(vshControl *ctl, const vshCmd *cmd, * See vshCommandOptInt() */ int -vshCommandOptScaledInt(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, +vshCommandOptScaledInt(vshControl *ctl, const vshCmd *cmd, const char *name, unsigned long long *value, int scale, unsigned long long max) { @@ -1824,9 +1842,16 @@ vshCommandOptScaledInt(vshControl *ctl ATTRIBUTE_UNUSED, const vshCmd *cmd, return ret; if (virStrToLong_ullp(arg->data, &end, 10, value) < 0 || virScaleInteger(value, end, scale, max) < 0) - return -1; + { + vshError(ctl, + _("Numeric value '%s' for <%s> option is malformed or out of range"), + arg->data, name); + ret = -1; + } else { + ret = 1; + } - return 1; + return ret; } @@ -1891,18 +1916,15 @@ vshCommandOptTimeoutToMs(vshControl *ctl, const vshCmd *cmd, int ret; unsigned int utimeout; - if ((ret = vshCommandOptUInt(ctl, cmd, "timeout", &utimeout)) < 0) - vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), - "timeout"); - if (ret <= 0) + if ((ret = vshCommandOptUInt(ctl, cmd, "timeout", &utimeout)) <= 0) return ret; /* Ensure that the timeout is not zero and that we can convert * it from seconds to milliseconds without overflowing. */ if (utimeout == 0 || utimeout > INT_MAX / 1000) { vshError(ctl, - _("Numeric value for <%s> option is malformed or out of range"), + _("Numeric value '%u' for <%s> option is malformed or out of range"), + utimeout, "timeout"); ret = -1; } else { -- 2.1.0

On 06/02/2015 05:17 AM, Andrea Bolognani wrote:
As suggested by Michal: now that we have a generic error message for failures related to the parsing of integer options, it makes sense to perform the corresponding check in a single spot instead of replicating it every time vshCommandOpt*() is used.
Andrea Bolognani (6): tests: Add a bunch of new tests to virsh-optparse virsh: Use standard error messages in vshCommandOptTimeoutToMs() virsh: Improve vshCommandOptTimeoutToMs() virsh: Make vshCommandOptScaledInt() use vshCommandOpt() virsh: Pass vshControl to all vshCommandOpt*() calls virsh: Move error messages inside vshCommandOpt*() functions
tests/vcpupin | 4 +- tests/virsh-optparse | 179 ++++++++++++++++++++++++++++++ tools/virsh-domain-monitor.c | 17 +-- tools/virsh-domain.c | 226 ++++++++++++-------------------------- tools/virsh-host.c | 67 +++--------- tools/virsh-interface.c | 6 +- tools/virsh-network.c | 10 +- tools/virsh-nodedev.c | 4 +- tools/virsh-snapshot.c | 2 +- tools/virsh-volume.c | 26 +---- tools/virsh.c | 252 ++++++++++++++++++++++++++----------------- tools/virsh.h | 66 ++++++------ 12 files changed, 461 insertions(+), 398 deletions(-)
ACK series... and pushed John
participants (2)
-
Andrea Bolognani
-
John Ferlan