Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the fly

2017-09-06 Thread Markus Armbruster
Michal Privoznik  writes:

> On 09/05/2017 10:36 PM, Eric Blake wrote:
>> On 09/05/2017 08:22 AM, Michal Privoznik wrote:
>>> Currently, the only time that users can set watchdog action is at
>>> the start as all we expose is this -watchdog-action command line
>>> argument. This is suboptimal when users want to plug the device
>>> later via monitor. Alternatively, they might want to change the
>>> action for already existing device on the fly.
>>>
>>> At the same time, drop local redefinition of the actions eum in
>> 
>> s/eum/enum/
>> 
>>> watchdog.h in favour of the ones defined in schema.
>>>
>>> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169
>>>
>>> Signed-off-by: Michal Privoznik 
[...]
>>> +++ b/qapi-schema.json
>>> @@ -6539,3 +6539,12 @@
>>>  # Since 2.9
>>>  ##
>>>  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
>>> +
>>> +##
>>> +# @watchdog-set-action:
>>> +#
>>> +# Set watchdog action
>>> +#
>>> +# Since 2.11
>>> +##
>>> +{ 'command': 'watchdog-set-action', 'data' : {'action': 
>>> 'WatchdogExpirationAction'} }
>> 
>> Can a machine have more than one watchdog device, in which case you'd
>> want a device name to select which watchdog gets which action?  But the
>> command-line version that you are replacing seems to be global, so I
>> guess you're okay with this interface.
>> 
>
> Yeah, I don't think that a guest can have more than one watchdog:
>
> /qemu.git $ ./x86_64-softmmu/qemu-system-x86_64 \
> -watchdog ib700 -watchdog i6300esb
> qemu-system-x86_64: -watchdog i6300esb: only one watchdog option may be
> given

-watchdog is a thin wrapper around -device, and all it adds is this
error.  It's quite redundant.  I recommend using -device instead.

"-device i6300esb -device i6300esb" works.  It's just a PCI device after
all.

> Also, would it make sense? I mean, what would be the benefit of having
> two or more watchdogs?

None.  All it would accomplish is complicating things.



Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the fly

2017-09-06 Thread Markus Armbruster
Michal Privoznik  writes:

> On 09/06/2017 07:59 AM, Markus Armbruster wrote:
>> Eric Blake  writes:
>> 
>>> On 09/05/2017 08:22 AM, Michal Privoznik wrote:
 Currently, the only time that users can set watchdog action is at
 the start as all we expose is this -watchdog-action command line
 argument. This is suboptimal when users want to plug the device
 later via monitor. Alternatively, they might want to change the
 action for already existing device on the fly.

 At the same time, drop local redefinition of the actions eum in
>>>
>>> s/eum/enum/
>>>
 watchdog.h in favour of the ones defined in schema.

 Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169

 Signed-off-by: Michal Privoznik 
 ---
>>>
 @@ -77,27 +80,17 @@ int select_watchdog(const char *p)
  
  int select_watchdog_action(const char *p)
  {
>>>
 +int action;
  
 +action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,
 + WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);
>>>
>>> Semantic conflict; you need to rebase now that the qapi enum lookup code
>>> has landed (see commit ebf677c8 and parents)
>>>
 +
 +case WATCHDOG_EXPIRATION_ACTION__MAX:
 +/* keep gcc happy */
 +break;
>>>
>>> Could use g_assert_not_reached() here instead.
>> 
>> Catches one out of approximately 2^64 invalid values.  To catches all,
>> and in fewer words:
>> 
>> default:
>> assert(0);
>
> I'm not a fan of this, but I'm not a qemu developer so I don't know what
> your coding style is (if any for this case). However, since this switch
> is over an enum, compiler will scream if a new value is introduced to
> the enum and not handled in the switch. IMO it's useful because when I'm
> adding new value I can immediately see what other places I need to consider.

Two separate purposes: catch invalid state, catch forgotten code update.
I can't see how to serve both.  Your patch picks the latter.

