[PATCH 0/3] Deprecate -watchdog, cleanup -watchdog-action

[PATCH 0/3] Deprecate -watchdog, cleanup -watchdog-action

[Paolo Bonzini]

-watchdog is a simple wrapper around -device, but it includes quite
a bit of code to register and list available watchdog devices.
Add a category for watchdogs so that they are listed as such in
'-device help', and deprecate it.

For -watchdog-action, make it 100% a shortcut for "-action watchdog",
instead of calling into watchdog.c from the command line parser.
This is a small improvement because "-watchdog-action" and "-action
watchdog" now override each other based on the position on the
command line; previously, "-action watchdog" always won.

Paolo Bonzini (3):
  watchdog: add information from -watchdog help to -device help
  vl: deprecate -watchdog
  watchdog: remove select_watchdog_action

 docs/about/deprecated.rst  |  5 +++++
 hw/watchdog/sbsa_gwdt.c    |  3 ++-
 hw/watchdog/watchdog.c     | 14 --------------
 hw/watchdog/wdt_aspeed.c   |  3 ++-
 hw/watchdog/wdt_diag288.c  |  3 ++-
 hw/watchdog/wdt_i6300esb.c |  3 ++-
 hw/watchdog/wdt_ib700.c    |  3 ++-
 hw/watchdog/wdt_imx2.c     |  4 ++--
 include/hw/qdev-core.h     |  1 +
 include/sysemu/watchdog.h  |  1 -
 monitor/misc.c             | 15 ++++++++++++---
 softmmu/qdev-monitor.c     |  1 +
 softmmu/vl.c               | 11 ++++++-----
 13 files changed, 37 insertions(+), 30 deletions(-)

-- 
2.31.1

[PATCH 1/3] watchdog: add information from -watchdog help to -device help

[Paolo Bonzini]

List all watchdog devices in a separate category, and populate
their descriptions.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/watchdog/sbsa_gwdt.c    | 3 ++-
 hw/watchdog/wdt_aspeed.c   | 3 ++-
 hw/watchdog/wdt_diag288.c  | 3 ++-
 hw/watchdog/wdt_i6300esb.c | 3 ++-
 hw/watchdog/wdt_ib700.c    | 3 ++-
 hw/watchdog/wdt_imx2.c     | 4 ++--
 include/hw/qdev-core.h     | 1 +
 softmmu/qdev-monitor.c     | 1 +
 8 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/hw/watchdog/sbsa_gwdt.c b/hw/watchdog/sbsa_gwdt.c
index d0998f8489..e49cacd0e2 100644
--- a/hw/watchdog/sbsa_gwdt.c
+++ b/hw/watchdog/sbsa_gwdt.c
@@ -273,8 +273,9 @@ static void wdt_sbsa_gwdt_class_init(ObjectClass *klass, void *data)
     dc->realize = wdt_sbsa_gwdt_realize;
     dc->reset = wdt_sbsa_gwdt_reset;
     dc->hotpluggable = false;
-    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    set_bit(DEVICE_CATEGORY_WATCHDOG, dc->categories);
     dc->vmsd = &vmstate_sbsa_gwdt;
