[Qemu-devel] [PULL 0/3] QAPI patches for 2017-10-02

[Qemu-devel] [PULL 0/3] QAPI patches for 2017-10-02

[Markus Armbruster]

The following changes since commit ab161529261928ae7f3556e3220829c34b2686ec:

  Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20170927a' into staging (2017-09-27 22:44:51 +0100)

are available in the git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2017-10-02

for you to fetch changes up to f0df84c6c46cb632dac2d9fae5fdbe6001527c3b:

  watchdog: Allow setting action on the fly (2017-10-02 13:09:09 +0200)

----------------------------------------------------------------
QAPI patches for 2017-10-02

----------------------------------------------------------------
Michal Privoznik (3):
      qapi: Rename WatchdogExpirationAction enum
      watchdog.h: Drop local redefinition of actions enum
      watchdog: Allow setting action on the fly

 hw/watchdog/watchdog.c    | 65 ++++++++++++++++++++++++-----------------------
 hw/watchdog/wdt_diag288.c |  6 ++---
 include/sysemu/watchdog.h | 12 ++-------
 monitor.c                 |  4 +--
 qapi-schema.json          |  9 +++++++
 qapi/run-state.json       |  6 ++---
 6 files changed, 52 insertions(+), 50 deletions(-)

-- 
2.13.6

[Qemu-devel] [PULL 1/3] qapi: Rename WatchdogExpirationAction enum

[Markus Armbruster]

From: Michal Privoznik <mprivozn@redhat.com>

The new name is WatchdogAction which is shorter,

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Message-Id: <dbd61a0928821348486d0d6260be2bd3b02b6402.1504771369.git.mprivozn@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/watchdog/watchdog.c | 14 +++++++-------
 monitor.c              |  4 ++--
 qapi/run-state.json    |  6 +++---
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
index 0c5c9cde1c..358d79804d 100644
--- a/hw/watchdog/watchdog.c
+++ b/hw/watchdog/watchdog.c
@@ -109,17 +109,17 @@ void watchdog_perform_action(void)
 {
     switch (watchdog_action) {
     case WDT_RESET:             /* same as 'system_reset' in monitor */
-        qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_RESET, &error_abort);
+        qapi_event_send_watchdog(WATCHDOG_ACTION_RESET, &error_abort);
         qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
         break;
 
     case WDT_SHUTDOWN:          /* same as 'system_powerdown' in monitor */
-        qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_SHUTDOWN, &error_abort);
+        qapi_event_send_watchdog(WATCHDOG_ACTION_SHUTDOWN, &error_abort);
         qemu_system_powerdown_request();
         break;
 
     case WDT_POWEROFF:          /* same as 'quit' command in monitor */
-        qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_POWEROFF, &error_abort);
+        qapi_event_send_watchdog(WATCHDOG_ACTION_POWEROFF, &error_abort);
         exit(0);
 
     case WDT_PAUSE:             /* same as 'stop' command in monitor */
@@ -127,21 +127,21 @@ void watchdog_perform_action(void)
          * you would get a deadlock.  Bypass the problem.
          */
         qemu_system_vmstop_request_prepare();
-        qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_PAUSE, &error_abort);
+        qapi_event_send_watchdog(WATCHDOG_ACTION_PAUSE, &error_abort);
         qemu_system_vmstop_request(RUN_STATE_WATCHDOG);
         break;
 
     case WDT_DEBUG:
-        qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_DEBUG, &error_abort);
+        qapi_event_send_watchdog(WATCHDOG_ACTION_DEBUG, &error_abort);
         fprintf(stderr, "watchdog: timer fired\n");
         break;
 
     case WDT_NONE:
-        qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_NONE, &error_abort);
+        qapi_event_send_watchdog(WATCHDOG_ACTION_NONE, &error_abort);
         break;
 
     case WDT_NMI:
-        qapi_event_send_watchdog(WATCHDOG_EXPIRATION_ACTION_INJECT_NMI,
+        qapi_event_send_watchdog(WATCHDOG_ACTION_INJECT_NMI,
                                  &error_abort);
         nmi_monitor_handle(0, NULL);
         break;
diff --git a/monitor.c b/monitor.c
index f4856b9268..94fb197c0d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3541,8 +3541,8 @@ void watchdog_action_completion(ReadLineState *rs, int nb_args, const char *str)
         return;
     }
     readline_set_completion_index(rs, strlen(str));