> Also, yeah, I am going to rename the enum in v3.

Thanks!



Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the fly

2017-09-05 Thread Michal Privoznik
On 09/06/2017 07:59 AM, Markus Armbruster wrote:
> Eric Blake  writes:
> 
>> On 09/05/2017 08:22 AM, Michal Privoznik wrote:
>>> Currently, the only time that users can set watchdog action is at
>>> the start as all we expose is this -watchdog-action command line
>>> argument. This is suboptimal when users want to plug the device
>>> later via monitor. Alternatively, they might want to change the
>>> action for already existing device on the fly.
>>>
>>> At the same time, drop local redefinition of the actions eum in
>>
>> s/eum/enum/
>>
>>> watchdog.h in favour of the ones defined in schema.
>>>
>>> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169
>>>
>>> Signed-off-by: Michal Privoznik 
>>> ---
>>
>>> @@ -77,27 +80,17 @@ int select_watchdog(const char *p)
>>>  
>>>  int select_watchdog_action(const char *p)
>>>  {
>>
>>> +int action;
>>>  
>>> +action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,
>>> + WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);
>>
>> Semantic conflict; you need to rebase now that the qapi enum lookup code
>> has landed (see commit ebf677c8 and parents)
>>
>>> +
>>> +case WATCHDOG_EXPIRATION_ACTION__MAX:
>>> +/* keep gcc happy */
>>> +break;
>>
>> Could use g_assert_not_reached() here instead.
> 
> Catches one out of approximately 2^64 invalid values.  To catches all,
> and in fewer words:
> 
> default:
> assert(0);

I'm not a fan of this, but I'm not a qemu developer so I don't know what
your coding style is (if any for this case). However, since this switch
is over an enum, compiler will scream if a new value is introduced to
the enum and not handled in the switch. IMO it's useful because when I'm
adding new value I can immediately see what other places I need to consider.

Also, yeah, I am going to rename the enum in v3.

Michal



Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the fly

2017-09-05 Thread Markus Armbruster
Michal Privoznik  writes:

> Currently, the only time that users can set watchdog action is at
> the start as all we expose is this -watchdog-action command line
> argument. This is suboptimal when users want to plug the device
> later via monitor. Alternatively, they might want to change the
> action for already existing device on the fly.
>
> At the same time, drop local redefinition of the actions eum in
> watchdog.h in favour of the ones defined in schema.
>
> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169
>
> Signed-off-by: Michal Privoznik 
> ---
>
> diff to v1:
> - instead of allowing 'action' to be arbitrary string, use enum for it and 
> thus
>   drop local redefinition of the enum.
>
>  hw/watchdog/watchdog.c| 52 
> ---
>  hw/watchdog/wdt_diag288.c |  6 +++---
>  include/sysemu/watchdog.h | 12 ++-
>  qapi-schema.json  |  9 
>  4 files changed, 41 insertions(+), 38 deletions(-)
>
> diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
> index 0c5c9cde1c..a3e9d7b59b 100644
> --- a/hw/watchdog/watchdog.c
> +++ b/hw/watchdog/watchdog.c
> @@ -29,8 +29,11 @@
>  #include "qapi-event.h"
>  #include "hw/nmi.h"
>  #include "qemu/help_option.h"
> +#include "qmp-commands.h"
> +#include "qapi/qmp/qerror.h"
> +#include "qapi/util.h"
>  
> -static int watchdog_action = WDT_RESET;
> +static WatchdogExpirationAction watchdog_action = 
> WATCHDOG_EXPIRATION_ACTION_RESET;
>  static QLIST_HEAD(watchdog_list, WatchdogTimerModel) watchdog_list;
>  
>  void watchdog_add_model(WatchdogTimerModel *model)
> @@ -77,27 +80,17 @@ int select_watchdog(const char *p)
>  
>  int select_watchdog_action(const char *p)
>  {
> -if (strcasecmp(p, "reset") == 0)
> -watchdog_action = WDT_RESET;
> -else if (strcasecmp(p, "shutdown") == 0)
> -watchdog_action = WDT_SHUTDOWN;
> -else if (strcasecmp(p, "poweroff") == 0)
> -watchdog_action = WDT_POWEROFF;
> -else if (strcasecmp(p, "pause") == 0)
> -watchdog_action = WDT_PAUSE;
> -else if (strcasecmp(p, "debug") == 0)
> -watchdog_action = WDT_DEBUG;
> -else if (strcasecmp(p, "none") == 0)
> -watchdog_action = WDT_NONE;
> -else if (strcasecmp(p, "inject-nmi") == 0)
> -watchdog_action = WDT_NMI;
> -else
> -return -1;
> +int action;
>  
> +action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,
> + WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);
> +if (action < 0)
> +return -1;
> +qmp_watchdog_set_action(action, NULL);

