[libvirt] [PATCH] virsh: report error if vcpu number exceed the guest maxvcpu number

lhuang lhuang at redhat.com
Thu Jul 2 02:44:36 UTC 2015


On 07/02/2015 04:29 AM, John Ferlan wrote:
>
> On 06/28/2015 10:10 PM, Luyao Huang wrote:
>> If usr pass a vcpu which exceed guest maxvcpu number, virsh client
>> will only output an header like this:
>>
>>   # virsh vcpupin test3 1000
>>   VCPU: CPU Affinity
>>   ----------------------------------
>>
>> After this patch:
>>
>>   # virsh vcpupin test3 1000
>>   error: vcpu 1000 is out of range of cpu count 2
>>
>> Signed-off-by: Luyao Huang <lhuang at redhat.com>
>> ---
>>   tools/virsh-domain.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
> Seemed odd that this check wasn't there - so I did some digging...
>
> Pavel's commit id '81dd81e' removed a check that seems to be what is
> desired in this path (or was there before his change):
>
>      if (vcpu >= info.nrVirtCpu) {
>          vshError(ctl, "%s", _("vcpupin: vCPU index out of range."));
>          goto cleanup;
>          return false;
>      }
>
> As part of this commit, you'll note there was a test change in
> "tests/vcpupin":
>
>   # An out-of-range vCPU number deserves a diagnostic, too.
>   $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.
> +error: invalid argument: requested vcpu is higher than allocated vcpus
>
>
> Searching on their error message lands one in test_driver.c/
> testDomainPinVcpu().  So something specific for a set path, but the path
> you're hitting is the get path.

Yes, i noticed this and checked if need introduce a test or change the 
old test, but
i found test driver not support get vcpupin.

>
> FWIW: If a similar test was run on my system I get:
> # virsh vcpupin $dom 1000 0,1
> error: invalid argument: vcpu 1000 is out of range of live cpu count 2
>
> #
>
>
> So, if I understand everything that was done - then while your change is
> mostly correct, I think you could perhaps message the error similar to
> the vshCPUCountCollect failure (see the attached patch)

I saw the attached patch, but there is some problem about check the flag 
(actually
i had a try with check flags and output a better error before).

If check flags like vshCPUCountCollect failure, there will be a problem 
when do not
pass flag or just pass current flag to vcpupin, we will get error like 
this (pass a too big vcpu
number):

# virsh list;virsh vcpupin rhel7.0 1000 --current
  Id    Name                           State
----------------------------------------------------
  3     rhel7.0                        running

error: vcpu 1000 is out of range of persistent cpu count 4

In this case, we output "persistent" instead of "live", this is because 
vshCPUCountCollect()
cannot return certain flags (although there is a description say 
"Returns the count of vCPUs for a
domain and certain flags"). So we need more check for current flags, 
maybe like this :

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 27d62e9..334fd3a 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -6497,6 +6497,19 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
              goto cleanup;
          }

+        if (got_vcpu && vcpu >= ncpus) {
+            if (flags & VIR_DOMAIN_AFFECT_LIVE ||
+                (flags & VIR_DOMAIN_AFFECT_CURRENT && 
virDomainIsActive(dom) == 1))
+                vshError(ctl,
+                         _("vcpu %d is out of range of live cpu count %d"),
+                         vcpu, ncpus);
+            else
+                vshError(ctl,
+                         _("vcpu %d is out of range of persistent cpu 
count %d"),
+                         vcpu, ncpus);
+            goto cleanup;
+        }
+
          cpumaplen = VIR_CPU_MAPLEN(maxcpu);
          cpumap = vshMalloc(ctl, ncpus * cpumaplen);
          if ((ncpus = virDomainGetVcpuPinInfo(dom, ncpus, cpumap,

>
> Before I make that change for you - hopefully Pavel can take a look as
> well to be sure I haven't missed something.
>
> With any luck we this could be addressed before the 1.2.17 release, but
> if not since it's been a regression since 1.2.13 and no one's noticed,
> then another release probably won't hurt.

Right, if we can fix it in 1.2.17, it will be better :)

Thanks a lot for your help and review.

> John
>

Luyao




More information about the libvir-list mailing list