+    dc->desc = "SBSA-compliant generic watchdog device";
 }
 
 static const TypeInfo wdt_sbsa_gwdt_info = {
diff --git a/hw/watchdog/wdt_aspeed.c b/hw/watchdog/wdt_aspeed.c
index 146ffcd713..6aa6f90b66 100644
--- a/hw/watchdog/wdt_aspeed.c
+++ b/hw/watchdog/wdt_aspeed.c
@@ -293,9 +293,10 @@ static void aspeed_wdt_class_init(ObjectClass *klass, void *data)
     dc->desc = "ASPEED Watchdog Controller";
     dc->realize = aspeed_wdt_realize;
     dc->reset = aspeed_wdt_reset;
-    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    set_bit(DEVICE_CATEGORY_WATCHDOG, dc->categories);
     dc->vmsd = &vmstate_aspeed_wdt;
     device_class_set_props(dc, aspeed_wdt_properties);
+    dc->desc = "Aspeed watchdog device";
 }
 
 static const TypeInfo aspeed_wdt_info = {
diff --git a/hw/watchdog/wdt_diag288.c b/hw/watchdog/wdt_diag288.c
index e135a4de8b..9e8882a11c 100644
--- a/hw/watchdog/wdt_diag288.c
+++ b/hw/watchdog/wdt_diag288.c
@@ -122,9 +122,10 @@ static void wdt_diag288_class_init(ObjectClass *klass, void *data)
     dc->unrealize = wdt_diag288_unrealize;
     dc->reset = wdt_diag288_reset;
     dc->hotpluggable = false;
-    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    set_bit(DEVICE_CATEGORY_WATCHDOG, dc->categories);
     dc->vmsd = &vmstate_diag288;
     diag288->handle_timer = wdt_diag288_handle_timer;
+    dc->desc = "diag288 device for s390x platform";
 }
 
 static const TypeInfo wdt_diag288_info = {
diff --git a/hw/watchdog/wdt_i6300esb.c b/hw/watchdog/wdt_i6300esb.c
index 4c52e3bb9e..f99a1c9d29 100644
--- a/hw/watchdog/wdt_i6300esb.c
+++ b/hw/watchdog/wdt_i6300esb.c
@@ -476,7 +476,8 @@ static void i6300esb_class_init(ObjectClass *klass, void *data)
     k->class_id = PCI_CLASS_SYSTEM_OTHER;
     dc->reset = i6300esb_reset;
     dc->vmsd = &vmstate_i6300esb;
-    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    set_bit(DEVICE_CATEGORY_WATCHDOG, dc->categories);
+    dc->desc = "Intel 6300ESB";
 }
 
 static const TypeInfo i6300esb_info = {
diff --git a/hw/watchdog/wdt_ib700.c b/hw/watchdog/wdt_ib700.c
index 177aaa503f..91d1bdc0da 100644
--- a/hw/watchdog/wdt_ib700.c
+++ b/hw/watchdog/wdt_ib700.c
@@ -140,7 +140,8 @@ static void wdt_ib700_class_init(ObjectClass *klass, void *data)
     dc->realize = wdt_ib700_realize;
     dc->reset = wdt_ib700_reset;
     dc->vmsd = &vmstate_ib700;
-    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    set_bit(DEVICE_CATEGORY_WATCHDOG, dc->categories);
+    dc->desc = "iBASE 700";
 }
 
 static const TypeInfo wdt_ib700_info = {
diff --git a/hw/watchdog/wdt_imx2.c b/hw/watchdog/wdt_imx2.c
index a5fb76308f..c3128370b5 100644
--- a/hw/watchdog/wdt_imx2.c
+++ b/hw/watchdog/wdt_imx2.c
@@ -280,8 +280,8 @@ static void imx2_wdt_class_init(ObjectClass *klass, void *data)
     dc->realize = imx2_wdt_realize;
     dc->reset = imx2_wdt_reset;
     dc->vmsd = &vmstate_imx2_wdt;
-    dc->desc = "i.MX watchdog timer";
-    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    dc->desc = "i.MX2 watchdog timer";
+    set_bit(DEVICE_CATEGORY_WATCHDOG, dc->categories);
 }
 
 static const TypeInfo imx2_wdt_info = {
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 4ff19c714b..0dae5b6f6c 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -26,6 +26,7 @@ typedef enum DeviceCategory {
     DEVICE_CATEGORY_SOUND,
     DEVICE_CATEGORY_MISC,
     DEVICE_CATEGORY_CPU,
+    DEVICE_CATEGORY_WATCHDOG,
     DEVICE_CATEGORY_MAX
 } DeviceCategory;
 
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 3df99ce9fc..87dd87e801 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -162,6 +162,7 @@ static void qdev_print_devinfos(bool show_no_user)
         [DEVICE_CATEGORY_SOUND]   = "Sound",
         [DEVICE_CATEGORY_MISC]    = "Misc",
         [DEVICE_CATEGORY_CPU]     = "CPU",
+        [DEVICE_CATEGORY_WATCHDOG]= "Watchdog",
         [DEVICE_CATEGORY_MAX]     = "Uncategorized",
     };
     GSList *list, *elt;
-- 
2.31.1

[PATCH 2/3] vl: deprecate -watchdog

[Paolo Bonzini]

-watchdog is the same as -device except that it is case insensitive (and it
allows only watchdog devices of course).  Now that "-device help" can list
as such the available watchdog devices, we can deprecate it.

Note that even though -watchdog tries to be case insensitive, it fails
at that: "-watchdog i6300xyz" fails with "Unknown -watchdog device",
but "-watchdog i6300ESB" also fails (when the generated -device option
is processed) with an error "'i6300ESB' is not a valid device model name".
For this reason, the documentation update does not mention the case
insensitivity of -watchdog.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 docs/about/deprecated.rst | 5 +++++
 softmmu/vl.c              | 1 +
 2 files changed, 6 insertions(+)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 0bed6ecb1d..3540f5754b 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -160,6 +160,11 @@ Use ``-display sdl`` instead.
 
 Use ``-display curses`` instead.
 
