11:43 a.m.
Jincheng Miao (1):
virsh: forbid negative vcpu argument to vcpupin
Peter Krempa (3):
virsh: Reject negative numbers in vshCommandOptUInt
virsh: Reject negative numbers in vshCommandOptUL
virsh: Reject negative numbers in vshCommandOptULongLong
tests/vcpupin | 29 ++++++++++-
tools/virsh-domain.c | 39 ++++++++-------
tools/virsh-volume.c | 8 +--
tools/virsh.c | 137 +++++++++++++++++++++++++++++++++++++++++----------
tools/virsh.h | 9 ++++
5 files changed, 172 insertions(+), 50 deletions(-)
--
1.9.3
11:43 a.m.
New subject: [libvirt] [PATCHv4 1/4] virsh: Reject negative numbers in vshCommandOptUInt
Use virStrToLong_uip instead of virStrToLong_ui to reject negative
numbers in the helper. None of the callers expects the wraparound
"feature" for negative numbers.
Also add a function that allows wrapping of negative numbers as it might
be used in the future and be explicit about the new semantics in the
function docs.
---
tools/virsh.c | 48 ++++++++++++++++++++++++++++++++++++++----------
tools/virsh.h | 3 +++
2 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index 15f3025..cbab6b0 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1494,6 +1494,28 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
return 1;
}
+static int
+vshCommandOptUIntInternal(const vshCmd *cmd,
+ const char *name,
+ unsigned int *value,
+ bool wrap)
+{
+ vshCmdOpt *arg;
+ int ret;
+
+ 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;
+ }
+
+ return 1;
+}
/**
* vshCommandOptUInt:
@@ -1501,22 +1523,28 @@ vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
* @name option name
* @value result
*
- * Convert option to unsigned int
+ * Convert option to unsigned int, reject negative numbers
* See vshCommandOptInt()
*/
int
vshCommandOptUInt(const vshCmd *cmd, const char *name, unsigned int *value)
{
- vshCmdOpt *arg;
- int ret;
-
- ret = vshCommandOpt(cmd, name, &arg, true);
- if (ret <= 0)
- return ret;
+ return vshCommandOptUIntInternal(cmd, name, value, false);
+}
- if (virStrToLong_ui(arg->data, NULL, 10, value) < 0)
- return -1;
- return 1;
+/**
+ * vshCommandOptUIntWrap:
+ * @cmd command reference
+ * @name option name
+ * @value result
+ *
+ * Convert option to unsigned int, wraps negative numbers to positive
+ * See vshCommandOptInt()
+ */
+int
+vshCommandOptUIntWrap(const vshCmd *cmd, const char *name, unsigned int *value)
+{
+ return vshCommandOptUIntInternal(cmd, name, value, true);
}
diff --git a/tools/virsh.h b/tools/virsh.h
index 1ffc003..9afbbb5 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -285,6 +285,9 @@ int vshCommandOptInt(const vshCmd *cmd, const char *name, int *value)
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;
--
1.9.3
11:43 a.m.
New subject: [libvirt] [PATCHv4 2/4] virsh: Reject negative numbers in vshCommandOptUL
To follow the new semantics of the vshCommandOptToU* functions convert
this one to reject negative numbers too. To allow using -1 for "maximum"
semantics for the two bandwidth functions that use this helper introduce
vshCommandOptULWrap. Although currently the migrate-setspeed function
for the qemu driver will reject -1 as maximum.
---
tools/virsh-domain.c | 4 ++--
tools/virsh.c | 46 +++++++++++++++++++++++++++++++++++++---------
tools/virsh.h | 3 +++
3 files changed, 42 insertions(+), 11 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 84a6706..d76865b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1457,7 +1457,7 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
if (vshCommandOptStringReq(ctl, cmd, "path", &path) < 0)
goto cleanup;
- if (vshCommandOptUL(cmd, "bandwidth", &bandwidth) < 0) {
+ if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) {
vshError(ctl, "%s", _("bandwidth must be a number"));
goto cleanup;
}
@@ -9223,7 +9223,7 @@ cmdMigrateSetMaxSpeed(vshControl *ctl, const vshCmd *cmd)
if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
return false;
- if (vshCommandOptUL(cmd, "bandwidth", &bandwidth) < 0) {
+ if (vshCommandOptULWrap(cmd, "bandwidth", &bandwidth) < 0) {
vshError(ctl, "%s", _("migrate: Invalid bandwidth"));
goto done;
}
diff --git a/tools/virsh.c b/tools/virsh.c
index cbab6b0..77a4f99 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1547,6 +1547,28 @@ vshCommandOptUIntWrap(const vshCmd *cmd, const char *name, unsigned
int *value)
return vshCommandOptUIntInternal(cmd, name, value, true);
}
+static int
+vshCommandOptULInternal(const vshCmd *cmd,
+ const char *name,
+ unsigned long *value,
+ bool wrap)
+{
+ vshCmdOpt *arg;
+ int ret;
+
+ 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;
+ }
+
+ return 1;
+}
/*
* vshCommandOptUL:
@@ -1560,16 +1582,22 @@ vshCommandOptUIntWrap(const vshCmd *cmd, const char *name,
unsigned int *value)
int
vshCommandOptUL(const vshCmd *cmd, const char *name, unsigned long *value)
{
- vshCmdOpt *arg;
- int ret;
-
- ret = vshCommandOpt(cmd, name, &arg, true);
- if (ret <= 0)
- return ret;
+ return vshCommandOptULInternal(cmd, name, value, false);
+}
- if (virStrToLong_ul(arg->data, NULL, 10, value) < 0)
- return -1;
- return 1;
+/**
+ * vshCommandOptULWrap:
+ * @cmd command reference
+ * @name option name
+ * @value result
+ *
+ * Convert option to unsigned long, wraps negative numbers to positive
+ * See vshCommandOptInt()
+ */
+int
+vshCommandOptULWrap(const vshCmd *cmd, const char *name, unsigned long *value)
+{
+ return vshCommandOptULInternal(cmd, name, value, true);
}
/**
diff --git a/tools/virsh.h b/tools/virsh.h
index 9afbbb5..4f5c336 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -291,6 +291,9 @@ int vshCommandOptUIntWrap(const vshCmd *cmd, const char *name,
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;
--
1.9.3
11:43 a.m.
New subject: [libvirt] [PATCHv4 3/4] virsh: Reject negative numbers in vshCommandOptULongLong
To follow the new semantics of the vshCommandOptToU* functions convert
this one to reject negative numbers too. To allow using -1 for "maximum"
semantics for the vol-*load two bandwidth functions that use this helper
introduce vshCommandOptULongLongWrap.
---
tools/virsh-volume.c | 8 ++++----
tools/virsh.c | 51 ++++++++++++++++++++++++++++++++++++++++-----------
tools/virsh.h | 3 +++
3 files changed, 47 insertions(+), 15 deletions(-)
diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c
index f284fa5..724a86b 100644
--- a/tools/virsh-volume.c
+++ b/tools/virsh-volume.c
@@ -677,12 +677,12 @@ cmdVolUpload(vshControl *ctl, const vshCmd *cmd)
const char *name = NULL;
unsigned long long offset = 0, length = 0;
- if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) {
+ if (vshCommandOptULongLongWrap(cmd, "offset", &offset) < 0) {
vshError(ctl, _("Unable to parse integer"));
return false;
}
- if (vshCommandOptULongLong(cmd, "length", &length) < 0) {
+ if (vshCommandOptULongLongWrap(cmd, "length", &length) < 0) {
vshError(ctl, _("Unable to parse integer"));
return false;
}
@@ -787,12 +787,12 @@ cmdVolDownload(vshControl *ctl, const vshCmd *cmd)
unsigned long long offset = 0, length = 0;
bool created = false;
- if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) {
+ if (vshCommandOptULongLongWrap(cmd, "offset", &offset) < 0) {
vshError(ctl, _("Unable to parse integer"));
return false;
}
- if (vshCommandOptULongLong(cmd, "length", &length) < 0) {
+ if (vshCommandOptULongLongWrap(cmd, "length", &length) < 0) {
vshError(ctl, _("Unable to parse integer"));
return false;
}
diff --git a/tools/virsh.c b/tools/virsh.c
index 77a4f99..82669b0 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1699,31 +1699,60 @@ vshCommandOptLongLong(const vshCmd *cmd, const char *name,
return 1;
}
+static int
+vshCommandOptULongLongInternal(const vshCmd *cmd,
+ const char *name,
+ unsigned long long *value,
+ bool wrap)
+{
+ vshCmdOpt *arg;
+ int ret;
+
+ 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;
+ }
+
+ return 1;
+}
+
/**
* vshCommandOptULongLong:
* @cmd command reference
* @name option name
* @value result
*
- * Returns option as long long
+ * Returns option as long long, rejects negative numbers
* See vshCommandOptInt()
*/
int
vshCommandOptULongLong(const vshCmd *cmd, const char *name,
unsigned long long *value)
{
- vshCmdOpt *arg;
- int ret;
-
- ret = vshCommandOpt(cmd, name, &arg, true);
- if (ret <= 0)
- return ret;
-
- if (virStrToLong_ull(arg->data, NULL, 10, value) < 0)
- return -1;
- return 1;
+ return vshCommandOptULongLongInternal(cmd, name, value, false);
}
+/**
+ * vshCommandOptULongLongWrap:
+ * @cmd command reference
+ * @name option name
+ * @value result
+ *
+ * Returns option as long long, wraps negative numbers to positive
+ * See vshCommandOptInt()
+ */
+int
+vshCommandOptULongLongWrap(const vshCmd *cmd, const char *name,
+ unsigned long long *value)
+{
+ return vshCommandOptULongLongInternal(cmd, name, value, true);
+}
/**
* vshCommandOptScaledInt:
diff --git a/tools/virsh.h b/tools/virsh.h
index 4f5c336..7656407 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -307,6 +307,9 @@ int vshCommandOptLongLong(const vshCmd *cmd, const char *name,
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)
--
1.9.3
11:43 a.m.
New subject: [libvirt] [PATCHv4 4/4] virsh: forbid negative vcpu argument to vcpupin
From: Jincheng Miao <jmiao(a)redhat.com>
The vcpupin command allowed specifying a negative number for the --vcpu
argument. This would the overflow when the underlying virDomainPinVcpu
API was called.
$ virsh vcpupin r7 -1 0
error: numerical overflow: input too large: 4294967295
Switch the vCPU variable to a unsigned int and parse it using the
corresponding function.
Also improve the vcpupin test to cover all the defects.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1101059
Signed-off-by: Jincheng Miao <jmiao(a)redhat.com>
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/vcpupin | 29 ++++++++++++++++++++++++++++-
tools/virsh-domain.c | 35 ++++++++++++++++++-----------------
2 files changed, 46 insertions(+), 18 deletions(-)
diff --git a/tests/vcpupin b/tests/vcpupin
index f1fb038..9f34ec0 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -34,7 +34,7 @@ fail=0
$abs_top_builddir/tools/virsh --connect test:///default vcpupin test a 0,1 > out
2>&1
test $? = 1 || fail=1
cat <<\EOF > exp || fail=1
-error: vcpupin: Invalid or missing vCPU number.
+error: vcpupin: Invalid vCPU number.
EOF
compare exp out || fail=1
@@ -43,9 +43,36 @@ compare exp out || fail=1
$abs_top_builddir/tools/virsh --connect test:///default vcpupin test 100 0,1 > out
2>&1
test $? = 1 || fail=1
cat <<\EOF > exp || fail=1
+error: vcpupin: vCPU index out of range.
+
+EOF
+compare exp out || fail=1
+
+# Negative number
+$abs_top_builddir/tools/virsh --connect test:///default vcpupin test -100 0,1 > out
2>&1
+test $? = 1 || fail=1
+cat <<\EOF > exp || fail=1
error: vcpupin: Invalid vCPU number.
EOF
compare exp out || fail=1
+# missing argument
+$abs_top_builddir/tools/virsh --connect test:///default vcpupin test --cpulist 0,1 >
out 2>&1
+test $? = 1 || fail=1
+cat <<\EOF > exp || fail=1
+error: vcpupin: Missing vCPU number in pin mode.
+
+EOF
+compare exp out || fail=1
+
+# without arguments. This should succeed but the backend function in the
+# test driver isn't implemented
+$abs_top_builddir/tools/virsh --connect test:///default vcpupin test > out
2>&1
+test $? = 1 || fail=1
+cat <<\EOF > exp || fail=1
+error: this function is not supported by the connection driver: virDomainGetVcpuPinInfo
+
+EOF
+compare exp out || fail=1
(exit $fail); exit $fail
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d76865b..e43c380 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -5797,7 +5797,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
{
virDomainInfo info;
virDomainPtr dom;
- int vcpu = -1;
+ unsigned int vcpu = 0;
const char *cpulist = NULL;
bool ret = false;
unsigned char *cpumap = NULL;
@@ -5809,6 +5809,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
bool live = vshCommandOptBool(cmd, "live");
bool current = vshCommandOptBool(cmd, "current");
bool query = false; /* Query mode if no cpulist */
+ int got_vcpu;
unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
VSH_EXCLUSIVE_OPTIONS_VAR(current, live);
@@ -5830,29 +5831,29 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
query = !cpulist;
- /* In query mode, "vcpu" is optional */
- if (vshCommandOptInt(cmd, "vcpu", &vcpu) < !query) {
- vshError(ctl, "%s",
- _("vcpupin: Invalid or missing vCPU number."));
- virDomainFree(dom);
- return false;
+ if ((got_vcpu = vshCommandOptUInt(cmd, "vcpu", &vcpu)) < 0) {
+ vshError(ctl, "%s", _("vcpupin: Invalid vCPU number."));
+ goto cleanup;
}
- if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0) {
- virDomainFree(dom);
- return false;
+ /* In pin mode, "vcpu" is necessary */
+ if (!query && got_vcpu == 0) {
+ vshError(ctl, "%s", _("vcpupin: Missing vCPU number in pin
mode."));
+ goto cleanup;
}
if (virDomainGetInfo(dom, &info) != 0) {
vshError(ctl, "%s", _("vcpupin: failed to get domain
information."));
- virDomainFree(dom);
- return false;
+ goto cleanup;
}
if (vcpu >= info.nrVirtCpu) {
- vshError(ctl, "%s", _("vcpupin: Invalid vCPU number."));
- virDomainFree(dom);
- return false;
+ vshError(ctl, "%s", _("vcpupin: vCPU index out of range."));
+ goto cleanup;
+ }
+
+ if ((maxcpu = vshNodeGetCPUCount(ctl->conn)) < 0) {
+ goto cleanup;
}
cpumaplen = VIR_CPU_MAPLEN(maxcpu);
@@ -5871,7 +5872,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
vshPrintExtra(ctl, "%s %s\n", _("VCPU:"), _("CPU
Affinity"));
vshPrintExtra(ctl, "----------------------------------\n");
for (i = 0; i < ncpus; i++) {
- if (vcpu != -1 && i != vcpu)
+ if (got_vcpu && i != vcpu)
continue;
vshPrint(ctl, "%4zu: ", i);
@@ -5880,8 +5881,8 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
if (!ret)
break;
}
-
}
+
VIR_FREE(cpumaps);
goto cleanup;
}
--
1.9.3
2:05 p.m.
On 06/04/2014 11:43 AM, Peter Krempa wrote:
Jincheng Miao (1):
virsh: forbid negative vcpu argument to vcpupin
Peter Krempa (3):
virsh: Reject negative numbers in vshCommandOptUInt
virsh: Reject negative numbers in vshCommandOptUL
virsh: Reject negative numbers in vshCommandOptULongLong
tests/vcpupin | 29 ++++++++++-
tools/virsh-domain.c | 39 ++++++++-------
tools/virsh-volume.c | 8 +--
tools/virsh.c | 137 +++++++++++++++++++++++++++++++++++++++++----------
tools/virsh.h | 9 ++++
5 files changed, 172 insertions(+), 50 deletions(-)
ACK
2:13 p.m.
On 06/12/14 14:05, Ján Tomko wrote:
On 06/04/2014 11:43 AM, Peter Krempa wrote:
> Jincheng Miao (1):
> virsh: forbid negative vcpu argument to vcpupin
>
> Peter Krempa (3):
> virsh: Reject negative numbers in vshCommandOptUInt
> virsh: Reject negative numbers in vshCommandOptUL
> virsh: Reject negative numbers in vshCommandOptULongLong
>
> tests/vcpupin | 29 ++++++++++-
> tools/virsh-domain.c | 39 ++++++++-------
> tools/virsh-volume.c | 8 +--
> tools/virsh.c | 137 +++++++++++++++++++++++++++++++++++++++++----------
> tools/virsh.h | 9 ++++
> 5 files changed, 172 insertions(+), 50 deletions(-)
>
ACK
Series pushed; Thanks.
Peter
3753
days inactive
3761
days old
6 comments
2 participants
participants (2)
-
Ján Tomko -
Peter Krempa