[v1] qemu: fix vcpupin fail as no vcpu tid set

[libvirt] [PATCH] qemu: fix vcpupin fail as no vcpu tid set

Posted by Wen Yang 5 years, 2 months ago

From: Yi Wang <wang.yi59@zte.com.cn>

vcpupin will fail when maxvcpus is larger than current
vcpu:

virsh vcpupin win7 --vcpu 0 --cpulist 5-6
error: Requested operation is not valid: cpu affinity is not supported

win7 xml in the command above is like below:
...
<vcpu current="3" placement="static">8</vcpu>
...

This issue is caused by qemuDomainRefreshVcpuInfo(), which mistake to
make validTIDs false and not set vcpu tid because vcpu[3] and vcpu[4]
have duplicate zero tids.

This patch fix this.

Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
---
 src/qemu/qemu_domain.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ac01e86..6987550 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -10701,6 +10701,9 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
      * impl which we can support.
      */
     for (i = 0; i < maxvcpus && validTIDs; i++) {
+        if (vm->def->virtType != VIR_DOMAIN_VIRT_QEMU)
+            break;
+
         if (info[i].tid == vm->pid) {
             VIR_DEBUG("vCPU[%zu] PID %llu duplicates process",
                       i, (unsigned long long)info[i].tid);
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] qemu: fix vcpupin fail as no vcpu tid set

Posted by Ján Tomko 5 years, 1 month ago

On Mon, Feb 18, 2019 at 05:21:31PM +0800, Wen Yang wrote:
>From: Yi Wang <wang.yi59@zte.com.cn>
>
>vcpupin will fail when maxvcpus is larger than current
>vcpu:
>
>virsh vcpupin win7 --vcpu 0 --cpulist 5-6
>error: Requested operation is not valid: cpu affinity is not supported
>
>win7 xml in the command above is like below:
>...
><vcpu current="3" placement="static">8</vcpu>
>...
>
>This issue is caused by qemuDomainRefreshVcpuInfo(), which mistake to
>make validTIDs false and not set vcpu tid because vcpu[3] and vcpu[4]
>have duplicate zero tids.

This problem description does not match the attempted fix.

>
>This patch fix this.
>
>Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
>---
> src/qemu/qemu_domain.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index ac01e86..6987550 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -10701,6 +10701,9 @@ qemuDomainRefreshVcpuInfo(virQEMUDriverPtr driver,
>      * impl which we can support.
>      */
>     for (i = 0; i < maxvcpus && validTIDs; i++) {
>+        if (vm->def->virtType != VIR_DOMAIN_VIRT_QEMU)
>+            break;
>+

This for loop was introduced by:
commit 34f77437da884f9cf0e2450f25f373b17cf527e2
    qemu: fix recording of vCPU pids for MTTCG

which started recording TIDs for TCG as well.
Skipping it for VIRT_QEMU reverts the benefits for multithreaded TCG,
while leaving the loop broken for VIRT_KVM when not all vCPUs are
enabled.

The proper fix here would be to stop considering zero TIDs as duplicate.

Jano

>         if (info[i].tid == vm->pid) {
>             VIR_DEBUG("vCPU[%zu] PID %llu duplicates process",
>                       i, (unsigned long long)info[i].tid);
>-- 
>1.8.3.1
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt][PATCH] qemu: fix vcpupin fail as no vcpu tid set

Posted by wang.yi59@zte.com.cn 5 years, 1 month ago

Hi Jano,
Thanks for your reply.

> On Mon, Feb 18, 2019 at 05:21:31PM +0800, Wen Yang wrote:
> >From: Yi Wang <wang.yi59@zte.com.cn>
> >
> >vcpupin will fail when maxvcpus is larger than current
> >vcpu:
> >

...

>
> This for loop was introduced by:
> commit 34f77437da884f9cf0e2450f25f373b17cf527e2
>     qemu: fix recording of vCPU pids for MTTCG
>
> which started recording TIDs for TCG as well.
> Skipping it for VIRT_QEMU reverts the benefits for multithreaded TCG,
> while leaving the loop broken for VIRT_KVM when not all vCPUs are
> enabled.
>
> The proper fix here would be to stop considering zero TIDs as duplicate.

Ok, I will send a v2 patch. Thanks.

>
> Jano
>
> >         if (info[i].tid == vm->pid) {
> >             VIR_DEBUG("vCPU[%zu] PID %llu duplicates process",
> >                       i, (unsigned long long)info[i].tid);
> >--
> >1.8.3.1
> >
> >--
> >libvir-list mailing list
> >libvir-list@redhat.com
>
> >https://www.redhat.com/mailman/listinfo/libvir-list


---
Best wishes
Yi Wang--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list