-    for (i = 0; i < WATCHDOG_EXPIRATION_ACTION__MAX; i++) {
-        add_completion_option(rs, str, WatchdogExpirationAction_str(i));
+    for (i = 0; i < WATCHDOG_ACTION__MAX; i++) {
+        add_completion_option(rs, str, WatchdogAction_str(i));
     }
 }
 
diff --git a/qapi/run-state.json b/qapi/run-state.json
index d36ff49834..bca46a8785 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -253,10 +253,10 @@
 #
 ##
 { 'event': 'WATCHDOG',
-  'data': { 'action': 'WatchdogExpirationAction' } }
+  'data': { 'action': 'WatchdogAction' } }
 
 ##
-# @WatchdogExpirationAction:
+# @WatchdogAction:
 #
 # An enumeration of the actions taken when the watchdog device's timer is
 # expired
@@ -279,7 +279,7 @@
 #
 # Since: 2.1
 ##
-{ 'enum': 'WatchdogExpirationAction',
+{ 'enum': 'WatchdogAction',
   'data': [ 'reset', 'shutdown', 'poweroff', 'pause', 'debug', 'none',
             'inject-nmi' ] }
 
-- 
2.13.6

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

[Markus Armbruster]

From: Michal Privoznik <mprivozn@redhat.com>

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 <mprivozn@redhat.com>
Message-Id: <ce2790634e6a1b3b6cf90462399d17bad83f0290.1504771369.git.mprivozn@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 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
+    int action;
+    char *qapi_value;
+
+    qapi_value = g_ascii_strdown(p, -1);
+    action = qapi_enum_parse(&WatchdogAction_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, &error_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, &error_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, &error_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, &error_abort);
         fprintf(stderr, "watchdog: timer fired\n");
         break;
 
-    case WDT_NONE:
+    case WATCHDOG_ACTION_NONE:
         qapi_event_send_watchdog(WATCHDOG_ACTION_NONE, &error_abort);
         break;
 
-    case WDT_NMI:
+    case WATCHDOG_ACTION_INJECT_NMI:
         qapi_event_send_watchdog(WATCHDOG_ACTION_INJECT_NMI,
                                  &error_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_RESET        1      /* Hard reset. */
-#define WDT_SHUTDOWN     2      /* Shutdown. */
-#define WDT_POWEROFF     3      /* Quit. */
-#define WDT_PAUSE        4      /* Pause. */
-#define WDT_DEBUG        5      /* Prints a message and continues running. */
-#define WDT_NONE         6      /* Do nothing. */
-#define WDT_NMI          7      /* Inject nmi into the guest. */
+#include "qapi-types.h"
 
 struct WatchdogTimerModel {
     QLIST_ENTRY(WatchdogTimerModel) entry;
@@ -46,7 +38,7 @@ typedef struct WatchdogTimerModel WatchdogTimerModel;
 /* in hw/watchdog.c */
 int select_watchdog(const char *p);
 int select_watchdog_action(const char *action);
-int get_watchdog_action(void);
+WatchdogAction get_watchdog_action(void);
 void watchdog_add_model(WatchdogTimerModel *model);
 void watchdog_perform_action(void);
 
-- 
2.13.6

[Qemu-devel] [PULL 3/3] watchdog: Allow setting action on the fly

[Markus Armbruster]

From: Michal Privoznik <mprivozn@redhat.com>

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.

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

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Message-Id: <35d6ce6fe3d357122d73b8272bc8198134c74104.1504771369.git.mprivozn@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrange <berrange@redhat.com>
[Missing colon in doc comment fixed]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/watchdog/watchdog.c | 8 +++++++-
 qapi-schema.json       | 9 +++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
index 0d3eeed187..670114ecfe 100644
--- a/hw/watchdog/watchdog.c
+++ b/hw/watchdog/watchdog.c
@@ -29,6 +29,7 @@
 #include "qapi-event.h"
 #include "hw/nmi.h"
 #include "qemu/help_option.h"
+#include "qmp-commands.h"
 
 static WatchdogAction watchdog_action = WATCHDOG_ACTION_RESET;
 static QLIST_HEAD(watchdog_list, WatchdogTimerModel) watchdog_list;
@@ -85,7 +86,7 @@ int select_watchdog_action(const char *p)
     g_free(qapi_value);
     if (action < 0)
         return -1;
-    watchdog_action = action;
+    qmp_watchdog_set_action(action, &error_abort);
     return 0;
 }
 
@@ -142,3 +143,8 @@ void watchdog_perform_action(void)
         assert(0);
     }
 }
