8:10 a.m.
I was so ashamed of writing a incorrect commit '848ab68' and didn't test with
it. Patch 1/2 fix the issue and Patch 2/2 introduce a test for it.
Luyao Huang (2):
virsh: really fix the error if vcpu number exceed the guest maxvcpu
number
test: introduce a function in test driver for check get vcpupin info
src/test/test_driver.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++
tests/vcpupin | 34 +++++++++++++++++++++++++++----
tools/virsh-domain.c | 4 ++--
3 files changed, 87 insertions(+), 6 deletions(-)
--
1.8.3.1
8:10 a.m.
New subject: [libvirt] [PATCH 1/2] virsh: really fix the error if vcpu number exceed the guest maxvcpu number
Commit '848ab68' left a issue: when we try to get a vcpupin info with no
no flags or --current flags, we still will get the wrong error. Because
VIR_DOMAIN_AFFECT_CURRENT is 0, so this check flags & VIR_DOMAIN_AFFECT_CURRENT
will always get false.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
Reported by: Peter Krempa <pkrempa(a)redhat.com>
---
tools/virsh-domain.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index ac04ded..00b012b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6499,8 +6499,8 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
if (got_vcpu && vcpu >= ncpus) {
if (flags & VIR_DOMAIN_AFFECT_LIVE ||
- (flags & VIR_DOMAIN_AFFECT_CURRENT &&
- virDomainIsActive(dom) == 1))
+ ((flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG))
+ == VIR_DOMAIN_AFFECT_CURRENT && virDomainIsActive(dom) == 1))
vshError(ctl,
_("vcpu %d is out of range of live cpu count %d"),
vcpu, ncpus);
--
1.8.3.1
4:21 p.m.
New subject: [libvirt] [PATCH 1/2] virsh: really fix the error if vcpu number exceed the guest maxvcpu number
On 07/14/2015 09:10 AM, Luyao Huang wrote:
Commit '848ab68' left a issue: when we try to get a vcpupin
info with no
no flags or --current flags, we still will get the wrong error. Because
VIR_DOMAIN_AFFECT_CURRENT is 0, so this check flags & VIR_DOMAIN_AFFECT_CURRENT
will always get false.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
Reported by: Peter Krempa <pkrempa(a)redhat.com>
---
tools/virsh-domain.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
hmmm ... see Michal's patch on this (and my response):
http://www.redhat.com/archives/libvir-list/2015-July/msg00555.html
Of course when I merged your change when I made the original adjustment
based on review, I never ran it through my coverity checker either).
John
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index ac04ded..00b012b 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6499,8 +6499,8 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
if (got_vcpu && vcpu >= ncpus) {
if (flags & VIR_DOMAIN_AFFECT_LIVE ||
- (flags & VIR_DOMAIN_AFFECT_CURRENT &&
- virDomainIsActive(dom) == 1))
+ ((flags & (VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG))
+ == VIR_DOMAIN_AFFECT_CURRENT && virDomainIsActive(dom) == 1))
vshError(ctl,
_("vcpu %d is out of range of live cpu count %d"),
vcpu, ncpus);
7:50 p.m.
New subject: [libvirt] [PATCH 1/2] virsh: really fix the error if vcpu number exceed the guest maxvcpu number
On 07/15/2015 05:21 AM, John Ferlan wrote:
On 07/14/2015 09:10 AM, Luyao Huang wrote:
> Commit '848ab68' left a issue: when we try to get a vcpupin info with no
> no flags or --current flags, we still will get the wrong error. Because
> VIR_DOMAIN_AFFECT_CURRENT is 0, so this check flags & VIR_DOMAIN_AFFECT_CURRENT
> will always get false.
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> Reported by: Peter Krempa <pkrempa(a)redhat.com>
> ---
> tools/virsh-domain.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
hmmm ... see Michal's patch on this (and my response):
http://www.redhat.com/archives/libvir-list/2015-July/msg00555.html
Of course when I merged your change when I made the original adjustment
based on review, I never ran it through my coverity checker either).
yes, Michal must notice the coverity either, so his patch will fix this
issue.
Thanks a lot for your review.
John
Luyao
8:10 a.m.
New subject: [libvirt] [PATCH 2/2] test: introduce a function in test driver to check get vcpupin info
As there is a regression in use vcpupin get info, introduce a new function
to test the virsh client.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/test/test_driver.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++
tests/vcpupin | 34 +++++++++++++++++++++++++++----
2 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index d38006f..213a9a1 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -2518,6 +2518,60 @@ static int testDomainPinVcpu(virDomainPtr domain,
return ret;
}
+static int
+testDomainGetVcpuPinInfo(virDomainPtr dom,
+ int ncpumaps,
+ unsigned char *cpumaps,
+ int maplen,
+ unsigned int flags)
+{
+ testDriverPtr privconn = dom->conn->privateData;
+ virDomainObjPtr privdom;
+ virDomainDefPtr def;
+ int ret = -1, hostcpus, vcpu;
+ virBitmapPtr allcpumap = NULL;
+
+ if (!(privdom = testDomObjFromDomain(dom)))
+ return -1;
+
+ if (!(def = virDomainObjGetOneDef(privdom, flags)))
+ goto cleanup;
+
+ hostcpus = VIR_NODEINFO_MAXCPUS(privconn->nodeInfo);
+
+ if (!(allcpumap = virBitmapNew(hostcpus)))
+ goto cleanup;
+
+ virBitmapSetAll(allcpumap);
+
+ /* Clamp to actual number of vcpus */
+ if (ncpumaps > def->vcpus)
+ ncpumaps = def->vcpus;
+
+ for (vcpu = 0; vcpu < ncpumaps; vcpu++) {
+ virDomainPinDefPtr pininfo;
+ virBitmapPtr bitmap = NULL;
+
+ pininfo = virDomainPinFind(def->cputune.vcpupin,
+ def->cputune.nvcpupin,
+ vcpu);
+
+ if (pininfo && pininfo->cpumask)
+ bitmap = pininfo->cpumask;
+ else
+ bitmap = allcpumap;
+
+ virBitmapToDataBuf(bitmap, VIR_GET_CPUMAP(cpumaps, maplen, vcpu), maplen);
+ }
+
+ ret = ncpumaps;
+
+ cleanup:
+ virBitmapFree(allcpumap);
+ virDomainObjEndAPI(&privdom);
+ return ret;
+}
+
static char *testDomainGetXMLDesc(virDomainPtr domain, unsigned int flags)
{
virDomainDefPtr def;
@@ -6598,6 +6652,7 @@ static virHypervisorDriver testHypervisorDriver = {
.domainGetVcpusFlags = testDomainGetVcpusFlags, /* 0.8.5 */
.domainPinVcpu = testDomainPinVcpu, /* 0.7.3 */
.domainGetVcpus = testDomainGetVcpus, /* 0.7.3 */
+ .domainGetVcpuPinInfo = testDomainGetVcpuPinInfo, /* 1.2.18 */
.domainGetMaxVcpus = testDomainGetMaxVcpus, /* 0.7.3 */
.domainGetXMLDesc = testDomainGetXMLDesc, /* 0.1.4 */
.connectListDefinedDomains = testConnectListDefinedDomains, /* 0.1.11 */
diff --git a/tests/vcpupin b/tests/vcpupin
index b6b8b31..213db93 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -66,12 +66,38 @@ 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
+# An out-of-range vCPU number when get information with live flag
+$abs_top_builddir/tools/virsh --connect test:///default vcpupin test 100 --live > out
2>&1
test $? = 1 || fail=1
cat <<\EOF > exp || fail=1
-error: this function is not supported by the connection driver: virDomainGetVcpuPinInfo
+error: vcpu 100 is out of range of live cpu count 2
+
+EOF
+compare exp out || fail=1
+
+# An out-of-range vCPU number when get information without flag
+$abs_top_builddir/tools/virsh --connect test:///default vcpupin test 100 > out
2>&1
+test $? = 1 || fail=1
+cat <<\EOF > exp || fail=1
+error: vcpu 100 is out of range of live cpu count 2
+
+EOF
+compare exp out || fail=1
+
+# An out-of-range vCPU number when get information with config flag
+$abs_top_builddir/tools/virsh --connect test:///default vcpupin test 100 --config >
out 2>&1
+test $? = 1 || fail=1
+cat <<\EOF > exp || fail=1
+error: vcpu 100 is out of range of persistent cpu count 2
+
+EOF
+compare exp out || fail=1
+
+# An out-of-range vCPU number when get information with current flag
+$abs_top_builddir/tools/virsh --connect test:///default vcpupin test 100 --current >
out 2>&1
+test $? = 1 || fail=1
+cat <<\EOF > exp || fail=1
+error: vcpu 100 is out of range of live cpu count 2
EOF
compare exp out || fail=1
--
1.8.3.1
5:52 a.m.
New subject: [libvirt] [PATCH 2/2] test: introduce a function in test driver to check get vcpupin info
On 07/14/2015 09:10 AM, Luyao Huang wrote:
As there is a regression in use vcpupin get info, introduce a new
function
to test the virsh client.
Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
---
src/test/test_driver.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++
tests/vcpupin | 34 +++++++++++++++++++++++++++----
2 files changed, 85 insertions(+), 4 deletions(-)
ACK and pushed the test (since 1/2 was fixed by a different commit)
John
8:07 p.m.
New subject: [libvirt] [PATCH 2/2] test: introduce a function in test driver to check get vcpupin info
On 07/24/2015 06:52 PM, John Ferlan wrote:
On 07/14/2015 09:10 AM, Luyao Huang wrote:
> As there is a regression in use vcpupin get info, introduce a new function
> to test the virsh client.
>
> Signed-off-by: Luyao Huang <lhuang(a)redhat.com>
> ---
> src/test/test_driver.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++
> tests/vcpupin | 34 +++++++++++++++++++++++++++----
> 2 files changed, 85 insertions(+), 4 deletions(-)
>
ACK and pushed the test (since 1/2 was fixed by a different commit)
Thanks a lot for your review.
John
Luyao
3408
days inactive
3421
days old
6 comments
3 participants
participants (3)
-
John Ferlan -
lhuang -
Luyao Huang