+``-watchdog`` (since 6.2)
+'''''''''''''''''''''''''
+
+Use ``-device`` instead.
+
 ``-smp`` ("parameter=0" SMP configurations) (since 6.2)
 '''''''''''''''''''''''''''''''''''''''''''''''''''''''
 
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 55ab70eb97..33908860e7 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3216,6 +3216,7 @@ void qemu_init(int argc, char **argv, char **envp)
                     error_report("only one watchdog option may be given");
                     exit(1);
                 }
+                warn_report("-watchdog is deprecated; use -device instead.");
                 watchdog = optarg;
                 break;
             case QEMU_OPTION_action:
-- 
2.31.1

[PATCH 3/3] watchdog: remove select_watchdog_action

[Paolo Bonzini]

Instead of invoking select_watchdog_action from both HMP and command line,
go directly from HMP to QMP and use QemuOpts as the intermediary for the
command line.

This makes -watchdog-action explicitly a shortcut for "-action watchdog",
so that "-watchdog-action" and "-action watchdog" override each other
based on the position on the command line; previously, "-action watchdog"
always won.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/watchdog/watchdog.c    | 14 --------------
 include/sysemu/watchdog.h |  1 -
 monitor/misc.c            | 15 ++++++++++++---
 softmmu/vl.c              | 10 +++++-----
 4 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
index 0e98ffb73f..1437e6c5b6 100644
--- a/hw/watchdog/watchdog.c
+++ b/hw/watchdog/watchdog.c
@@ -76,20 +76,6 @@ int select_watchdog(const char *p)
     return 1;
 }
 
-int select_watchdog_action(const char *p)
-{
-    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;
-    qmp_watchdog_set_action(action, &error_abort);
-    return 0;
-}
-
 WatchdogAction get_watchdog_action(void)
 {
     return watchdog_action;
diff --git a/include/sysemu/watchdog.h b/include/sysemu/watchdog.h
index a08d16380d..d2d4901dbb 100644
--- a/include/sysemu/watchdog.h
+++ b/include/sysemu/watchdog.h
@@ -37,7 +37,6 @@ typedef struct WatchdogTimerModel WatchdogTimerModel;
 
 /* in hw/watchdog.c */
 int select_watchdog(const char *p);
-int select_watchdog_action(const char *action);
 WatchdogAction get_watchdog_action(void);
 void watchdog_add_model(WatchdogTimerModel *model);
 void watchdog_perform_action(void);
diff --git a/monitor/misc.c b/monitor/misc.c
index ffe7966870..f411675e32 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -70,6 +70,7 @@
 #include "qapi/qapi-commands-migration.h"
 #include "qapi/qapi-commands-misc.h"
 #include "qapi/qapi-commands-qom.h"
+#include "qapi/qapi-commands-run-state.h"
 #include "qapi/qapi-commands-trace.h"
 #include "qapi/qapi-init-commands.h"
 #include "qapi/error.h"
@@ -470,10 +471,18 @@ static void hmp_gdbserver(Monitor *mon, const QDict *qdict)
 
 static void hmp_watchdog_action(Monitor *mon, const QDict *qdict)
 {
-    const char *action = qdict_get_str(qdict, "action");
-    if (select_watchdog_action(action) == -1) {
-        monitor_printf(mon, "Unknown watchdog action '%s'\n", action);
+    Error *err = NULL;
+    WatchdogAction action;
+    char *qapi_value;
+
+    qapi_value = g_ascii_strdown(qdict_get_str(qdict, "action"), -1);
+    action = qapi_enum_parse(&WatchdogAction_lookup, qapi_value, -1, &err);
+    g_free(qapi_value);
+    if (err) {
+        hmp_handle_error(mon, err);
+        return;
     }
+    qmp_watchdog_set_action(action, &error_abort);
 }
 
 static void monitor_printc(Monitor *mon, int c)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 33908860e7..cf01bd5084 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3225,12 +3225,12 @@ void qemu_init(int argc, char **argv, char **envp)
                      exit(1);
                 }
                 break;
-            case QEMU_OPTION_watchdog_action:
-                if (select_watchdog_action(optarg) == -1) {
-                    error_report("unknown -watchdog-action parameter");
-                    exit(1);
-                }
+            case QEMU_OPTION_watchdog_action: {
+                QemuOpts *opts;
+                opts = qemu_opts_create(qemu_find_opts("action"), NULL, 0, &error_abort);
+                qemu_opt_set(opts, "watchdog", optarg, &error_abort);
                 break;
+            }
             case QEMU_OPTION_parallel:
                 add_device_config(DEV_PARALLEL, optarg);
                 default_parallel = 0;
-- 
2.31.1