&error_abort

>  return 0;
>  }
>  
> -int get_watchdog_action(void)
> +WatchdogExpirationAction get_watchdog_action(void)
>  {
>  return watchdog_action;
>  }
> @@ -108,21 +101,21 @@ int get_watchdog_action(void)
>  void watchdog_perform_action(void)
>  {
>  switch (watchdog_action) {
> -case WDT_RESET: /* same as 'system_reset' in monitor */
> +case WATCHDOG_EXPIRATION_ACTION_RESET:  /* same as 'system_reset' in 
> monitor */
>  qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_RESET, 
> &error_abort);
>  qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>  break;
>  
> -case WDT_SHUTDOWN:  /* same as 'system_powerdown' in monitor */
> +case WATCHDOG_EXPIRATION_ACTION_SHUTDOWN:   /* same as 
> 'system_powerdown' in monitor */
>  qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_SHUTDOWN, 
> &error_abort);
>  qemu_system_powerdown_request();
>  break;
>  
> -case WDT_POWEROFF:  /* same as 'quit' command in monitor */
> +case WATCHDOG_EXPIRATION_ACTION_POWEROFF:   /* same as 'quit' command in 
> monitor */
>  qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_POWEROFF, 
> &error_abort);
>  exit(0);
>  
> -case WDT_PAUSE: /* same as 'stop' command in monitor */
> +case WATCHDOG_EXPIRATION_ACTION_PAUSE:  /* same as 'stop' command in 
> monitor */
>  /* In a timer callback, when vm_stop calls qemu_clock_enable
>   * you would get a deadlock.  Bypass the problem.
>   */
> @@ -131,19 +124,28 @@ void watchdog_perform_action(void)
>  qemu_system_vmstop_request(RUN_STATE_WATCHDOG);
>  break;
>  
> -case WDT_DEBUG:
> +case WATCHDOG_EXPIRATION_ACTION_DEBUG:
>  qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_DEBUG, 
> &error_abort);
>  fprintf(stderr, "watchdog: timer fired\n");
>  break;
>  
> -case WDT_NONE:
> +case WATCHDOG_EXPIRATION_ACTION_NONE:
>  qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_NONE, 
> &error_abort);
>  break;
>  
> -case WDT_NMI:
> +case WATCHDOG_EXPIRATION_ACTION_INJECT_NMI:
>  qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_INJECT_NMI,
>   &error_abort);
>  nmi_monitor_handle(0, NULL);
>  br

Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the fly

2017-09-05 Thread Michal Privoznik
On 09/05/2017 10:36 PM, Eric Blake wrote:
> On 09/05/2017 08:22 AM, Michal Privoznik wrote:
>> Currently, the only time that users can set watchdog action is at
>> the start as all we expose is this -watchdog-action command line
>> argument. This is suboptimal when users want to plug the device
>> later via monitor. Alternatively, they might want to change the
>> action for already existing device on the fly.
>>
>> At the same time, drop local redefinition of the actions eum in
> 
> s/eum/enum/
> 
>> watchdog.h in favour of the ones defined in schema.
>>
>> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169
>>
>> Signed-off-by: Michal Privoznik 
>> ---
> 
>> @@ -77,27 +80,17 @@ int select_watchdog(const char *p)
>>  
>>  int select_watchdog_action(const char *p)
>>  {
> 
>> +int action;
>>  
>> +action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,
>> + WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);
> 
> Semantic conflict; you need to rebase now that the qapi enum lookup code
> has landed (see commit ebf677c8 and parents)