+
+void qmp_watchdog_set_action(WatchdogAction action, Error **errp)
+{
+    watchdog_action = action;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index a3ba1c9a1c..a9dd043f65 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3191,3 +3191,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': 'WatchdogAction'} }
-- 
2.13.6

Re: [Qemu-devel] [PULL 0/3] QAPI patches for 2017-10-02

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20171002111111.26932-1-armbru@redhat.com
Subject: [Qemu-devel] [PULL 0/3] QAPI patches for 2017-10-02

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
0915be50ad watchdog: Allow setting action on the fly
44e77bbf9d watchdog.h: Drop local redefinition of actions enum
8fcd8a0f85 qapi: Rename WatchdogExpirationAction enum

=== OUTPUT BEGIN ===
Checking PATCH 1/3: qapi: Rename WatchdogExpirationAction enum...
Checking PATCH 2/3: watchdog.h: Drop local redefinition of actions enum...
ERROR: braces {} are necessary for all arms of this statement
#57: FILE: hw/watchdog/watchdog.c:86:
+    if (action < 0)
[...]

total: 1 errors, 0 warnings, 130 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/3: watchdog: Allow setting action on the fly...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

Re: [Qemu-devel] [PULL 0/3] QAPI patches for 2017-10-02

[Eric Blake]

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

On 10/02/2017 06:42 AM, no-reply@patchew.org wrote:
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 

> Checking PATCH 2/3: watchdog.h: Drop local redefinition of actions enum...
> ERROR: braces {} are necessary for all arms of this statement
> #57: FILE: hw/watchdog/watchdog.c:86:
> +    if (action < 0)
> [...]

Pre-existing problem, but would indeed be easy to fix up as part of this
series.  I don't know if that means Markus should respin the pull request?

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


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

Re: [Qemu-devel] [PULL 0/3] QAPI patches for 2017-10-02

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 10/02/2017 06:42 AM, no-reply@patchew.org wrote:
>> Hi,
>> 
>> This series seems to have some coding style problems. See output below for
>> more information:
>> 
>
>> Checking PATCH 2/3: watchdog.h: Drop local redefinition of actions enum...
>> ERROR: braces {} are necessary for all arms of this statement
>> #57: FILE: hw/watchdog/watchdog.c:86:
>> +    if (action < 0)
>> [...]
>
> Pre-existing problem, but would indeed be easy to fix up as part of this
> series.  I don't know if that means Markus should respin the pull request?

I would've touched it up in my tree had I seen it in time, but I doubt
it's worth the bother now.

Re: [Qemu-devel] [PULL 0/3] QAPI patches for 2017-10-02

[Peter Maydell]

On 2 October 2017 at 12:11, Markus Armbruster <armbru@redhat.com> wrote:
> The following changes since commit ab161529261928ae7f3556e3220829c34b2686ec:
>
>   Merge remote-tracking branch 'remotes/dgilbert/tags/pull-migration-20170927a' into staging (2017-09-27 22:44:51 +0100)
>
> are available in the git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2017-10-02
>
> for you to fetch changes up to f0df84c6c46cb632dac2d9fae5fdbe6001527c3b:
>
>   watchdog: Allow setting action on the fly (2017-10-02 13:09:09 +0200)
>
> ----------------------------------------------------------------
> QAPI patches for 2017-10-02
>
> ----------------------------------------------------------------
> Michal Privoznik (3):
>       qapi: Rename WatchdogExpirationAction enum
>       watchdog.h: Drop local redefinition of actions enum
>       watchdog: Allow setting action on the fly
>
Applied, thanks.

-- PMM