Patchwork qmp problems with --enable-kvm

login
register
mail settings
Submitter Jan Kiszka
Date Nov. 22, 2012, 5:09 p.m.
Message ID <50AE5C58.8030204@siemens.com>
Download mbox | patch
Permalink /patch/201100/
State New
Headers show

Comments

Jan Kiszka - Nov. 22, 2012, 5:09 p.m.
On 2012-11-22 18:07, Paolo Bonzini wrote:
> Il 22/11/2012 17:58, Luiz Capitulino ha scritto:
>>>>> It seems like mon->mc->command_mode is set wrong, looking at
>>>>> qmp_cmd_mode and its callers.  Luiz may have more ideas.
>>>>
>>>> Checking. What I've just found is that qmp_capabilites will fail if the
>>>> VM is stopped (!?).
>>>
>>> It's a regression somewhere. I doubt it's qmp, but could be.
>>>
>>> Here are the symptoms, Doc:
>>>
>>>  1. Start qemu and stop it right away. Connect to the QMP socket and you
>>>     won't get qmp's greeting
>>>
>>>  2. Start qemu, connect to the QMP socket and run the qmp_capabilities
>>>     command. Then stop qemu and disconnect from the qmp socket and connect
>>>     again: you'll see you're still in the previous session
>>>
>>> I do not get this with qemu 1.0.
>>>
>>> Dietmar got this because the suspend command automatically stops the VM
>>> after migration...
>>>
>>> Bisecting...
>>
>> Didn't try to understand what's wrong with it, but bisect brings:
>>
>> commit ac4119c023c72b15f54238af43e4a178fcf41494
>> Author: Jan Kiszka <jan.kiszka@siemens.com>
>> Date:   Fri Oct 12 09:52:49 2012 +0200
>>
>>     chardev: Use timer instead of bottom-half to postpone open event
>>     
>>     As the block layer may decide to flush bottom-halfs while the machine is
>>     still initializing (e.g. to read geometry data from the disk), our
>>     postponed open event may be processed before the last frontend
>>     registered with a muxed chardev.
>>     
>>     Until the semantics of BHs have been clarified, use an expired timer to
>>     achieve the same effect (suggested by Paolo Bonzini). This requires to
>>     perform the alarm timer initialization earlier as otherwise timer
>>     subsystem can be used before being ready.
>>     
>>     Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Ouch, in retrospect it actually makes sense since this patch uses a
> vm_clock timer.  Elementary, Watson... :)
> 
> I don't think there is a fix, short of reverting this commit.

We have more timers than the one based on vm_clock. Does this help?

Jan
Dietmar Maurer - Nov. 23, 2012, 6:29 a.m.
> We have more timers than the one based on vm_clock. Does this help?

Yes, that patch fixes the problem for me.

> 
> diff --git a/qemu-char.c b/qemu-char.c
> index 88f4025..242b799 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -134,9 +134,9 @@ static void qemu_chr_fire_open_event(void
> *opaque)  void qemu_chr_generic_open(CharDriverState *s)  {
>      if (s->open_timer == NULL) {
> -        s->open_timer = qemu_new_timer_ms(vm_clock,
> +        s->open_timer = qemu_new_timer_ms(rt_clock,
>                                            qemu_chr_fire_open_event, s);
> -        qemu_mod_timer(s->open_timer, qemu_get_clock_ms(vm_clock) - 1);
> +        qemu_mod_timer(s->open_timer, qemu_get_clock_ms(rt_clock) - 1);
>      }
>  }

Patch

diff --git a/qemu-char.c b/qemu-char.c
index 88f4025..242b799 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -134,9 +134,9 @@  static void qemu_chr_fire_open_event(void *opaque)
 void qemu_chr_generic_open(CharDriverState *s)
 {
     if (s->open_timer == NULL) {
-        s->open_timer = qemu_new_timer_ms(vm_clock,
+        s->open_timer = qemu_new_timer_ms(rt_clock,
                                           qemu_chr_fire_open_event, s);
-        qemu_mod_timer(s->open_timer, qemu_get_clock_ms(vm_clock) - 1);
+        qemu_mod_timer(s->open_timer, qemu_get_clock_ms(rt_clock) - 1);
     }
 }