[Qemu-devel] [PATCH] vl: take iothread lock very early

[Qemu-devel] [PATCH] vl: take iothread lock very early

[Paolo Bonzini]

If the iothread lock isn't taken by the main thread, the RCU callbacks
might run concurrently with the main thread.  QEMU's not ready for that.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/vl.c b/vl.c
index 09693f5..d743727 100644
--- a/vl.c
+++ b/vl.c
@@ -2763,6 +2763,9 @@ int main(int argc, char **argv, char **envp)
     FILE *vmstate_dump_file = NULL;
     Error *main_loop_err = NULL;
 
+    qemu_init_cpu_loop();
+    qemu_mutex_lock_iothread();
+
     atexit(qemu_run_exit_notifiers);
     error_set_progname(argv[0]);
     qemu_init_exec_dir(argv[0]);
@@ -4098,9 +4101,6 @@ int main(int argc, char **argv, char **envp)
 
     os_set_line_buffering();
 
-    qemu_init_cpu_loop();
-    qemu_mutex_lock_iothread();
-
 #ifdef CONFIG_SPICE
     /* spice needs the timers to be initialized by this point */
     qemu_spice_init();
-- 
2.3.0

Re: [Qemu-devel] [PATCH] vl: take iothread lock very early

[Christian Borntraeger]

Am 03.03.2015 um 11:36 schrieb Paolo Bonzini:
> If the iothread lock isn't taken by the main thread, the RCU callbacks
> might run concurrently with the main thread.  QEMU's not ready for that.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>


> ---
>  vl.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index 09693f5..d743727 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2763,6 +2763,9 @@ int main(int argc, char **argv, char **envp)
>      FILE *vmstate_dump_file = NULL;
>      Error *main_loop_err = NULL;
> 
> +    qemu_init_cpu_loop();
> +    qemu_mutex_lock_iothread();
> +
>      atexit(qemu_run_exit_notifiers);
>      error_set_progname(argv[0]);
>      qemu_init_exec_dir(argv[0]);
> @@ -4098,9 +4101,6 @@ int main(int argc, char **argv, char **envp)
> 
>      os_set_line_buffering();
> 
> -    qemu_init_cpu_loop();
> -    qemu_mutex_lock_iothread();
> -
>  #ifdef CONFIG_SPICE
>      /* spice needs the timers to be initialized by this point */
>      qemu_spice_init();
> 

Re: [Qemu-devel] [PATCH] vl: take iothread lock very early

[Gonglei]

On 2015/3/3 18:36, Paolo Bonzini wrote:
> If the iothread lock isn't taken by the main thread, the RCU callbacks
> might run concurrently with the main thread.  QEMU's not ready for that.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  vl.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
Tested-by: Gonglei <arei.gonglei@huawei.com>

Regards,
-Gonglei

Re: [Qemu-devel] [PATCH] vl: take iothread lock very early

[Peter Maydell]

On 3 March 2015 at 19:36, Paolo Bonzini <pbonzini@redhat.com> wrote:
> If the iothread lock isn't taken by the main thread, the RCU callbacks
> might run concurrently with the main thread.  QEMU's not ready for that.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] vl: take iothread lock very early

[Oliver Francke]

Hi all,

just brief, this small one breaks my qemu-system-x86_64 with:

     qemu: qemu_cond_wait: Operation not permitted

My most complete qemu-starter:

/usr/local/qemu-2.3.0/bin/qemu-system-x86_64 -daemonize -usbdevice 
tablet -enable-kvm -pidfile /var/run/qemu-server/3371.pid -monitor 
unix:/var/run/qemu-server/3371.mon,server,nowait -vnc 
unix:/var/run/qemu-server/3371.vnc,password -qmp 
unix:/var/run/qemu-server/3371.qmp,server,nowait -nodefaults -serial 
none -parallel none -device rtl8139,mac=00:F1:70:00:D2:B0,netdev=vlan0d0 
-netdev 
type=tap,id=vlan0d0,ifname=tap3371i0d0,script=/etc/fcms/add_if.sh,downscript=/etc/fcms/downscript.sh 
-name 1155823384-7 -vga cirrus -k de -smp sockets=1,cores=1 -m 512 
-device virtio-blk-pci,drive=virtio0 -drive 
format=raw,file=rbd:4/vm-3371-disk-1.rbd:rbd_cache=true:rbd_cache_size=16777216:rbd_cache_max_dirty=12582912:rbd_cache_target_dirty=8388608,cache=writeback,if=none,id=virtio0,media=disk,index=0 
-drive 
format=raw,file=rbd:4/vm-3371-swap-1.rbd:rbd_cache=true:rbd_cache_size=16777216:rbd_cache_max_dirty=12582912:rbd_cache_target_dirty=8388608,cache=writeback,if=virtio,media=disk,index=1 
-drive if=ide,media=cdrom,id=ide1-cd0,readonly=on -drive 
if=ide,media=cdrom,id=ide1-cd1,readonly=on -boot order=dc

