[Qemu-devel] [PATCH] watchdog: fix deadlock with -watchdog-action pause

[Qemu-devel] [PATCH] watchdog: fix deadlock with -watchdog-action pause

[Paolo Bonzini]

qemu_clock_enable says:

/* Disabling the clock will wait for related timerlists to stop
 * executing qemu_run_timers.  Thus, this functions should not
 * be used from the callback of a timer that is based on @clock.
 * Doing so would cause a deadlock.
 */

and it indeed does: vm_stop uses qemu_clock_enable on QEMU_CLOCK_VIRTUAL
and watchdogs are based on QEMU_CLOCK_VIRTUAL, and we get a deadlock.

Use qemu_system_vmstop_request_prepare()/qemu_system_vmstop_request()
instead; the alternative could be a BH.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/watchdog/watchdog.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
index 9f607d4..aa25b6f 100644
--- a/hw/watchdog/watchdog.c
+++ b/hw/watchdog/watchdog.c
@@ -123,7 +123,8 @@ void watchdog_perform_action(void)
 
     case WDT_PAUSE:             /* same as 'stop' command in monitor */
         qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_PAUSE, &error_abort);
-        vm_stop(RUN_STATE_WATCHDOG);
+        qemu_system_vmstop_request_prepare();
+        qemu_system_vmstop_request(RUN_STATE_WATCHDOG);
         break;
 
     case WDT_DEBUG:
-- 
1.9.3

Re: [Qemu-devel] [PATCH] watchdog: fix deadlock with -watchdog-action pause

[Luiz Capitulino]

On Fri,  4 Jul 2014 15:48:50 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> qemu_clock_enable says:
> 
> /* Disabling the clock will wait for related timerlists to stop
>  * executing qemu_run_timers.  Thus, this functions should not
>  * be used from the callback of a timer that is based on @clock.
>  * Doing so would cause a deadlock.
>  */
> 
> and it indeed does: vm_stop uses qemu_clock_enable on QEMU_CLOCK_VIRTUAL
> and watchdogs are based on QEMU_CLOCK_VIRTUAL, and we get a deadlock.
> 
> Use qemu_system_vmstop_request_prepare()/qemu_system_vmstop_request()
> instead; the alternative could be a BH.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Tested-by: Luiz Capitulino <lcapitulino@redhat.com>

> ---
>  hw/watchdog/watchdog.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
> index 9f607d4..aa25b6f 100644
> --- a/hw/watchdog/watchdog.c
> +++ b/hw/watchdog/watchdog.c
> @@ -123,7 +123,8 @@ void watchdog_perform_action(void)
>  
>      case WDT_PAUSE:             /* same as 'stop' command in monitor */
>          qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_PAUSE, &error_abort);
> -        vm_stop(RUN_STATE_WATCHDOG);
> +        qemu_system_vmstop_request_prepare();
> +        qemu_system_vmstop_request(RUN_STATE_WATCHDOG);
>          break;
>  
>      case WDT_DEBUG: