7:43 a.m.
*** BLURB HERE ***
Michal Privoznik (7):
virsh: Make vshCommandOpt* report error
virsh-domain: Adapt to new error reporting
virsh-domain-monitor: Adapt to new error reporting
virsh-host: Adapt to new error reporting
virsh-interface: Adapt to new error reporting
virsh-network: Adapt to new error reporting
virsh-volume: Adapt to new error reporting
tests/vcpupin | 2 +-
tools/virsh-domain-monitor.c | 5 +--
tools/virsh-domain.c | 75 ++++++++++++++------------------------------
tools/virsh-host.c | 19 +++--------
tools/virsh-interface.c | 4 +--
tools/virsh-network.c | 4 +--
tools/virsh-volume.c | 16 +++-------
tools/virsh.c | 72 +++++++++++++++++++++++++++++++++++++-----
tools/virsh.h | 2 +-
9 files changed, 102 insertions(+), 97 deletions(-)
--
1.9.0
7:43 a.m.
New subject: [libvirt] [PATCH 1/7] 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>
---
tests/vcpupin | 1 +
tools/virsh.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
tools/virsh.h | 2 +-
3 files changed, 67 insertions(+), 8 deletions(-)
diff --git a/tests/vcpupin b/tests/vcpupin
index f1fb038..a616216 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -34,6 +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: Unable to parse integer parameter to --vcpu
error: vcpupin: Invalid or missing vCPU number.
EOF
diff --git a/tools/virsh.c b/tools/virsh.c
index f2e4c4a..1371abb 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1489,8 +1489,18 @@ 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) {
+ if (arg->def->flags & VSH_OFLAG_REQ) {
+ vshError(NULL,
+ _("missing --%s parameter value"),
+ name);
+ } else {
+ vshError(NULL,
+ _("Unable to parse integer parameter to --%s"),
+ name);
+ }
return -1;
+ }
return 1;
}
@@ -1514,8 +1524,18 @@ 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) {
+ if (arg->def->flags & VSH_OFLAG_REQ) {
+ vshError(NULL,
+ _("missing --%s parameter value"),
+ name);
+ } else {
+ vshError(NULL,
+ _("Unable to parse integer parameter to --%s"),
+ name);
+ }
return -1;
+ }
return 1;
}
@@ -1539,8 +1559,18 @@ 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) {
+ if (arg->def->flags & VSH_OFLAG_REQ) {
+ vshError(NULL,
+ _("missing --%s parameter value"),
+ name);
+ } else {
+ vshError(NULL,
+ _("Unable to parse integer parameter to --%s"),
+ name);
+ }
return -1;
+ }
return 1;
}
@@ -1638,8 +1668,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) {
+ if (arg->def->flags & VSH_OFLAG_REQ) {
+ vshError(NULL,
+ _("missing --%s parameter value"),
+ name);
+ } else {
+ vshError(NULL,
+ _("Unable to parse integer parameter to --%s"),
+ name);
+ }
return -1;
+ }
return 1;
}
@@ -1663,8 +1703,18 @@ 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) {
+ if (arg->def->flags & VSH_OFLAG_REQ) {
+ vshError(NULL,
+ _("missing --%s parameter value"),
+ name);
+ } else {
+ vshError(NULL,
+ _("Unable to parse integer parameter to --%s"),
+ name);
+ }
return -1;
+ }
return 1;
}
@@ -1692,9 +1742,17 @@ 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(NULL,
+ _("Unable to parse integer parameter to --%s"),
+ name);
return -1;
+ }
+
+ if (virScaleInteger(value, end, scale, max) < 0) {
+ /* Error reported by the helper. */
+ return -1;
+ }
return 1;
}
diff --git a/tools/virsh.h b/tools/virsh.h
index 3e0251b..6eb17b3 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 */
--
1.9.0
4:12 a.m.
New subject: [libvirt] [PATCH 1/7] virsh: Make vshCommandOpt* report error
On 04/02/2014 02:43 PM, Michal Privoznik wrote:
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>
---
tests/vcpupin | 1 +
tools/virsh.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++++------
vshCommandOptTimeoutToMs also calls vshCommandOptInt
tools/virsh.h | 2 +-
3 files changed, 67 insertions(+), 8 deletions(-)
diff --git a/tests/vcpupin b/tests/vcpupin
index f1fb038..a616216 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -34,6 +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: Unable to parse integer parameter to --vcpu
error: vcpupin: Invalid or missing vCPU number.
EOF
diff --git a/tools/virsh.c b/tools/virsh.c
index f2e4c4a..1371abb 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1489,8 +1489,18 @@ 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) {
+ if (arg->def->flags & VSH_OFLAG_REQ) {
+ vshError(NULL,
ctl should be used for reporting errors, just like in vshCommandOptStringReq.
+ _("missing --%s parameter value"),
+ name);
Missing values are caught in vshCommandParse. The error should be the same
regardless of VSH_OFLAG_REQ.
+ } else {
+ vshError(NULL,
+ _("Unable to parse integer parameter to --%s"),
+ name);
@@ -1692,9 +1742,17 @@ 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(NULL,
+ _("Unable to parse integer parameter to --%s"),
+ name);
return -1;
+ }
+
+ if (virScaleInteger(value, end, scale, max) < 0) {
+ /* Error reported by the helper. */
Needs vshReportError to propagate the error.
+ return -1;
+ }
return 1;
}
diff --git a/tools/virsh.h b/tools/virsh.h
index 3e0251b..6eb17b3 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 */
You can push this hunk separately as trivial.
Jan
7:43 a.m.
New subject: [libvirt] [PATCH 2/7] virsh-domain: Adapt to new error reporting
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/vcpupin | 1 -
tools/virsh-domain.c | 75 +++++++++++++++++-----------------------------------
2 files changed, 24 insertions(+), 52 deletions(-)
diff --git a/tests/vcpupin b/tests/vcpupin
index a616216..638f62b 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -35,7 +35,6 @@ $abs_top_builddir/tools/virsh --connect test:///default vcpupin test a
0,1 > out
test $? = 1 || fail=1
cat <<\EOF > exp || fail=1
error: Unable to parse integer parameter to --vcpu
-error: vcpupin: Invalid or missing vCPU number.
EOF
compare exp out || fail=1
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 73414f8..6e1eb17 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1120,7 +1120,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
if ((rv = vshCommandOptULongLong(cmd, "total-bytes-sec", &value)) <
0) {
- goto interror;
+ goto error;
} else if (rv > 0) {
if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC,
@@ -1129,7 +1129,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, "read-bytes-sec", &value)) <
0) {
- goto interror;
+ goto error;
} else if (rv > 0) {
if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC,
@@ -1138,7 +1138,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, "write-bytes-sec", &value)) <
0) {
- goto interror;
+ goto error;
} else if (rv > 0) {
if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC,
@@ -1147,7 +1147,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, "total-iops-sec", &value)) <
0) {
- goto interror;
+ goto error;
} else if (rv > 0) {
if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC,
@@ -1156,7 +1156,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, "read-iops-sec", &value)) < 0)
{
- goto interror;
+ goto error;
} else if (rv > 0) {
if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC,
@@ -1165,7 +1165,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, "write-iops-sec", &value)) <
0) {
- goto interror;
+ goto error;
} else if (rv > 0) {
if (virTypedParamsAddULLong(¶ms, &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;
}
/*
@@ -1316,7 +1312,6 @@ cmdBlkiotune(vshControl * ctl, const vshCmd * cmd)
return false;
if ((rv = vshCommandOptInt(cmd, "weight", &weight)) < 0) {
- vshError(ctl, "%s", _("Unable to parse integer parameter"));
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(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(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(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(cmd, "screen", &screen) < 0)
return false;
- }
if (!(dom = vshCommandOptDomain(ctl, cmd, &name)))
return false;
@@ -5820,8 +5807,6 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
/* In query mode, "vcpu" is optional */
if (vshCommandOptInt(cmd, "vcpu", &vcpu) < !query) {
- vshError(ctl, "%s",
- _("vcpupin: Invalid or missing vCPU number."));
virDomainFree(dom);
return false;
}
@@ -6089,7 +6074,10 @@ cmdSetvcpus(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptInt(cmd, "count", &count) < 0 || count <= 0) {
+ if (vshCommandOptInt(cmd, "count", &count) < 0)
+ goto cleanup;
+
+ if (count <= 0) {
vshError(ctl, "%s", _("Invalid number of virtual CPUs"));
goto cleanup;
}
@@ -6385,7 +6373,6 @@ 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"));
goto cleanup;
} else if (rv > 0) {
if (cpu < 0) {
@@ -6396,8 +6383,6 @@ cmdCPUStats(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptInt(cmd, "count", &show_count)) < 0) {
- vshError(ctl, "%s",
- _("Unable to parse integer parameter for CPUs to show"));
goto cleanup;
} else if (rv > 0) {
if (show_count < 0) {
@@ -7157,10 +7142,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(cmd, "holdtime", &holdtime) < 0)
goto cleanup;
- }
codeset = virKeycodeSetTypeFromString(codeset_option);
if (codeset < 0) {
@@ -7385,7 +7368,6 @@ cmdSetmem(vshControl *ctl, const vshCmd *cmd)
else
max = ULONG_MAX;
if (vshCommandOptScaledInt(cmd, "size", &bytes, 1024, max) < 0) {
- vshError(ctl, "%s", _("memory size has to be a number"));
virDomainFree(dom);
return false;
}
@@ -7482,7 +7464,6 @@ cmdSetmaxmem(vshControl *ctl, const vshCmd *cmd)
else
max = ULONG_MAX;
if (vshCommandOptScaledInt(cmd, "size", &bytes, 1024, max) < 0) {
- vshError(ctl, "%s", _("memory size has to be a number"));
virDomainFree(dom);
return false;
}
@@ -8124,10 +8105,8 @@ 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) {
- vshError(ctl, "%s", _("missing pid value"));
+ if (vshCommandOptUInt(cmd, "pid", &pid_value) <= 0)
goto cleanup;
- }
if (!(dom = virDomainQemuAttach(ctl->conn, pid_value, flags)))
goto cleanup;
@@ -8220,9 +8199,7 @@ cmdQemuAgentCommand(vshControl *ctl, const vshCmd *cmd)
}
guest_agent_cmd = virBufferContentAndReset(&buf);
- judge = vshCommandOptInt(cmd, "timeout", &timeout);
- if (judge < 0) {
- vshError(ctl, "%s", _("timeout number has to be a number"));
+ if ((judge = vshCommandOptInt(cmd, "timeout", &timeout)) < 0) {
goto cleanup;
} else if (judge > 0) {
judge = 1;
@@ -9095,8 +9072,10 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptLongLong(cmd, "downtime", &downtime) < 0 ||
- downtime < 1) {
+ if (vshCommandOptLongLong(cmd, "downtime", &downtime) < 0)
+ goto done;
+
+ if (downtime < 1) {
vshError(ctl, "%s", _("migrate: Invalid downtime"));
goto done;
}
@@ -9152,9 +9131,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(cmd, "size", &size)) < 0) {
goto cleanup;
} else if (rc != 0) {
if (virDomainMigrateSetCompressionCache(dom, size, 0) < 0)
@@ -9211,10 +9188,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(cmd, "bandwidth", &bandwidth) < 0)
goto done;
- }
if (virDomainMigrateSetMaxSpeed(dom, bandwidth, 0) < 0)
goto done;
@@ -11371,10 +11346,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(cmd, "minimum", &minimum) < 0)
goto cleanup;
- }
if (vshCommandOptStringReq(ctl, cmd, "mountpoint", &mountPoint) <
0)
goto cleanup;
--
1.9.0
4:12 a.m.
New subject: [libvirt] [PATCH 2/7] virsh-domain: Adapt to new error reporting
On 04/02/2014 02:43 PM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/vcpupin | 1 -
tools/virsh-domain.c | 75 +++++++++++++++++-----------------------------------
2 files changed, 24 insertions(+), 52 deletions(-)
diff --git a/tests/vcpupin b/tests/vcpupin
index a616216..638f62b 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -35,7 +35,6 @@ $abs_top_builddir/tools/virsh --connect test:///default vcpupin test a
0,1 > out
test $? = 1 || fail=1
cat <<\EOF > exp || fail=1
error: Unable to parse integer parameter to --vcpu
-error: vcpupin: Invalid or missing vCPU number.
EOF
compare exp out || fail=1
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 73414f8..6e1eb17 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1120,7 +1120,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
if ((rv = vshCommandOptULongLong(cmd, "total-bytes-sec", &value)) <
0) {
- goto interror;
+ goto error;
} else if (rv > 0) {
if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC,
@@ -1129,7 +1129,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, "read-bytes-sec", &value)) <
0) {
- goto interror;
+ goto error;
} else if (rv > 0) {
if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC,
@@ -1138,7 +1138,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, "write-bytes-sec", &value)) <
0) {
- goto interror;
+ goto error;
} else if (rv > 0) {
if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC,
@@ -1147,7 +1147,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, "total-iops-sec", &value)) <
0) {
- goto interror;
+ goto error;
} else if (rv > 0) {
if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC,
@@ -1156,7 +1156,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, "read-iops-sec", &value)) <
0) {
- goto interror;
+ goto error;
} else if (rv > 0) {
if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC,
@@ -1165,7 +1165,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, "write-iops-sec", &value)) <
0) {
- goto interror;
+ goto error;
} else if (rv > 0) {
if (virTypedParamsAddULLong(¶ms, &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;
}
These should jump to cleanup, not error.
Jan
8:09 a.m.
New subject: [libvirt] [PATCH 2/7] virsh-domain: Adapt to new error reporting
On 04.04.2014 11:12, Ján Tomko wrote:
On 04/02/2014 02:43 PM, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> tests/vcpupin | 1 -
> tools/virsh-domain.c | 75 +++++++++++++++++-----------------------------------
> 2 files changed, 24 insertions(+), 52 deletions(-)
>
> diff --git a/tests/vcpupin b/tests/vcpupin
> index a616216..638f62b 100755
> --- a/tests/vcpupin
> +++ b/tests/vcpupin
> @@ -35,7 +35,6 @@ $abs_top_builddir/tools/virsh --connect test:///default vcpupin
test a 0,1 > out
> test $? = 1 || fail=1
> cat <<\EOF > exp || fail=1
> error: Unable to parse integer parameter to --vcpu
> -error: vcpupin: Invalid or missing vCPU number.
>
> EOF
> compare exp out || fail=1
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 73414f8..6e1eb17 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -1120,7 +1120,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
> goto cleanup;
>
> if ((rv = vshCommandOptULongLong(cmd, "total-bytes-sec", &value))
< 0) {
> - goto interror;
> + goto error;
> } else if (rv > 0) {
> if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC,
> @@ -1129,7 +1129,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
> }
>
> if ((rv = vshCommandOptULongLong(cmd, "read-bytes-sec", &value))
< 0) {
> - goto interror;
> + goto error;
> } else if (rv > 0) {
> if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC,
> @@ -1138,7 +1138,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
> }
>
> if ((rv = vshCommandOptULongLong(cmd, "write-bytes-sec", &value))
< 0) {
> - goto interror;
> + goto error;
> } else if (rv > 0) {
> if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC,
> @@ -1147,7 +1147,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
> }
>
> if ((rv = vshCommandOptULongLong(cmd, "total-iops-sec", &value))
< 0) {
> - goto interror;
> + goto error;
> } else if (rv > 0) {
> if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC,
> @@ -1156,7 +1156,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
> }
>
> if ((rv = vshCommandOptULongLong(cmd, "read-iops-sec", &value))
< 0) {
> - goto interror;
> + goto error;
> } else if (rv > 0) {
> if (virTypedParamsAddULLong(¶ms, &nparams, &maxparams,
> VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC,
> @@ -1165,7 +1165,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
> }
>
> if ((rv = vshCommandOptULongLong(cmd, "write-iops-sec", &value))
< 0) {
> - goto interror;
> + goto error;
> } else if (rv > 0) {
> if (virTypedParamsAddULLong(¶ms, &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;
> }
>
These should jump to cleanup, not error.
Oh right, I've misread the label under interror. Since you're requesting
the function signature change in 1/7 I'll post v2. However, this time
it's going to be a single patch.
Michal
7:43 a.m.
New subject: [libvirt] [PATCH 3/7] virsh-domain-monitor: Adapt to new error reporting
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-domain-monitor.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 18d551a..f872f66 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -324,11 +324,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(cmd, "period", &period)) < 0) {
- vshError(ctl, "%s",
- _("Unable to parse integer parameter."));
+ if ((rv = vshCommandOptInt(cmd, "period", &period)) < 0)
goto cleanup;
- }
if (rv > 0) {
if (period < 0) {
vshError(ctl, _("Invalid collection period value '%d'"),
period);
--
1.9.0
7:43 a.m.
New subject: [libvirt] [PATCH 4/7] virsh-host: Adapt to new error reporting
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-host.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index cac6086..107aec5 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(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(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(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(cmd, "duration", &duration) < 0)
return false;
- }
if (STREQ(target, "mem"))
suspendTarget = VIR_NODE_SUSPEND_TARGET_MEM;
@@ -863,7 +855,6 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
size_t i;
if ((rc = vshCommandOptUInt(cmd, "shm-pages-to-scan", &value)) < 0)
{
- vshError(ctl, "%s", _("invalid shm-pages-to-scan number"));
goto cleanup;
} else if (rc > 0) {
if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams,
@@ -873,7 +864,6 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
}
if ((rc = vshCommandOptUInt(cmd, "shm-sleep-millisecs", &value)) <
0) {
- vshError(ctl, "%s", _("invalid shm-sleep-millisecs
number"));
goto cleanup;
} else if (rc > 0) {
if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams,
@@ -883,7 +873,6 @@ cmdNodeMemoryTune(vshControl *ctl, const vshCmd *cmd)
}
if ((rc = vshCommandOptUInt(cmd, "shm-merge-across-nodes", &value))
< 0) {
- vshError(ctl, "%s", _("invalid shm-merge-across-nodes
number"));
goto cleanup;
} else if (rc > 0) {
if (virTypedParamsAddUInt(¶ms, &nparams, &maxparams,
--
1.9.0
7:43 a.m.
New subject: [libvirt] [PATCH 5/7] virsh-interface: Adapt to new error reporting
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-interface.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c
index d4ec854..27820a7 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(cmd, "delay", &delay) < 0)
goto cleanup;
- }
nostart = vshCommandOptBool(cmd, "no-start");
--
1.9.0
7:43 a.m.
New subject: [libvirt] [PATCH 6/7] virsh-network: Adapt to new error reporting
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-network.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 4b0df62..3d202ce 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(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
--
1.9.0
7:44 a.m.
New subject: [libvirt] [PATCH 7/7] virsh-volume: Adapt to new error reporting
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tools/virsh-volume.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index 55bf6f0..71ff3b1 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(cmd, "offset", &offset) < 0)
return false;
- }
- if (vshCommandOptULongLong(cmd, "length", &length) < 0) {
- vshError(ctl, _("Unable to parse integer"));
+ if (vshCommandOptULongLong(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(cmd, "offset", &offset) < 0)
return false;
- }
- if (vshCommandOptULongLong(cmd, "length", &length) < 0) {
- vshError(ctl, _("Unable to parse integer"));
+ if (vshCommandOptULongLong(cmd, "length", &length) < 0)
return false;
- }
if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool",
&name)))
return false;
--
1.9.0
3885
days inactive
3887
days old
10 comments
2 participants
participants (2)
-
Ján Tomko -
Michal Privoznik