Up to the former commit everything works well as expected.

Hope it helps,

Oliver.


On 03/03/2015 11:36 AM, Paolo Bonzini wrote:
> If the iothread lock isn't taken by the main thread, the RCU callbacks
> might run concurrently with the main thread.  QEMU's not ready for that.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   vl.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/vl.c b/vl.c
> index 09693f5..d743727 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2763,6 +2763,9 @@ int main(int argc, char **argv, char **envp)
>       FILE *vmstate_dump_file = NULL;
>       Error *main_loop_err = NULL;
>   
> +    qemu_init_cpu_loop();
> +    qemu_mutex_lock_iothread();
> +
>       atexit(qemu_run_exit_notifiers);
>       error_set_progname(argv[0]);
>       qemu_init_exec_dir(argv[0]);
> @@ -4098,9 +4101,6 @@ int main(int argc, char **argv, char **envp)
>   
>       os_set_line_buffering();
>   
> -    qemu_init_cpu_loop();
> -    qemu_mutex_lock_iothread();
> -
>   #ifdef CONFIG_SPICE
>       /* spice needs the timers to be initialized by this point */
>       qemu_spice_init();


-- 

Oliver Francke

filoo GmbH
Moltkestraße 25a
33330 Gütersloh
HRB4355 AG Gütersloh

Geschäftsführer: J.Rehpöhler | C.Kunz

Folgen Sie uns auf Twitter: http://twitter.com/filoogmbh

Re: [Qemu-devel] [PATCH] vl: take iothread lock very early

[Paolo Bonzini]

On 03/03/2015 17:03, Oliver Francke wrote:
> 
> /usr/local/qemu-2.3.0/bin/qemu-system-x86_64 -daemonize -usbdevice
> tablet -enable-kvm -pidfile /var/run/qemu-server/3371.pid -monitor
> unix:/var/run/qemu-server/3371.mon,server,nowait -vnc
> unix:/var/run/qemu-server/3371.vnc,password -qmp
> unix:/var/run/qemu-server/3371.qmp,server,nowait -nodefaults -serial
> none -parallel none -device rtl8139,mac=00:F1:70:00:D2:B0,netdev=vlan0d0
> -netdev
> type=tap,id=vlan0d0,ifname=tap3371i0d0,script=/etc/fcms/add_if.sh,downscript=/etc/fcms/downscript.sh
> -name 1155823384-7 -vga cirrus -k de -smp sockets=1,cores=1 -m 512
> -device virtio-blk-pci,drive=virtio0 -drive
> format=raw,file=rbd:4/vm-3371-disk-1.rbd:rbd_cache=true:rbd_cache_size=16777216:rbd_cache_max_dirty=12582912:rbd_cache_target_dirty=8388608,cache=writeback,if=none,id=virtio0,media=disk,index=0
> -drive
> format=raw,file=rbd:4/vm-3371-swap-1.rbd:rbd_cache=true:rbd_cache_size=16777216:rbd_cache_max_dirty=12582912:rbd_cache_target_dirty=8388608,cache=writeback,if=virtio,media=disk,index=1
> -drive if=ide,media=cdrom,id=ide1-cd0,readonly=on -drive
> if=ide,media=cdrom,id=ide1-cd1,readonly=on -boot order=dc
> 
> Up to the former commit everything works well as expected.

Can you gather a backtrace?

Paolo

Re: [Qemu-devel] [PATCH] vl: take iothread lock very early

[Oliver Francke]

First bt try below...

