[Qemu-devel] [PATCH] cpu: skip unpluged cpu when querying cpus

[Qemu-devel] [PATCH] cpu: skip unpluged cpu when querying cpus

[linzhecheng]

From: XuYandong <xuyandong2@huawei.com>

After vcpu1 thread exiting, vcpu0 thread (received notification) is still waiting for
holding qemu_global_mutex in cpu_remove_sync, at this moment, vcpu1 is still in global cpus list.
If main thread grab qemu_global_mutex in order to handle qmp command "info cpus",
qmp_query_cpus visit unpluged vcpu1 will lead qemu process to exit.

Signed-off-by: XuYandong <xuyandong2@huawei.com>
---
 cpus.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/cpus.c b/cpus.c
index 2cb0af9..9b3a6c4 100644
--- a/cpus.c
+++ b/cpus.c
@@ -2018,6 +2018,11 @@ CpuInfoList *qmp_query_cpus(Error **errp)
 
     CPU_FOREACH(cpu) {
         CpuInfoList *info;
+
+        if (cpu->unplug) {
+            continue;
+        }
+
 #if defined(TARGET_I386)
         X86CPU *x86_cpu = X86_CPU(cpu);
         CPUX86State *env = &x86_cpu->env;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] cpu: skip unpluged cpu when querying cpus

[Igor Mammedov]

On Wed, 11 Apr 2018 19:16:02 +0800
linzhecheng <linzhecheng@huawei.com> wrote:

> From: XuYandong <xuyandong2@huawei.com>
> 
> After vcpu1 thread exiting, vcpu0 thread (received notification) is still waiting for
> holding qemu_global_mutex in cpu_remove_sync, at this moment, vcpu1 is still in global cpus list.
> If main thread grab qemu_global_mutex in order to handle qmp command "info cpus",
> qmp_query_cpus visit unpluged vcpu1 will lead qemu process to exit.
Add here exact error or better stack trace in case it crashes.

 
> Signed-off-by: XuYandong <xuyandong2@huawei.com>
> ---
>  cpus.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/cpus.c b/cpus.c
> index 2cb0af9..9b3a6c4 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -2018,6 +2018,11 @@ CpuInfoList *qmp_query_cpus(Error **errp)
>  
>      CPU_FOREACH(cpu) {
>          CpuInfoList *info;
> +
> +        if (cpu->unplug) {
> +            continue;
> +        }
Shouldn't be it done for qmp_query_cpus_fast() as well?

> +
>  #if defined(TARGET_I386)
>          X86CPU *x86_cpu = X86_CPU(cpu);
>          CPUX86State *env = &x86_cpu->env;

Re: [Qemu-devel] [PATCH] cpu: skip unpluged cpu when querying cpus

[xuyandong]

Yes, we should add it at qmp_query_cpus_fast() .
In this case, qmp_query_cpus() may kick a vcpu thread that has
stopped causing the qemu process to exit,  qmp_query_cpus_fast() does not 
interrupt vCPU threads, but may return invalid vCPU information.

> -----Original Message-----
> From: Igor Mammedov [mailto:imammedo@redhat.com]
> Sent: Wednesday, April 11, 2018 7:46 PM
> To: linzhecheng <linzhecheng@huawei.com>
> Cc: qemu-devel@nongnu.org; xuyandong <xuyandong2@huawei.com>;
> pbonzini@redhat.com; wangxin (U) <wangxinxin.wang@huawei.com>;
> rth@twiddle.net; crosthwaite.peter@gmail.com
> Subject: Re: [Qemu-devel] [PATCH] cpu: skip unpluged cpu when querying
> cpus
> 
> On Wed, 11 Apr 2018 19:16:02 +0800
> linzhecheng <linzhecheng@huawei.com> wrote:
> 
> > From: XuYandong <xuyandong2@huawei.com>
> >
> > After vcpu1 thread exiting, vcpu0 thread (received notification) is
> > still waiting for holding qemu_global_mutex in cpu_remove_sync, at this
> moment, vcpu1 is still in global cpus list.
> > If main thread grab qemu_global_mutex in order to handle qmp command
> > "info cpus", qmp_query_cpus visit unpluged vcpu1 will lead qemu process
> to exit.
> Add here exact error or better stack trace in case it crashes.
> 
> 
> > Signed-off-by: XuYandong <xuyandong2@huawei.com>
> > ---
> >  cpus.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/cpus.c b/cpus.c
> > index 2cb0af9..9b3a6c4 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -2018,6 +2018,11 @@ CpuInfoList *qmp_query_cpus(Error **errp)
> >
> >      CPU_FOREACH(cpu) {
> >          CpuInfoList *info;
> > +
> > +        if (cpu->unplug) {
> > +            continue;
> > +        }
> Shouldn't be it done for qmp_query_cpus_fast() as well?
> 
> > +
> >  #if defined(TARGET_I386)
> >          X86CPU *x86_cpu = X86_CPU(cpu);
> >          CPUX86State *env = &x86_cpu->env;