Re: [Qemu-devel] [PATCH v5 2/3] watchdog.h: Drop local redefinition of actions enum

[Daniel P. Berrange]

On Thu, Sep 07, 2017 at 10:05:25AM +0200, Michal Privoznik wrote:
> We already have enum that enumerates all the actions that a
> watchdog can take when hitting its timeout: WatchdogAction.
> Use that instead of inventing our own.
> 
> Signed-off-by: Michal Privoznik 
> ---
>  hw/watchdog/watchdog.c| 45 -
>  hw/watchdog/wdt_diag288.c |  6 +++---
>  include/sysemu/watchdog.h | 12 ++--
>  3 files changed, 25 insertions(+), 38 deletions(-)

Reviewed-by: Daniel P. Berrange 


Regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[Qemu-devel] [PATCH v5 2/3] watchdog.h: Drop local redefinition of actions enum

[Michal Privoznik]

We already have enum that enumerates all the actions that a
watchdog can take when hitting its timeout: WatchdogAction.
Use that instead of inventing our own.

Signed-off-by: Michal Privoznik 
---
 hw/watchdog/watchdog.c| 45 -
 hw/watchdog/wdt_diag288.c |  6 +++---
 include/sysemu/watchdog.h | 12 ++--
 3 files changed, 25 insertions(+), 38 deletions(-)

diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
index 358d79804d..0d3eeed187 100644
--- a/hw/watchdog/watchdog.c
+++ b/hw/watchdog/watchdog.c
@@ -30,7 +30,7 @@
 #include "hw/nmi.h"
 #include "qemu/help_option.h"
 
-static int watchdog_action = WDT_RESET;
+static WatchdogAction watchdog_action = WATCHDOG_ACTION_RESET;
 static QLIST_HEAD(watchdog_list, WatchdogTimerModel) watchdog_list;
 
 void watchdog_add_model(WatchdogTimerModel *model)
@@ -77,27 +77,19 @@ 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;
+char *qapi_value;
 
+qapi_value = g_ascii_strdown(p, -1);
+action = qapi_enum_parse(_lookup, qapi_value, -1, NULL);
+g_free(qapi_value);
+if (action < 0)
+return -1;
+watchdog_action = action;
 return 0;
 }
 
-int get_watchdog_action(void)
+WatchdogAction get_watchdog_action(void)
 {
 return watchdog_action;
 }
@@ -108,21 +100,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_ACTION_RESET: /* same as 'system_reset' in monitor */
 qapi_event_send_watchdog(WATCHDOG_ACTION_RESET, _abort);
 qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
 break;
 
-case WDT_SHUTDOWN:  /* same as 'system_powerdown' in monitor */
+case WATCHDOG_ACTION_SHUTDOWN:  /* same as 'system_powerdown' in monitor */
 qapi_event_send_watchdog(WATCHDOG_ACTION_SHUTDOWN, _abort);
 qemu_system_powerdown_request();
 break;
 
-case WDT_POWEROFF:  /* same as 'quit' command in monitor */
+case WATCHDOG_ACTION_POWEROFF:  /* same as 'quit' command in monitor */
 qapi_event_send_watchdog(WATCHDOG_ACTION_POWEROFF, _abort);
 exit(0);
 
-case WDT_PAUSE: /* same as 'stop' command in monitor */
+case WATCHDOG_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 +123,22 @@ void watchdog_perform_action(void)
 qemu_system_vmstop_request(RUN_STATE_WATCHDOG);
 break;
 
-case WDT_DEBUG:
+case WATCHDOG_ACTION_DEBUG:
 qapi_event_send_watchdog(WATCHDOG_ACTION_DEBUG, _abort);
 fprintf(stderr, "watchdog: timer fired\n");
 break;
 
-case WDT_NONE:
+case WATCHDOG_ACTION_NONE:
 qapi_event_send_watchdog(WATCHDOG_ACTION_NONE, _abort);
 break;
 
-case WDT_NMI:
+case WATCHDOG_ACTION_INJECT_NMI:
 qapi_event_send_watchdog(WATCHDOG_ACTION_INJECT_NMI,
  _abort);
 nmi_monitor_handle(0, NULL);
 break;
+
+default:
+assert(0);
 }
 }
diff --git a/hw/watchdog/wdt_diag288.c b/hw/watchdog/wdt_diag288.c
index 47f289216a..1475743527 100644
--- a/hw/watchdog/wdt_diag288.c
+++ b/hw/watchdog/wdt_diag288.c
@@ -57,9 +57,9 @@ static void diag288_timer_expired(void *dev)
  * the BQL; reset before triggering the action to avoid races with
  * diag288 instructions. */
 switch (get_watchdog_action()) {
-case WDT_DEBUG:
-case WDT_NONE:
-case WDT_PAUSE:
+case WATCHDOG_ACTION_DEBUG:
+case WATCHDOG_ACTION_NONE:
+case WATCHDOG_ACTION_PAUSE:
 break;
 default:
 wdt_diag288_reset(dev);
diff --git a/include/sysemu/watchdog.h b/include/sysemu/watchdog.h
index 72a4da07a6..677ace3945 100644
--- a/include/sysemu/watchdog.h
+++ b/include/sysemu/watchdog.h
@@ -23,15 +23,7 @@
 #define QEMU_WATCHDOG_H
 
 #include "qemu/queue.h"
-
-/* Possible values for action parameter. */
-#define WDT_RESET1  /* Hard reset. */
-#define WDT_SHUTDOWN 2  /* Shutdown. */
-#define WDT_POWEROFF 3  /* Quit. */
-#define WDT_PAUSE4  /* Pause. */