On 03/03/2015 05:51 PM, Paolo Bonzini wrote:
>
> On 03/03/2015 17:03, Oliver Francke wrote:
>> /usr/local/qemu-2.3.0/bin/qemu-system-x86_64 -daemonize -usbdevice
>> tablet -enable-kvm -pidfile /var/run/qemu-server/3371.pid -monitor
>> unix:/var/run/qemu-server/3371.mon,server,nowait -vnc
>> unix:/var/run/qemu-server/3371.vnc,password -qmp
>> unix:/var/run/qemu-server/3371.qmp,server,nowait -nodefaults -serial
>> none -parallel none -device rtl8139,mac=00:F1:70:00:D2:B0,netdev=vlan0d0
>> -netdev
>> type=tap,id=vlan0d0,ifname=tap3371i0d0,script=/etc/fcms/add_if.sh,downscript=/etc/fcms/downscript.sh
>> -name 1155823384-7 -vga cirrus -k de -smp sockets=1,cores=1 -m 512
>> -device virtio-blk-pci,drive=virtio0 -drive
>> format=raw,file=rbd:4/vm-3371-disk-1.rbd:rbd_cache=true:rbd_cache_size=16777216:rbd_cache_max_dirty=12582912:rbd_cache_target_dirty=8388608,cache=writeback,if=none,id=virtio0,media=disk,index=0
>> -drive
>> format=raw,file=rbd:4/vm-3371-swap-1.rbd:rbd_cache=true:rbd_cache_size=16777216:rbd_cache_max_dirty=12582912:rbd_cache_target_dirty=8388608,cache=writeback,if=virtio,media=disk,index=1
>> -drive if=ide,media=cdrom,id=ide1-cd0,readonly=on -drive
>> if=ide,media=cdrom,id=ide1-cd1,readonly=on -boot order=dc
>>
>> Up to the former commit everything works well as expected.
> Can you gather a backtrace?
(gdb) bt
#0  __GI_exit (status=1) at exit.c:104
#1  0x000055555575cf15 in os_daemonize () at os-posix.c:227
#2  0x0000555555773f2e in main (argc=50, argv=0x7fffffffe0d8, 
envp=0x7fffffffe270) at vl.c:3770

compiled with "--enable-debug" and in gdb "b exit". Not a coder, so if 
this is _not_ sufficient, please give me some adivce ;)

Oliver.

>
> Paolo


-- 

Oliver Francke

filoo GmbH
Moltkestraße 25a
33330 Gütersloh
HRB4355 AG Gütersloh

Geschäftsführer: J.Rehpöhler | C.Kunz

Folgen Sie uns auf Twitter: http://twitter.com/filoogmbh

Re: [Qemu-devel] [PATCH] vl: take iothread lock very early

[Paolo Bonzini]

On 03/03/2015 18:19, Oliver Francke wrote:
> 
> #0  __GI_exit (status=1) at exit.c:104
> #1  0x000055555575cf15 in os_daemonize () at os-posix.c:227
> #2  0x0000555555773f2e in main (argc=50, argv=0x7fffffffe0d8,
> envp=0x7fffffffe270) at vl.c:3770
> 
> compiled with "--enable-debug" and in gdb "b exit". Not a coder, so if
> this is _not_ sufficient, please give me some adivce ;)

Try removing -daemonize (hmm, that might actually fix the bug).  You
should not need "b exit", because the error is a SIGABRT and gdb stops
automatically.  The fix could be something like this:

diff --git a/vl.c b/vl.c
index e1ffd0a..af61835 100644
--- a/vl.c
+++ b/vl.c
@@ -3759,7 +3759,9 @@ int main(int argc, char **argv, char **envp)

     loc_set_none();

+    qemu_mutex_unlock_iothread();
     os_daemonize();
+    qemu_mutex_lock_iothread();

     if (qemu_init_main_loop(&main_loop_err)) {
         error_report_err(main_loop_err);

Paolo

Re: [Qemu-devel] [PATCH] vl: take iothread lock very early

[Oliver Francke]

Hi,

> Am 03.03.2015 um 18:32 schrieb Paolo Bonzini <pbonzini@redhat.com>:
> 
> 
> 
> On 03/03/2015 18:19, Oliver Francke wrote:
>> 
>> #0  __GI_exit (status=1) at exit.c:104
>> #1  0x000055555575cf15 in os_daemonize () at os-posix.c:227
>> #2  0x0000555555773f2e in main (argc=50, argv=0x7fffffffe0d8,
>> envp=0x7fffffffe270) at vl.c:3770
>> 
>> compiled with "--enable-debug" and in gdb "b exit". Not a coder, so if
>> this is _not_ sufficient, please give me some adivce ;)
> 
> Try removing -daemonize (hmm, that might actually fix the bug).  You
> should not need "b exit", because the error is a SIGABRT and gdb stops
> automatically.  The fix could be something like this:
> 

I can confirm, that un-daemonized the VM works, as well as…

> diff --git a/vl.c b/vl.c
> index e1ffd0a..af61835 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3759,7 +3759,9 @@ int main(int argc, char **argv, char **envp)
> 
>     loc_set_none();
> 
> +    qemu_mutex_unlock_iothread();
>     os_daemonize();
> +    qemu_mutex_lock_iothread();
> 

… after applying these 2 lines.


