Re: [libvirt] [PATCH 1/1] Remove contiguous CPU indexes assumption
On 2013年02月28日 07:13, Eric Blake wrote:
On 02/27/2013 05:13 AM, Li Zhang wrote:
> Hi Eric,
>
> This should belong to bug-fix, could it be pushed to 1.0.3?
Without any test case under 'make check' that exposes the failure, I'm a
bit worried that this might cause regressions on other setups. I'm not
seven sure I even understand the scope of the problem. Is it something
specific to running a qemu-system-ppc64 emulator (but can be reproduced
on any host architecture), or is it specific to running on a powerpc
host (but happens for any version of qemu-* target architecture), or is
it something else altogether?
So sorry to see the failure.
I tried to 'make check' with my patch on ppc64, all of these cases pass.
For x86, I tried it with on x86 server, and pulled the latest version of
libvirt.
Even without my patch, it also fails during execute ./virshtest and the
tests with cpuset.
+++ out 2013-02-28 13:02:14.290794080 +0800
@@ -1,2 +1,3 @@
error: vcpupin: Invalid or missing vCPU number.
--- exp 2013-02-28 13:02:14.321794080 +0800
+++ out 2013-02-28 13:02:14.320794080 +0800
@@ -1,2 +1,3 @@
error: vcpupin: Invalid vCPU number.
FAIL: vcpupin
I think this problem is specific ppc64.
I did some experiment on X86.
X86 machine: 4 CPUs (0~3)
I disable CPU1 and CPU2 by writing 0 to /sys/.../cpuX/online.
Only 0 and 3 are online.
Although only 2 cpus are online, but after executing "query-cpus", it
can get all the information of them.
[{"return": [{"current": true, "CPU": 0, "pc":
4294967280, "halted":
false, "thread_id": 31831}, {"current": false, "CPU": 1,
"pc":
4294967280, "halted": false, "thread_id": 31832},
{"current": false,
"CPU": 2, "pc": 4294967280, "halted": false,
"thread_id": 31833},
{"current": false, "CPU": 3, "pc": 4294967280,
"halted": false,
"thread_id": 31834}], "id": "libvirt-3"}]
Qemu can expose all of these CPUs offline for X86.
But for ppc64, it can't expose these offline CPUs. The following is the
return value from ppc64.
[{"current": true, "CPU": 0, "nip": 256, "halted":
false, "thread_id":
25964}, {"current": false, "CPU": 4, "nip": 256,
"halted": true,
"thread_id": 25965}, {"current": false, "CPU": 8,
"nip": 256, "halted":
true, "thread_id": 25966}, {"current": false, "CPU": 12,
"nip": 256,
"halted": true, "thread_id": 25967}], "id":
"libvirt-3"}
We have test framework in place to allow replaying of a QMP JSON
response and seeing how qemu_monitor_json.c deals with it; what I'd
really like to see is a side-by-side comparison of what the QMP
'query-cpus' command returns for a guest with multiple vcpus on a setup
where you are seeing the problem, when compared to a setup that does not
have the issue. You can get at this with virsh qemu-monitor-command
$dom '{"execute":"query-cpus"}', if that helps.
Thanks
a lot. Tried with ppc64, it is different from x86 even with
some CPUs disabled.
To help you out, here's what I got for a guest using 3 vcpus on my
x86_64 host machine and using the qemu-kvm binary:
# virsh qemu-monitor-command guest
'{"execute":"query-cpus"}'
{"return":[{"current":true,"CPU":0,"pc":1025829,"halted":false,"thread_id":5849},{"current":false,"CPU":1,"pc":1005735,"halted":true,"thread_id":5850},{"current":false,"CPU":2,"pc":1005735,"halted":true,"thread_id":5851}],"id":"libvirt-9"}
That is, your patch might be right, but I have not yet been convinced
that it is right; and while things may currently be broken on ppc, it is
not a recent breakage, so being conservative and either proving the fix
is right (via testsuite addition) or deferring the fix until post 1.0.3
seems like safer alternatives. Or maybe someone else will chime in and
agree to take it now, without waiting for my desired burden of proof.
OK, I will tried to see the testsuite problems for x86 even without my
patch.
I couldn't think of what kind of problems will this patch cause now.
Really thank you, Eric for your review and suggestion.
I don't know whether others have suggestions about my patch.
Any suggestion is appreciated.
Thanks.
--Li