D'oh. Okay.

> 
>> +
>> +case WATCHDOG_EXPIRATION_ACTION__MAX:
>> +/* keep gcc happy */
>> +break;
> 
> Could use g_assert_not_reached() here instead.
> 
>> +++ b/qapi-schema.json
>> @@ -6539,3 +6539,12 @@
>>  # Since 2.9
>>  ##
>>  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
>> +
>> +##
>> +# @watchdog-set-action:
>> +#
>> +# Set watchdog action
>> +#
>> +# Since 2.11
>> +##
>> +{ 'command': 'watchdog-set-action', 'data' : {'action': 
>> 'WatchdogExpirationAction'} }
> 
> Can a machine have more than one watchdog device, in which case you'd
> want a device name to select which watchdog gets which action?  But the
> command-line version that you are replacing seems to be global, so I
> guess you're okay with this interface.
> 

Yeah, I don't think that a guest can have more than one watchdog:

/qemu.git $ ./x86_64-softmmu/qemu-system-x86_64 \
-watchdog ib700 -watchdog i6300esb
qemu-system-x86_64: -watchdog i6300esb: only one watchdog option may be
given

Also, would it make sense? I mean, what would be the benefit of having
two or more watchdogs?

Michal



Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the fly

2017-09-05 Thread Markus Armbruster
Eric Blake  writes:

> On 09/05/2017 08:22 AM, Michal Privoznik wrote:
>> Currently, the only time that users can set watchdog action is at
>> the start as all we expose is this -watchdog-action command line
>> argument. This is suboptimal when users want to plug the device
>> later via monitor. Alternatively, they might want to change the
>> action for already existing device on the fly.
>> 
>> At the same time, drop local redefinition of the actions eum in
>
> s/eum/enum/
>
>> watchdog.h in favour of the ones defined in schema.
>> 
>> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169
>> 
>> Signed-off-by: Michal Privoznik 
>> ---
>
>> @@ -77,27 +80,17 @@ int select_watchdog(const char *p)
>>  
>>  int select_watchdog_action(const char *p)
>>  {
>
>> +int action;
>>  
>> +action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,
>> + WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);
>
> Semantic conflict; you need to rebase now that the qapi enum lookup code
> has landed (see commit ebf677c8 and parents)
>
>> +
>> +case WATCHDOG_EXPIRATION_ACTION__MAX:
>> +/* keep gcc happy */
>> +break;
>
> Could use g_assert_not_reached() here instead.

Catches one out of approximately 2^64 invalid values.  To catches all,
and in fewer words:

default:
assert(0);

>> +++ b/qapi-schema.json
>> @@ -6539,3 +6539,12 @@
>>  # Since 2.9
>>  ##
>>  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
>> +
>> +##
>> +# @watchdog-set-action:
>> +#
>> +# Set watchdog action
>> +#
>> +# Since 2.11
>> +##
>> +{ 'command': 'watchdog-set-action', 'data' : {'action': 
>> 'WatchdogExpirationAction'} }
>
> Can a machine have more than one watchdog device, in which case you'd
> want a device name to select which watchdog gets which action?  But the
> command-line version that you are replacing seems to be global, so I
> guess you're okay with this interface.

For better or worse, all watchdogs share the action, see
hw/watchdog/watchdog.c.

Watchdogs predate qdev.  Commit 9dd986c lets you configure up to one,
with two command line options: --watchdog and --watchdog-action.  In a
way, --watchdog configures the frontend, and --watchdog-action the
backend (having watchdog.c in hw/watchdog/ contradicts this view,
though).