So thnx very much for the quick help/fix.

Oliver.

>     if (qemu_init_main_loop(&main_loop_err)) {
>         error_report_err(main_loop_err);
> 
> Paolo

Re: [Qemu-devel] [PATCH] vl: take iothread lock very early

[Christian Borntraeger]

Am 03.03.2015 um 18:32 schrieb Paolo Bonzini:
> 
> 
> On 03/03/2015 18:19, Oliver Francke wrote:
>>
>> #0  __GI_exit (status=1) at exit.c:104
>> #1  0x000055555575cf15 in os_daemonize () at os-posix.c:227
>> #2  0x0000555555773f2e in main (argc=50, argv=0x7fffffffe0d8,
>> envp=0x7fffffffe270) at vl.c:3770
>>
>> compiled with "--enable-debug" and in gdb "b exit". Not a coder, so if
>> this is _not_ sufficient, please give me some adivce ;)
> 
> Try removing -daemonize (hmm, that might actually fix the bug).  You
> should not need "b exit", because the error is a SIGABRT and gdb stops
> automatically.  The fix could be something like this:
> 
> diff --git a/vl.c b/vl.c
> index e1ffd0a..af61835 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3759,7 +3759,9 @@ int main(int argc, char **argv, char **envp)
> 
>      loc_set_none();
> 
> +    qemu_mutex_unlock_iothread();
>      os_daemonize();
> +    qemu_mutex_lock_iothread();
> 
>      if (qemu_init_main_loop(&main_loop_err)) {
>          error_report_err(main_loop_err);
> 
> Paolo
> 

This also fixes some strange issues with libvirt after qemu updates, so probably also a candidate for a quick merge.

Christian

Re: [Qemu-devel] [PATCH] vl: take iothread lock very early

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]

On 03/03/2015 10:32 AM, Paolo Bonzini wrote:
> 
> 
> On 03/03/2015 18:19, Oliver Francke wrote:
>>
>> #0  __GI_exit (status=1) at exit.c:104
>> #1  0x000055555575cf15 in os_daemonize () at os-posix.c:227
>> #2  0x0000555555773f2e in main (argc=50, argv=0x7fffffffe0d8,
>> envp=0x7fffffffe270) at vl.c:3770
>>
>> compiled with "--enable-debug" and in gdb "b exit". Not a coder, so if
>> this is _not_ sufficient, please give me some adivce ;)
> 
> Try removing -daemonize (hmm, that might actually fix the bug).

I've confirmed that libvirt required -daemonize, and that (temporarily)
removing -daemonize makes things work again.  So we definitely need this.

Tested-by: Eric Blake <eblake@redhat.com>

>  You
> should not need "b exit", because the error is a SIGABRT and gdb stops
> automatically.  The fix could be something like this:
> 
> diff --git a/vl.c b/vl.c
> index e1ffd0a..af61835 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3759,7 +3759,9 @@ int main(int argc, char **argv, char **envp)
> 
>      loc_set_none();
> 
> +    qemu_mutex_unlock_iothread();
>      os_daemonize();
> +    qemu_mutex_lock_iothread();
> 
>      if (qemu_init_main_loop(&main_loop_err)) {
>          error_report_err(main_loop_err);
> 
> Paolo
> 
> 
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH] vl: take iothread lock very early

[Christian Borntraeger]

Am 05.03.2015 um 01:40 schrieb Eric Blake:
> On 03/03/2015 10:32 AM, Paolo Bonzini wrote:
>>
>>
>> On 03/03/2015 18:19, Oliver Francke wrote:
>>>
>>> #0  __GI_exit (status=1) at exit.c:104
>>> #1  0x000055555575cf15 in os_daemonize () at os-posix.c:227
>>> #2  0x0000555555773f2e in main (argc=50, argv=0x7fffffffe0d8,
>>> envp=0x7fffffffe270) at vl.c:3770
>>>
>>> compiled with "--enable-debug" and in gdb "b exit". Not a coder, so if
>>> this is _not_ sufficient, please give me some adivce ;)
>>
>> Try removing -daemonize (hmm, that might actually fix the bug).
> 
> I've confirmed that libvirt required -daemonize, and that (temporarily)
> removing -daemonize makes things work again.  So we definitely need this.
> 
> Tested-by: Eric Blake <eblake@redhat.com>

Yes, came to the same conclusion.
Is there a chance to harden the error detection of libvirt somewhat?
I got things like 
"unsupported OS type hvm"
"unsupported configuration: QEMU 2.2.50 is too new for help parsing"

which made it quite hard to find out what was wrong.

Christian