Since the frontends got qdevified (commit 09aaa16), you can configure
any number of watchdog frontends, but still only one backend shared by
all.  Unsharing the backend wasn't worth the trouble, because having
multiple watchdogs would be weird, and multiple watchdogs with different
actions would be weirder.  Trouble would include maintaining command
line backward compatibility, as always.

Of course, if we *want* to unshare the backend, we should do it *before*
we add QMP backward compatibility to the trouble.



Re: [Qemu-devel] [PATCH v2] watchdog: Allow setting action on the fly

2017-09-05 Thread Eric Blake
On 09/05/2017 08:22 AM, Michal Privoznik wrote:
> Currently, the only time that users can set watchdog action is at
> the start as all we expose is this -watchdog-action command line
> argument. This is suboptimal when users want to plug the device
> later via monitor. Alternatively, they might want to change the
> action for already existing device on the fly.
> 
> At the same time, drop local redefinition of the actions eum in

s/eum/enum/

> watchdog.h in favour of the ones defined in schema.
> 
> Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169
> 
> Signed-off-by: Michal Privoznik 
> ---

> @@ -77,27 +80,17 @@ int select_watchdog(const char *p)
>  
>  int select_watchdog_action(const char *p)
>  {

> +int action;
>  
> +action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,
> + WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);

Semantic conflict; you need to rebase now that the qapi enum lookup code
has landed (see commit ebf677c8 and parents)

> +
> +case WATCHDOG_EXPIRATION_ACTION__MAX:
> +/* keep gcc happy */
> +break;

Could use g_assert_not_reached() here instead.

> +++ b/qapi-schema.json
> @@ -6539,3 +6539,12 @@
>  # Since 2.9
>  ##
>  { 'command': 'query-vm-generation-id', 'returns': 'GuidInfo' }
> +
> +##
> +# @watchdog-set-action:
> +#
> +# Set watchdog action
> +#
> +# Since 2.11
> +##
> +{ 'command': 'watchdog-set-action', 'data' : {'action': 
> 'WatchdogExpirationAction'} }

Can a machine have more than one watchdog device, in which case you'd
want a device name to select which watchdog gets which action?  But the
command-line version that you are replacing seems to be global, so I
guess you're okay with this interface.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH v2] watchdog: Allow setting action on the fly

2017-09-05 Thread Michal Privoznik
Currently, the only time that users can set watchdog action is at
the start as all we expose is this -watchdog-action command line
argument. This is suboptimal when users want to plug the device
later via monitor. Alternatively, they might want to change the
action for already existing device on the fly.

At the same time, drop local redefinition of the actions eum in
watchdog.h in favour of the ones defined in schema.

Inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=1447169

Signed-off-by: Michal Privoznik 
---

diff to v1:
- instead of allowing 'action' to be arbitrary string, use enum for it and thus
  drop local redefinition of the enum.

 hw/watchdog/watchdog.c| 52 ---
 hw/watchdog/wdt_diag288.c |  6 +++---
 include/sysemu/watchdog.h | 12 ++-
 qapi-schema.json  |  9 
 4 files changed, 41 insertions(+), 38 deletions(-)

diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
index 0c5c9cde1c..a3e9d7b59b 100644
--- a/hw/watchdog/watchdog.c
+++ b/hw/watchdog/watchdog.c
@@ -29,8 +29,11 @@
 #include "qapi-event.h"
 #include "hw/nmi.h"
 #include "qemu/help_option.h"
+#include "qmp-commands.h"
+#include "qapi/qmp/qerror.h"
+#include "qapi/util.h"
 
-static int watchdog_action = WDT_RESET;
+static WatchdogExpirationAction watchdog_action = 
WATCHDOG_EXPIRATION_ACTION_RESET;
 static QLIST_HEAD(watchdog_list, WatchdogTimerModel) watchdog_list;
 
 void watchdog_add_model(WatchdogTimerModel *model)
@@ -77,27 +80,17 @@ int select_watchdog(const char *p)
 
 int select_watchdog_action(const char *p)
 {
-if (strcasecmp(p, "reset") == 0)
-watchdog_action = WDT_RESET;
-else if (strcasecmp(p, "shutdown") == 0)
-watchdog_action = WDT_SHUTDOWN;
-else if (strcasecmp(p, "poweroff") == 0)
-watchdog_action = WDT_POWEROFF;
-else if (strcasecmp(p, "pause") == 0)
-watchdog_action = WDT_PAUSE;
-else if (strcasecmp(p, "debug") == 0)
-watchdog_action = WDT_DEBUG;
-else if (strcasecmp(p, "none") == 0)
-watchdog_action = WDT_NONE;
-else if (strcasecmp(p, "inject-nmi") == 0)
-watchdog_action = WDT_NMI;
-else
-return -1;
+int action;
 
+action = qapi_enum_parse(WatchdogExpirationAction_lookup, p,
+ WATCHDOG_EXPIRATION_ACTION__MAX, -1, NULL);
+if (action < 0)
+return -1;
+qmp_watchdog_set_action(action, NULL);
 return 0;
 }
 
-int get_watchdog_action(void)
+WatchdogExpirationAction get_watchdog_action(void)
 {
 return watchdog_action;
 }
@@ -108,21 +101,21 @@ int get_watchdog_action(void)
 void watchdog_perform_action(void)
 {
 switch (watchdog_action) {
-case WDT_RESET: /* same as 'system_reset' in monitor */
+case WATCHDOG_EXPIRATION_ACTION_RESET:  /* same as 'system_reset' in 
monitor */
 qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_RESET, 
&error_abort);
 qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
 break;
 
-case WDT_SHUTDOWN:  /* same as 'system_powerdown' in monitor */
+case WATCHDOG_EXPIRATION_ACTION_SHUTDOWN:   /* same as 'system_powerdown' 
in monitor */
 qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_SHUTDOWN, 
&error_abort);
 qemu_system_powerdown_request();
 break;
 
-case WDT_POWEROFF:  /* same as 'quit' command in monitor */
+case WATCHDOG_EXPIRATION_ACTION_POWEROFF:   /* same as 'quit' command in 
monitor */
 qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_POWEROFF, 
&error_abort);
 exit(0);
 
-case WDT_PAUSE: /* same as 'stop' command in monitor */
+case WATCHDOG_EXPIRATION_ACTION_PAUSE:  /* same as 'stop' command in 
monitor */
 /* In a timer callback, when vm_stop calls qemu_clock_enable
  * you would get a deadlock.  Bypass the problem.
  */
@@ -131,19 +124,28 @@ void watchdog_perform_action(void)
 qemu_system_vmstop_request(RUN_STATE_WATCHDOG);
 break;
 
-case WDT_DEBUG:
+case WATCHDOG_EXPIRATION_ACTION_DEBUG:
 qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_DEBUG, 
&error_abort);
 fprintf(stderr, "watchdog: timer fired\n");
 break;
 
-case WDT_NONE:
+case WATCHDOG_EXPIRATION_ACTION_NONE:
 qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_NONE, 
&error_abort);
 break;
 
-case WDT_NMI:
+case WATCHDOG_EXPIRATION_ACTION_INJECT_NMI:
 qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_INJECT_NMI,
  &error_abort);
 nmi_monitor_handle(0, NULL);
 break;
+
+case WATCHDOG_EXPIRATION_ACTION__MAX:
+/* keep gcc happy */
+break;
 }
 }
+
+void qmp_watchdog_set_action(WatchdogExpirationAction action, Error **errp)
+{
+watchdog_action = action;
+}
diff --git a/hw/watchdog/wdt_diag288.c b/hw/watchdog/wdt_diag288.c
index 47f289216a..4