qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] monitor: Refactoring in preparation for qemu-storage-daemon
@ 2019-12-18 16:19 Kevin Wolf
  2019-12-18 16:19 ` [PATCH 1/4] monitor: Move monitor option parsing to monitor/monitor.c Kevin Wolf
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Kevin Wolf @ 2019-12-18 16:19 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, armbru, qemu-devel

This series creates a QAPI module 'monitor' that can be used from tools
outside the system emulator and movees some monitor initialisation code
from vl.c to monitor code.

It is split from the series to introduce qemu-storage-daemon because
these refactorings make sense on their own as cleanups.

Kevin Wolf (4):
  monitor: Move monitor option parsing to monitor/monitor.c
  qapi: Create module 'monitor'
  monitor: Create monitor/qmp-cmds-monitor.c
  monitor: Move qmp_query_qmp_schema to qmp-cmds-monitor.c

 qapi/misc.json             | 212 ------------------------------------
 qapi/monitor.json          | 218 +++++++++++++++++++++++++++++++++++++
 qapi/qapi-schema.json      |   1 +
 include/monitor/monitor.h  |   4 +
 include/sysemu/sysemu.h    |   1 -
 monitor/monitor-internal.h |   4 +
 monitor/hmp-cmds.c         |   1 +
 monitor/misc.c             | 126 ---------------------
 monitor/monitor.c          |  52 +++++++++
 monitor/qmp-cmds-monitor.c | 169 ++++++++++++++++++++++++++++
 monitor/qmp-cmds.c         |  15 +--
 monitor/qmp.c              |   2 +-
 tests/qmp-test.c           |   2 +-
 ui/gtk.c                   |   1 +
 vl.c                       |  45 +-------
 monitor/Makefile.objs      |   3 +-
 qapi/Makefile.objs         |   4 +-
 17 files changed, 458 insertions(+), 402 deletions(-)
 create mode 100644 qapi/monitor.json
 create mode 100644 monitor/qmp-cmds-monitor.c

-- 
2.20.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] monitor: Move monitor option parsing to monitor/monitor.c
  2019-12-18 16:19 [PATCH 0/4] monitor: Refactoring in preparation for qemu-storage-daemon Kevin Wolf
@ 2019-12-18 16:19 ` Kevin Wolf
  2019-12-18 19:07   ` Markus Armbruster
  2019-12-18 16:19 ` [PATCH 2/4] qapi: Create module 'monitor' Kevin Wolf
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Kevin Wolf @ 2019-12-18 16:19 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, armbru, qemu-devel

Both the system emulators and tools with QMP support (specifically, the
planned storage daemon) will need to parse monitor options, so move that
code to monitor/monitor.c, which can be linked into binaries that aren't
a system emulator.

This patch moves the monitor option parsing from vl.c and adds an
allow_hmp parameter so that callers can support QMP without HMP.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 include/monitor/monitor.h |  4 +++
 include/sysemu/sysemu.h   |  1 -
 monitor/monitor.c         | 52 +++++++++++++++++++++++++++++++++++++++
 vl.c                      | 45 +--------------------------------
 4 files changed, 57 insertions(+), 45 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index a81eeff5f8..d3e8da36a5 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -3,6 +3,7 @@
 
 #include "block/block.h"
 #include "qapi/qapi-types-misc.h"
+#include "qemu/option.h"
 #include "qemu/readline.h"
 
 extern __thread Monitor *cur_mon;
@@ -10,12 +11,15 @@ typedef struct MonitorHMP MonitorHMP;
 
 #define QMP_REQ_QUEUE_LEN_MAX 8
 
+extern QemuOptsList qemu_mon_opts;
+
 bool monitor_cur_is_qmp(void);
 
 void monitor_init_globals(void);
 void monitor_init_globals_core(void);
 void monitor_init_qmp(Chardev *chr, bool pretty);
 void monitor_init_hmp(Chardev *chr, bool use_readline);
+int monitor_init_opts(QemuOpts *opts, bool allow_hmp, Error **errp);
 void monitor_cleanup(void);
 
 int monitor_suspend(Monitor *mon);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 80c57fdc4e..bbd02cf941 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -128,7 +128,6 @@ extern QemuOptsList qemu_netdev_opts;
 extern QemuOptsList qemu_nic_opts;
 extern QemuOptsList qemu_net_opts;
 extern QemuOptsList qemu_global_opts;
-extern QemuOptsList qemu_mon_opts;
 extern QemuOptsList qemu_semihosting_config_opts;
 
 #endif
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 12898b6448..316b71b928 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -609,6 +609,58 @@ void monitor_init_globals_core(void)
                                    NULL);
 }
 
+int monitor_init_opts(QemuOpts *opts, bool allow_hmp, Error **errp)
+{
+    Chardev *chr;
+    bool qmp;
+    bool pretty = false;
+    const char *chardev;
+    const char *mode;
+
+    mode = qemu_opt_get(opts, "mode");
+    if (mode == NULL) {
+        mode = allow_hmp ? "readline" : "control";
+    }
+    if (strcmp(mode, "readline") == 0) {
+        qmp = false;
+    } else if (strcmp(mode, "control") == 0) {
+        qmp = true;
+    } else {
+        error_setg(errp, "unknown monitor mode \"%s\"", mode);
+        return -1;
+    }
+    if (!allow_hmp && !qmp) {
+        error_setg(errp, "Only QMP is supported");
+        return -1;
+    }
+
+    if (!qmp && qemu_opt_get(opts, "pretty")) {
+        warn_report("'pretty' is deprecated for HMP monitors, it has no effect "
+                    "and will be removed in future versions");
+    }
+    if (qemu_opt_get_bool(opts, "pretty", 0)) {
+        pretty = true;
+    }
+
+    chardev = qemu_opt_get(opts, "chardev");
+    if (!chardev) {
+        error_report("chardev is required");
+        exit(1);
+    }
+    chr = qemu_chr_find(chardev);
+    if (chr == NULL) {
+        error_setg(errp, "chardev \"%s\" not found", chardev);
+        return -1;
+    }
+
+    if (qmp) {
+        monitor_init_qmp(chr, pretty);
+    } else {
+        monitor_init_hmp(chr, true);
+    }
+    return 0;
+}
+
 QemuOptsList qemu_mon_opts = {
     .name = "mon",
     .implied_opt_name = "chardev",
diff --git a/vl.c b/vl.c
index 94508300c3..352c68c897 100644
--- a/vl.c
+++ b/vl.c
@@ -2116,50 +2116,7 @@ static int fsdev_init_func(void *opaque, QemuOpts *opts, Error **errp)
 
 static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
 {
-    Chardev *chr;
-    bool qmp;
-    bool pretty = false;
-    const char *chardev;
-    const char *mode;
-
-    mode = qemu_opt_get(opts, "mode");
-    if (mode == NULL) {
-        mode = "readline";
-    }
-    if (strcmp(mode, "readline") == 0) {
-        qmp = false;
-    } else if (strcmp(mode, "control") == 0) {
-        qmp = true;
-    } else {
-        error_setg(errp, "unknown monitor mode \"%s\"", mode);
-        return -1;
-    }
-
-    if (!qmp && qemu_opt_get(opts, "pretty")) {
-        warn_report("'pretty' is deprecated for HMP monitors, it has no effect "
-                    "and will be removed in future versions");
-    }
-    if (qemu_opt_get_bool(opts, "pretty", 0)) {
-        pretty = true;
-    }
-
-    chardev = qemu_opt_get(opts, "chardev");
-    if (!chardev) {
-        error_report("chardev is required");
-        exit(1);
-    }
-    chr = qemu_chr_find(chardev);
-    if (chr == NULL) {
-        error_setg(errp, "chardev \"%s\" not found", chardev);
-        return -1;
-    }
-
-    if (qmp) {
-        monitor_init_qmp(chr, pretty);
-    } else {
-        monitor_init_hmp(chr, true);
-    }
-    return 0;
+    return monitor_init_opts(opts, true, errp);
 }
 
 static void monitor_parse(const char *optarg, const char *mode, bool pretty)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] qapi: Create module 'monitor'
  2019-12-18 16:19 [PATCH 0/4] monitor: Refactoring in preparation for qemu-storage-daemon Kevin Wolf
  2019-12-18 16:19 ` [PATCH 1/4] monitor: Move monitor option parsing to monitor/monitor.c Kevin Wolf
@ 2019-12-18 16:19 ` Kevin Wolf
  2019-12-18 19:20   ` Markus Armbruster
  2019-12-18 16:19 ` [PATCH 3/4] monitor: Create monitor/qmp-cmds-monitor.c Kevin Wolf
  2019-12-18 16:19 ` [PATCH 4/4] monitor: Move qmp_query_qmp_schema to qmp-cmds-monitor.c Kevin Wolf
  3 siblings, 1 reply; 9+ messages in thread
From: Kevin Wolf @ 2019-12-18 16:19 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, armbru, qemu-devel

misc.json contains definitions that are related to the system emulator,
so it can't be used for the storage daemon. This patch moves basic
functionality that is related to the monitor itself into a new
monitor.json, which could be used in tools as well.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/misc.json             | 212 ------------------------------------
 qapi/monitor.json          | 218 +++++++++++++++++++++++++++++++++++++
 qapi/qapi-schema.json      |   1 +
 monitor/monitor-internal.h |   1 +
 monitor/hmp-cmds.c         |   1 +
 monitor/qmp-cmds.c         |   1 +
 monitor/qmp.c              |   2 +-
 tests/qmp-test.c           |   2 +-
 ui/gtk.c                   |   1 +
 qapi/Makefile.objs         |   4 +-
 10 files changed, 227 insertions(+), 216 deletions(-)
 create mode 100644 qapi/monitor.json

diff --git a/qapi/misc.json b/qapi/misc.json
index 33b94e3589..cf656e9d4b 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -7,159 +7,6 @@
 
 { 'include': 'common.json' }
 
-##
-# @qmp_capabilities:
-#
-# Enable QMP capabilities.
-#
-# Arguments:
-#
-# @enable:   An optional list of QMPCapability values to enable.  The
-#            client must not enable any capability that is not
-#            mentioned in the QMP greeting message.  If the field is not
-#            provided, it means no QMP capabilities will be enabled.
-#            (since 2.12)
-#
-# Example:
-#
-# -> { "execute": "qmp_capabilities",
-#      "arguments": { "enable": [ "oob" ] } }
-# <- { "return": {} }
-#
-# Notes: This command is valid exactly when first connecting: it must be
-# issued before any other command will be accepted, and will fail once the
-# monitor is accepting other commands. (see qemu docs/interop/qmp-spec.txt)
-#
-# The QMP client needs to explicitly enable QMP capabilities, otherwise
-# all the QMP capabilities will be turned off by default.
-#
-# Since: 0.13
-#
-##
-{ 'command': 'qmp_capabilities',
-  'data': { '*enable': [ 'QMPCapability' ] },
-  'allow-preconfig': true }
-
-##
-# @QMPCapability:
-#
-# Enumeration of capabilities to be advertised during initial client
-# connection, used for agreeing on particular QMP extension behaviors.
-#
-# @oob:   QMP ability to support out-of-band requests.
-#         (Please refer to qmp-spec.txt for more information on OOB)
-#
-# Since: 2.12
-#
-##
-{ 'enum': 'QMPCapability',
-  'data': [ 'oob' ] }
-
-##
-# @VersionTriple:
-#
-# A three-part version number.
-#
-# @major:  The major version number.
-#
-# @minor:  The minor version number.
-#
-# @micro:  The micro version number.
-#
-# Since: 2.4
-##
-{ 'struct': 'VersionTriple',
-  'data': {'major': 'int', 'minor': 'int', 'micro': 'int'} }
-
-
-##
-# @VersionInfo:
-#
-# A description of QEMU's version.
-#
-# @qemu:        The version of QEMU.  By current convention, a micro
-#               version of 50 signifies a development branch.  A micro version
-#               greater than or equal to 90 signifies a release candidate for
-#               the next minor version.  A micro version of less than 50
-#               signifies a stable release.
-#
-# @package:     QEMU will always set this field to an empty string.  Downstream
-#               versions of QEMU should set this to a non-empty string.  The
-#               exact format depends on the downstream however it highly
-#               recommended that a unique name is used.
-#
-# Since: 0.14.0
-##
-{ 'struct': 'VersionInfo',
-  'data': {'qemu': 'VersionTriple', 'package': 'str'} }
-
-##
-# @query-version:
-#
-# Returns the current version of QEMU.
-#
-# Returns:  A @VersionInfo object describing the current version of QEMU.
-#
-# Since: 0.14.0
-#
-# Example:
-#
-# -> { "execute": "query-version" }
-# <- {
-#       "return":{
-#          "qemu":{
-#             "major":0,
-#             "minor":11,
-#             "micro":5
-#          },
-#          "package":""
-#       }
-#    }
-#
-##
-{ 'command': 'query-version', 'returns': 'VersionInfo',
-  'allow-preconfig': true }
-
-##
-# @CommandInfo:
-#
-# Information about a QMP command
-#
-# @name: The command name
-#
-# Since: 0.14.0
-##
-{ 'struct': 'CommandInfo', 'data': {'name': 'str'} }
-
-##
-# @query-commands:
-#
-# Return a list of supported QMP commands by this server
-#
-# Returns: A list of @CommandInfo for all supported commands
-#
-# Since: 0.14.0
-#
-# Example:
-#
-# -> { "execute": "query-commands" }
-# <- {
-#      "return":[
-#         {
-#            "name":"query-balloon"
-#         },
-#         {
-#            "name":"system_powerdown"
-#         }
-#      ]
-#    }
-#
-# Note: This example has been shortened as the real response is too long.
-#
-##
-{ 'command': 'query-commands', 'returns': ['CommandInfo'],
-  'allow-preconfig': true }
-
 ##
 # @LostTickPolicy:
 #
@@ -300,48 +147,6 @@
 ##
 { 'command': 'query-uuid', 'returns': 'UuidInfo', 'allow-preconfig': true }
 
-##
-# @EventInfo:
-#
-# Information about a QMP event
-#
-# @name: The event name
-#
-# Since: 1.2.0
-##
-{ 'struct': 'EventInfo', 'data': {'name': 'str'} }
-
-##
-# @query-events:
-#
-# Return information on QMP events.
-#
-# Returns: A list of @EventInfo.
-#
-# Since: 1.2.0
-#
-# Note: This command is deprecated, because its output doesn't reflect
-# compile-time configuration.  Use query-qmp-schema instead.
-#
-# Example:
-#
-# -> { "execute": "query-events" }
-# <- {
-#      "return": [
-#          {
-#             "name":"SHUTDOWN"
-#          },
-#          {
-#             "name":"RESET"
-#          }
-#       ]
-#    }
-#
-# Note: This example has been shortened as the real response is too long.
-#
-##
-{ 'command': 'query-events', 'returns': ['EventInfo'] }
-
 ##
 # @IOThreadInfo:
 #
@@ -764,23 +569,6 @@
 ##
 { 'command': 'query-pci', 'returns': ['PciInfo'] }
 
-##
-# @quit:
-#
-# This command will cause the QEMU process to exit gracefully.  While every
-# attempt is made to send the QMP response before terminating, this is not
-# guaranteed.  When using this interface, a premature EOF would not be
-# unexpected.
-#
-# Since: 0.14.0
-#
-# Example:
-#
-# -> { "execute": "quit" }
-# <- { "return": {} }
-##
-{ 'command': 'quit' }
-
 ##
 # @stop:
 #
diff --git a/qapi/monitor.json b/qapi/monitor.json
new file mode 100644
index 0000000000..a82a18da1a
--- /dev/null
+++ b/qapi/monitor.json
@@ -0,0 +1,218 @@
+# -*- Mode: Python -*-
+#
+
+##
+# = Monitor definitions (shared between system emulator and tools)
+##
+
+##
+# @qmp_capabilities:
+#
+# Enable QMP capabilities.
+#
+# Arguments:
+#
+# @enable:   An optional list of QMPCapability values to enable.  The
+#            client must not enable any capability that is not
+#            mentioned in the QMP greeting message.  If the field is not
+#            provided, it means no QMP capabilities will be enabled.
+#            (since 2.12)
+#
+# Example:
+#
+# -> { "execute": "qmp_capabilities",
+#      "arguments": { "enable": [ "oob" ] } }
+# <- { "return": {} }
+#
+# Notes: This command is valid exactly when first connecting: it must be
+# issued before any other command will be accepted, and will fail once the
+# monitor is accepting other commands. (see qemu docs/interop/qmp-spec.txt)
+#
+# The QMP client needs to explicitly enable QMP capabilities, otherwise
+# all the QMP capabilities will be turned off by default.
+#
+# Since: 0.13
+#
+##
+{ 'command': 'qmp_capabilities',
+  'data': { '*enable': [ 'QMPCapability' ] },
+  'allow-preconfig': true }
+
+##
+# @QMPCapability:
+#
+# Enumeration of capabilities to be advertised during initial client
+# connection, used for agreeing on particular QMP extension behaviors.
+#
+# @oob:   QMP ability to support out-of-band requests.
+#         (Please refer to qmp-spec.txt for more information on OOB)
+#
+# Since: 2.12
+#
+##
+{ 'enum': 'QMPCapability',
+  'data': [ 'oob' ] }
+
+##
+# @VersionTriple:
+#
+# A three-part version number.
+#
+# @major:  The major version number.
+#
+# @minor:  The minor version number.
+#
+# @micro:  The micro version number.
+#
+# Since: 2.4
+##
+{ 'struct': 'VersionTriple',
+  'data': {'major': 'int', 'minor': 'int', 'micro': 'int'} }
+
+
+##
+# @VersionInfo:
+#
+# A description of QEMU's version.
+#
+# @qemu:        The version of QEMU.  By current convention, a micro
+#               version of 50 signifies a development branch.  A micro version
+#               greater than or equal to 90 signifies a release candidate for
+#               the next minor version.  A micro version of less than 50
+#               signifies a stable release.
+#
+# @package:     QEMU will always set this field to an empty string.  Downstream
+#               versions of QEMU should set this to a non-empty string.  The
+#               exact format depends on the downstream however it highly
+#               recommended that a unique name is used.
+#
+# Since: 0.14.0
+##
+{ 'struct': 'VersionInfo',
+  'data': {'qemu': 'VersionTriple', 'package': 'str'} }
+
+##
+# @query-version:
+#
+# Returns the current version of QEMU.
+#
+# Returns:  A @VersionInfo object describing the current version of QEMU.
+#
+# Since: 0.14.0
+#
+# Example:
+#
+# -> { "execute": "query-version" }
+# <- {
+#       "return":{
+#          "qemu":{
+#             "major":0,
+#             "minor":11,
+#             "micro":5
+#          },
+#          "package":""
+#       }
+#    }
+#
+##
+{ 'command': 'query-version', 'returns': 'VersionInfo',
+  'allow-preconfig': true }
+
+##
+# @CommandInfo:
+#
+# Information about a QMP command
+#
+# @name: The command name
+#
+# Since: 0.14.0
+##
+{ 'struct': 'CommandInfo', 'data': {'name': 'str'} }
+
+##
+# @query-commands:
+#
+# Return a list of supported QMP commands by this server
+#
+# Returns: A list of @CommandInfo for all supported commands
+#
+# Since: 0.14.0
+#
+# Example:
+#
+# -> { "execute": "query-commands" }
+# <- {
+#      "return":[
+#         {
+#            "name":"query-balloon"
+#         },
+#         {
+#            "name":"system_powerdown"
+#         }
+#      ]
+#    }
+#
+# Note: This example has been shortened as the real response is too long.
+#
+##
+{ 'command': 'query-commands', 'returns': ['CommandInfo'],
+  'allow-preconfig': true }
+
+##
+# @EventInfo:
+#
+# Information about a QMP event
+#
+# @name: The event name
+#
+# Since: 1.2.0
+##
+{ 'struct': 'EventInfo', 'data': {'name': 'str'} }
+
+##
+# @query-events:
+#
+# Return information on QMP events.
+#
+# Returns: A list of @EventInfo.
+#
+# Since: 1.2.0
+#
+# Note: This command is deprecated, because its output doesn't reflect
+# compile-time configuration.  Use query-qmp-schema instead.
+#
+# Example:
+#
+# -> { "execute": "query-events" }
+# <- {
+#      "return": [
+#          {
+#             "name":"SHUTDOWN"
+#          },
+#          {
+#             "name":"RESET"
+#          }
+#       ]
+#    }
+#
+# Note: This example has been shortened as the real response is too long.
+#
+##
+{ 'command': 'query-events', 'returns': ['EventInfo'] }
+
+##
+# @quit:
+#
+# This command will cause the QEMU process to exit gracefully.  While every
+# attempt is made to send the QMP response before terminating, this is not
+# guaranteed.  When using this interface, a premature EOF would not be
+# unexpected.
+#
+# Since: 0.14.0
+#
+# Example:
+#
+# -> { "execute": "quit" }
+# <- { "return": {} }
+##
+{ 'command': 'quit' }
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 9751b11f8f..be90422ffe 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -103,6 +103,7 @@
 { 'include': 'qdev.json' }
 { 'include': 'machine.json' }
 { 'include': 'machine-target.json' }
+{ 'include': 'monitor.json' }
 { 'include': 'misc.json' }
 { 'include': 'misc-target.json' }
 { 'include': 'audio.json' }
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index d78f5ca190..451aa64c1a 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -27,6 +27,7 @@
 
 #include "chardev/char-fe.h"
 #include "monitor/monitor.h"
+#include "qapi/qapi-types-monitor.h"
 #include "qapi/qmp/dispatch.h"
 #include "qapi/qmp/json-parser.h"
 #include "qemu/readline.h"
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index b2551c16d1..e858fcfc49 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -33,6 +33,7 @@
 #include "qapi/qapi-commands-char.h"
 #include "qapi/qapi-commands-migration.h"
 #include "qapi/qapi-commands-misc.h"
+#include "qapi/qapi-commands-monitor.h"
 #include "qapi/qapi-commands-net.h"
 #include "qapi/qapi-commands-rocker.h"
 #include "qapi/qapi-commands-run-state.h"
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 0880341a2d..b40a9b74de 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -34,6 +34,7 @@
 #include "qapi/qapi-commands-block-core.h"
 #include "qapi/qapi-commands-machine.h"
 #include "qapi/qapi-commands-misc.h"
+#include "qapi/qapi-commands-monitor.h"
 #include "qapi/qapi-commands-ui.h"
 #include "qapi/qmp/qerror.h"
 #include "hw/mem/memory-device.h"
diff --git a/monitor/qmp.c b/monitor/qmp.c
index b67a8e7d1f..711e5d7a40 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -27,7 +27,7 @@
 #include "chardev/char-io.h"
 #include "monitor-internal.h"
 #include "qapi/error.h"
-#include "qapi/qapi-commands-misc.h"
+#include "qapi/qapi-commands-monitor.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/qlist.h"
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 1b0eb69832..7fc646ffc2 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -13,7 +13,7 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 #include "qapi/error.h"
-#include "qapi/qapi-visit-misc.h"
+#include "qapi/qapi-visit-monitor.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qlist.h"
 #include "qapi/qobject-input-visitor.h"
diff --git a/ui/gtk.c b/ui/gtk.c
index 692ccc7bbb..7f4d28b5fd 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -33,6 +33,7 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-misc.h"
+#include "qapi/qapi-commands-monitor.h"
 #include "qemu/cutils.h"
 
 #include "ui/console.h"
diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
index dd3f5e6f94..519b6f1a8e 100644
--- a/qapi/Makefile.objs
+++ b/qapi/Makefile.objs
@@ -6,8 +6,8 @@ util-obj-y += qmp-event.o
 util-obj-y += qapi-util.o
 
 QAPI_COMMON_MODULES = audio authz block-core block char common crypto
-QAPI_COMMON_MODULES += dump error introspect job machine migration misc net
-QAPI_COMMON_MODULES += qdev qom rdma rocker run-state sockets tpm
+QAPI_COMMON_MODULES += dump error introspect job machine migration misc monitor
+QAPI_COMMON_MODULES += net qdev qom rdma rocker run-state sockets tpm
 QAPI_COMMON_MODULES += trace transaction ui
 QAPI_TARGET_MODULES = machine-target misc-target
 QAPI_MODULES = $(QAPI_COMMON_MODULES) $(QAPI_TARGET_MODULES)
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] monitor: Create monitor/qmp-cmds-monitor.c
  2019-12-18 16:19 [PATCH 0/4] monitor: Refactoring in preparation for qemu-storage-daemon Kevin Wolf
  2019-12-18 16:19 ` [PATCH 1/4] monitor: Move monitor option parsing to monitor/monitor.c Kevin Wolf
  2019-12-18 16:19 ` [PATCH 2/4] qapi: Create module 'monitor' Kevin Wolf
@ 2019-12-18 16:19 ` Kevin Wolf
  2019-12-18 16:19 ` [PATCH 4/4] monitor: Move qmp_query_qmp_schema to qmp-cmds-monitor.c Kevin Wolf
  3 siblings, 0 replies; 9+ messages in thread
From: Kevin Wolf @ 2019-12-18 16:19 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, armbru, qemu-devel

Move all of the QMP commands handlers to implement the 'monitor' module
(qapi/monitor.json) that can be shared between the system emulator and
tools such as a storage daemon to a new file monitor/qmp-cmds-monitor.c.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 monitor/misc.c             | 110 --------------------------
 monitor/qmp-cmds-monitor.c | 153 +++++++++++++++++++++++++++++++++++++
 monitor/qmp-cmds.c         |  14 ----
 monitor/Makefile.objs      |   3 +-
 4 files changed, 155 insertions(+), 125 deletions(-)
 create mode 100644 monitor/qmp-cmds-monitor.c

diff --git a/monitor/misc.c b/monitor/misc.c
index 3baa15f3bf..c647ba6313 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -67,7 +67,6 @@
 #include "qemu/thread.h"
 #include "block/qapi.h"
 #include "qapi/qapi-commands.h"
-#include "qapi/qapi-emit-events.h"
 #include "qapi/error.h"
 #include "qapi/qmp-event.h"
 #include "qapi/qapi-introspect.h"
@@ -230,58 +229,6 @@ static void hmp_info_help(Monitor *mon, const QDict *qdict)
     help_cmd(mon, "info");
 }
 
-static void query_commands_cb(QmpCommand *cmd, void *opaque)
-{
-    CommandInfoList *info, **list = opaque;
-
-    if (!cmd->enabled) {
-        return;
-    }
-
-    info = g_malloc0(sizeof(*info));
-    info->value = g_malloc0(sizeof(*info->value));
-    info->value->name = g_strdup(cmd->name);
-    info->next = *list;
-    *list = info;
-}
-
-CommandInfoList *qmp_query_commands(Error **errp)
-{
-    CommandInfoList *list = NULL;
-    MonitorQMP *mon;
-
-    assert(monitor_is_qmp(cur_mon));
-    mon = container_of(cur_mon, MonitorQMP, common);
-
-    qmp_for_each_command(mon->commands, query_commands_cb, &list);
-
-    return list;
-}
-
-EventInfoList *qmp_query_events(Error **errp)
-{
-    /*
-     * TODO This deprecated command is the only user of
-     * QAPIEvent_str() and QAPIEvent_lookup[].  When the command goes,
-     * they should go, too.
-     */
-    EventInfoList *info, *ev_list = NULL;
-    QAPIEvent e;
-
-    for (e = 0 ; e < QAPI_EVENT__MAX ; e++) {
-        const char *event_name = QAPIEvent_str(e);
-        assert(event_name != NULL);
-        info = g_malloc0(sizeof(*info));
-        info->value = g_malloc0(sizeof(*info->value));
-        info->value->name = g_strdup(event_name);
-
-        info->next = ev_list;
-        ev_list = info;
-    }
-
-    return ev_list;
-}
-
 /*
  * Minor hack: generated marshalling suppressed for this command
  * ('gen': false in the schema) so we can parse the JSON string
@@ -320,63 +267,6 @@ static void monitor_init_qmp_commands(void)
                          qmp_marshal_qmp_capabilities, QCO_ALLOW_PRECONFIG);
 }
 
-/*
- * Accept QMP capabilities in @list for @mon.
- * On success, set mon->qmp.capab[], and return true.
- * On error, set @errp, and return false.
- */
-static bool qmp_caps_accept(MonitorQMP *mon, QMPCapabilityList *list,
-                            Error **errp)
-{
-    GString *unavailable = NULL;
-    bool capab[QMP_CAPABILITY__MAX];
-
-    memset(capab, 0, sizeof(capab));
-
-    for (; list; list = list->next) {
-        if (!mon->capab_offered[list->value]) {
-            if (!unavailable) {
-                unavailable = g_string_new(QMPCapability_str(list->value));
-            } else {
-                g_string_append_printf(unavailable, ", %s",
-                                      QMPCapability_str(list->value));
-            }
-        }
-        capab[list->value] = true;
-    }
-
-    if (unavailable) {
-        error_setg(errp, "Capability %s not available", unavailable->str);
-        g_string_free(unavailable, true);
-        return false;
-    }
-
-    memcpy(mon->capab, capab, sizeof(capab));
-    return true;
-}
-
-void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable,
-                          Error **errp)
-{
-    MonitorQMP *mon;
-
-    assert(monitor_is_qmp(cur_mon));
-    mon = container_of(cur_mon, MonitorQMP, common);
-
-    if (mon->commands == &qmp_commands) {
-        error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
-                  "Capabilities negotiation is already complete, command "
-                  "ignored");
-        return;
-    }
-
-    if (!qmp_caps_accept(mon, enable, errp)) {
-        return;
-    }
-
-    mon->commands = &qmp_commands;
-}
-
 /* Set the current CPU defined by the user. Callers must hold BQL. */
 int monitor_set_cpu(int cpu_index)
 {
diff --git a/monitor/qmp-cmds-monitor.c b/monitor/qmp-cmds-monitor.c
new file mode 100644
index 0000000000..acebfd3716
--- /dev/null
+++ b/monitor/qmp-cmds-monitor.c
@@ -0,0 +1,153 @@
+/*
+ * QMP commands related to the monitor (common functions for sysemu and tools)
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+
+#include "monitor-internal.h"
+#include "qemu-version.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-monitor.h"
+#include "qapi/qapi-emit-events.h"
+
+/*
+ * Accept QMP capabilities in @list for @mon.
+ * On success, set mon->qmp.capab[], and return true.
+ * On error, set @errp, and return false.
+ */
+static bool qmp_caps_accept(MonitorQMP *mon, QMPCapabilityList *list,
+                            Error **errp)
+{
+    GString *unavailable = NULL;
+    bool capab[QMP_CAPABILITY__MAX];
+
+    memset(capab, 0, sizeof(capab));
+
+    for (; list; list = list->next) {
+        if (!mon->capab_offered[list->value]) {
+            if (!unavailable) {
+                unavailable = g_string_new(QMPCapability_str(list->value));
+            } else {
+                g_string_append_printf(unavailable, ", %s",
+                                      QMPCapability_str(list->value));
+            }
+        }
+        capab[list->value] = true;
+    }
+
+    if (unavailable) {
+        error_setg(errp, "Capability %s not available", unavailable->str);
+        g_string_free(unavailable, true);
+        return false;
+    }
+
+    memcpy(mon->capab, capab, sizeof(capab));
+    return true;
+}
+
+void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable,
+                          Error **errp)
+{
+    MonitorQMP *mon;
+
+    assert(monitor_is_qmp(cur_mon));
+    mon = container_of(cur_mon, MonitorQMP, common);
+
+    if (mon->commands == &qmp_commands) {
+        error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
+                  "Capabilities negotiation is already complete, command "
+                  "ignored");
+        return;
+    }
+
+    if (!qmp_caps_accept(mon, enable, errp)) {
+        return;
+    }
+
+    mon->commands = &qmp_commands;
+}
+
+VersionInfo *qmp_query_version(Error **errp)
+{
+    VersionInfo *info = g_new0(VersionInfo, 1);
+
+    info->qemu = g_new0(VersionTriple, 1);
+    info->qemu->major = QEMU_VERSION_MAJOR;
+    info->qemu->minor = QEMU_VERSION_MINOR;
+    info->qemu->micro = QEMU_VERSION_MICRO;
+    info->package = g_strdup(QEMU_PKGVERSION);
+
+    return info;
+}
+
+static void query_commands_cb(QmpCommand *cmd, void *opaque)
+{
+    CommandInfoList *info, **list = opaque;
+
+    if (!cmd->enabled) {
+        return;
+    }
+
+    info = g_malloc0(sizeof(*info));
+    info->value = g_malloc0(sizeof(*info->value));
+    info->value->name = g_strdup(cmd->name);
+    info->next = *list;
+    *list = info;
+}
+
+CommandInfoList *qmp_query_commands(Error **errp)
+{
+    CommandInfoList *list = NULL;
+    MonitorQMP *mon;
+
+    assert(monitor_is_qmp(cur_mon));
+    mon = container_of(cur_mon, MonitorQMP, common);
+
+    qmp_for_each_command(mon->commands, query_commands_cb, &list);
+
+    return list;
+}
+
+EventInfoList *qmp_query_events(Error **errp)
+{
+    /*
+     * TODO This deprecated command is the only user of
+     * QAPIEvent_str() and QAPIEvent_lookup[].  When the command goes,
+     * they should go, too.
+     */
+    EventInfoList *info, *ev_list = NULL;
+    QAPIEvent e;
+
+    for (e = 0 ; e < QAPI_EVENT__MAX ; e++) {
+        const char *event_name = QAPIEvent_str(e);
+        assert(event_name != NULL);
+        info = g_malloc0(sizeof(*info));
+        info->value = g_malloc0(sizeof(*info->value));
+        info->value->name = g_strdup(event_name);
+
+        info->next = ev_list;
+        ev_list = info;
+    }
+
+    return ev_list;
+}
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index b40a9b74de..22a3375194 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -15,7 +15,6 @@
 
 #include "qemu/osdep.h"
 #include "qemu-common.h"
-#include "qemu-version.h"
 #include "qemu/cutils.h"
 #include "qemu/option.h"
 #include "monitor/monitor.h"
@@ -52,19 +51,6 @@ NameInfo *qmp_query_name(Error **errp)
     return info;
 }
 
-VersionInfo *qmp_query_version(Error **errp)
-{
-    VersionInfo *info = g_new0(VersionInfo, 1);
-
-    info->qemu = g_new0(VersionTriple, 1);
-    info->qemu->major = QEMU_VERSION_MAJOR;
-    info->qemu->minor = QEMU_VERSION_MINOR;
-    info->qemu->micro = QEMU_VERSION_MICRO;
-    info->package = g_strdup(QEMU_PKGVERSION);
-
-    return info;
-}
-
 KvmInfo *qmp_query_kvm(Error **errp)
 {
     KvmInfo *info = g_malloc0(sizeof(*info));
diff --git a/monitor/Makefile.objs b/monitor/Makefile.objs
index e91a8581cd..15eb6380c5 100644
--- a/monitor/Makefile.objs
+++ b/monitor/Makefile.objs
@@ -1,3 +1,4 @@
 obj-y += misc.o
 common-obj-y += monitor.o qmp.o hmp.o
-common-obj-y += qmp-cmds.o hmp-cmds.o
+common-obj-y += qmp-cmds.o qmp-cmds-monitor.o
+common-obj-y += hmp-cmds.o
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] monitor: Move qmp_query_qmp_schema to qmp-cmds-monitor.c
  2019-12-18 16:19 [PATCH 0/4] monitor: Refactoring in preparation for qemu-storage-daemon Kevin Wolf
                   ` (2 preceding siblings ...)
  2019-12-18 16:19 ` [PATCH 3/4] monitor: Create monitor/qmp-cmds-monitor.c Kevin Wolf
@ 2019-12-18 16:19 ` Kevin Wolf
  3 siblings, 0 replies; 9+ messages in thread
From: Kevin Wolf @ 2019-12-18 16:19 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, armbru, qemu-devel

monitor/misc.c contains code that works only in the system emulator, so
it can't be linked to tools like a storage daemon. In order to make
schema introspection available for tools, move the function to
monitor/qmp-cmds-monitor.c, which can be linked into the storage daemon.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 monitor/monitor-internal.h |  3 +++
 monitor/misc.c             | 16 ----------------
 monitor/qmp-cmds-monitor.c | 16 ++++++++++++++++
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 451aa64c1a..b91cb09ee5 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -180,4 +180,7 @@ void help_cmd(Monitor *mon, const char *name);
 void handle_hmp_command(MonitorHMP *mon, const char *cmdline);
 int hmp_compare_cmd(const char *name, const char *list);
 
+void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
+                                 Error **errp);
+
 #endif
diff --git a/monitor/misc.c b/monitor/misc.c
index c647ba6313..54c89b9391 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -69,7 +69,6 @@
 #include "qapi/qapi-commands.h"
 #include "qapi/error.h"
 #include "qapi/qmp-event.h"
-#include "qapi/qapi-introspect.h"
 #include "sysemu/cpus.h"
 #include "qemu/cutils.h"
 #include "tcg/tcg.h"
@@ -229,21 +228,6 @@ static void hmp_info_help(Monitor *mon, const QDict *qdict)
     help_cmd(mon, "info");
 }
 
-/*
- * Minor hack: generated marshalling suppressed for this command
- * ('gen': false in the schema) so we can parse the JSON string
- * directly into QObject instead of first parsing it with
- * visit_type_SchemaInfoList() into a SchemaInfoList, then marshal it
- * to QObject with generated output marshallers, every time.  Instead,
- * we do it in test-qobject-input-visitor.c, just to make sure
- * qapi-gen.py's output actually conforms to the schema.
- */
-static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
-                                 Error **errp)
-{
-    *ret_data = qobject_from_qlit(&qmp_schema_qlit);
-}
-
 static void monitor_init_qmp_commands(void)
 {
     /*
diff --git a/monitor/qmp-cmds-monitor.c b/monitor/qmp-cmds-monitor.c
index acebfd3716..7215392f3e 100644
--- a/monitor/qmp-cmds-monitor.c
+++ b/monitor/qmp-cmds-monitor.c
@@ -29,6 +29,7 @@
 #include "qapi/error.h"
 #include "qapi/qapi-commands-monitor.h"
 #include "qapi/qapi-emit-events.h"
+#include "qapi/qapi-introspect.h"
 
 /*
  * Accept QMP capabilities in @list for @mon.
@@ -151,3 +152,18 @@ EventInfoList *qmp_query_events(Error **errp)
 
     return ev_list;
 }
+
+/*
+ * Minor hack: generated marshalling suppressed for this command
+ * ('gen': false in the schema) so we can parse the JSON string
+ * directly into QObject instead of first parsing it with
+ * visit_type_SchemaInfoList() into a SchemaInfoList, then marshal it
+ * to QObject with generated output marshallers, every time.  Instead,
+ * we do it in test-qobject-input-visitor.c, just to make sure
+ * qapi-gen.py's output actually conforms to the schema.
+ */
+void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
+                                 Error **errp)
+{
+    *ret_data = qobject_from_qlit(&qmp_schema_qlit);
+}
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] monitor: Move monitor option parsing to monitor/monitor.c
  2019-12-18 16:19 ` [PATCH 1/4] monitor: Move monitor option parsing to monitor/monitor.c Kevin Wolf
@ 2019-12-18 19:07   ` Markus Armbruster
  0 siblings, 0 replies; 9+ messages in thread
From: Markus Armbruster @ 2019-12-18 19:07 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-devel, qemu-block

Kevin Wolf <kwolf@redhat.com> writes:

> Both the system emulators and tools with QMP support (specifically, the
> planned storage daemon) will need to parse monitor options, so move that
> code to monitor/monitor.c, which can be linked into binaries that aren't
> a system emulator.
>
> This patch moves the monitor option parsing from vl.c and adds an
> allow_hmp parameter so that callers can support QMP without HMP.

New parameter is always true after this series.

Please split this patch: one patch to move the code, one to add the
parameter.  Delay the second one until you have a user.

> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  include/monitor/monitor.h |  4 +++
>  include/sysemu/sysemu.h   |  1 -
>  monitor/monitor.c         | 52 +++++++++++++++++++++++++++++++++++++++
>  vl.c                      | 45 +--------------------------------
>  4 files changed, 57 insertions(+), 45 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index a81eeff5f8..d3e8da36a5 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -3,6 +3,7 @@
>  
>  #include "block/block.h"
>  #include "qapi/qapi-types-misc.h"
> +#include "qemu/option.h"
>  #include "qemu/readline.h"
>  
>  extern __thread Monitor *cur_mon;
> @@ -10,12 +11,15 @@ typedef struct MonitorHMP MonitorHMP;
>  
>  #define QMP_REQ_QUEUE_LEN_MAX 8
>  
> +extern QemuOptsList qemu_mon_opts;
> +
>  bool monitor_cur_is_qmp(void);
>  
>  void monitor_init_globals(void);
>  void monitor_init_globals_core(void);
>  void monitor_init_qmp(Chardev *chr, bool pretty);
>  void monitor_init_hmp(Chardev *chr, bool use_readline);
> +int monitor_init_opts(QemuOpts *opts, bool allow_hmp, Error **errp);
>  void monitor_cleanup(void);
>  
>  int monitor_suspend(Monitor *mon);
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 80c57fdc4e..bbd02cf941 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -128,7 +128,6 @@ extern QemuOptsList qemu_netdev_opts;
>  extern QemuOptsList qemu_nic_opts;
>  extern QemuOptsList qemu_net_opts;
>  extern QemuOptsList qemu_global_opts;
> -extern QemuOptsList qemu_mon_opts;
>  extern QemuOptsList qemu_semihosting_config_opts;
>  
>  #endif
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 12898b6448..316b71b928 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -609,6 +609,58 @@ void monitor_init_globals_core(void)
>                                     NULL);
>  }
>  
> +int monitor_init_opts(QemuOpts *opts, bool allow_hmp, Error **errp)
> +{
> +    Chardev *chr;
> +    bool qmp;
> +    bool pretty = false;
> +    const char *chardev;
> +    const char *mode;
> +
> +    mode = qemu_opt_get(opts, "mode");
> +    if (mode == NULL) {
> +        mode = allow_hmp ? "readline" : "control";
> +    }
> +    if (strcmp(mode, "readline") == 0) {
> +        qmp = false;
> +    } else if (strcmp(mode, "control") == 0) {
> +        qmp = true;
> +    } else {
> +        error_setg(errp, "unknown monitor mode \"%s\"", mode);
> +        return -1;
> +    }
> +    if (!allow_hmp && !qmp) {
> +        error_setg(errp, "Only QMP is supported");
> +        return -1;
> +    }

I'm not sure we should recognize "mode" when !allow_hmp.  Hard to
decide as long as allow_hmp is always true.

> +
> +    if (!qmp && qemu_opt_get(opts, "pretty")) {
> +        warn_report("'pretty' is deprecated for HMP monitors, it has no effect "
> +                    "and will be removed in future versions");
> +    }
> +    if (qemu_opt_get_bool(opts, "pretty", 0)) {
> +        pretty = true;
> +    }
> +
> +    chardev = qemu_opt_get(opts, "chardev");
> +    if (!chardev) {
> +        error_report("chardev is required");
> +        exit(1);
> +    }
> +    chr = qemu_chr_find(chardev);
> +    if (chr == NULL) {
> +        error_setg(errp, "chardev \"%s\" not found", chardev);
> +        return -1;
> +    }
> +
> +    if (qmp) {
> +        monitor_init_qmp(chr, pretty);
> +    } else {
> +        monitor_init_hmp(chr, true);
> +    }
> +    return 0;
> +}
> +
>  QemuOptsList qemu_mon_opts = {
>      .name = "mon",
>      .implied_opt_name = "chardev",
> diff --git a/vl.c b/vl.c
> index 94508300c3..352c68c897 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2116,50 +2116,7 @@ static int fsdev_init_func(void *opaque, QemuOpts *opts, Error **errp)
>  
>  static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
>  {
> -    Chardev *chr;
> -    bool qmp;
> -    bool pretty = false;
> -    const char *chardev;
> -    const char *mode;
> -
> -    mode = qemu_opt_get(opts, "mode");
> -    if (mode == NULL) {
> -        mode = "readline";
> -    }
> -    if (strcmp(mode, "readline") == 0) {
> -        qmp = false;
> -    } else if (strcmp(mode, "control") == 0) {
> -        qmp = true;
> -    } else {
> -        error_setg(errp, "unknown monitor mode \"%s\"", mode);
> -        return -1;
> -    }
> -
> -    if (!qmp && qemu_opt_get(opts, "pretty")) {
> -        warn_report("'pretty' is deprecated for HMP monitors, it has no effect "
> -                    "and will be removed in future versions");
> -    }
> -    if (qemu_opt_get_bool(opts, "pretty", 0)) {
> -        pretty = true;
> -    }
> -
> -    chardev = qemu_opt_get(opts, "chardev");
> -    if (!chardev) {
> -        error_report("chardev is required");
> -        exit(1);
> -    }
> -    chr = qemu_chr_find(chardev);
> -    if (chr == NULL) {
> -        error_setg(errp, "chardev \"%s\" not found", chardev);
> -        return -1;
> -    }
> -
> -    if (qmp) {
> -        monitor_init_qmp(chr, pretty);
> -    } else {
> -        monitor_init_hmp(chr, true);
> -    }
> -    return 0;
> +    return monitor_init_opts(opts, true, errp);
>  }
>  
>  static void monitor_parse(const char *optarg, const char *mode, bool pretty)

Less the new @allow_hmp parameter:
Reviewed-by: Markus Armbruster <armbru@redhat.com>



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] qapi: Create module 'monitor'
  2019-12-18 16:19 ` [PATCH 2/4] qapi: Create module 'monitor' Kevin Wolf
@ 2019-12-18 19:20   ` Markus Armbruster
  2019-12-19  8:25     ` Kevin Wolf
  2019-12-19  8:28     ` Kevin Wolf
  0 siblings, 2 replies; 9+ messages in thread
From: Markus Armbruster @ 2019-12-18 19:20 UTC (permalink / raw)
  To: Kevin Wolf; +Cc: qemu-devel, qemu-block

Kevin Wolf <kwolf@redhat.com> writes:

> misc.json contains definitions that are related to the system emulator,
> so it can't be used for the storage daemon. This patch moves basic
> functionality that is related to the monitor itself into a new
> monitor.json, which could be used in tools as well.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/misc.json             | 212 ------------------------------------
>  qapi/monitor.json          | 218 +++++++++++++++++++++++++++++++++++++
>  qapi/qapi-schema.json      |   1 +
>  monitor/monitor-internal.h |   1 +
>  monitor/hmp-cmds.c         |   1 +
>  monitor/qmp-cmds.c         |   1 +
>  monitor/qmp.c              |   2 +-
>  tests/qmp-test.c           |   2 +-
>  ui/gtk.c                   |   1 +
>  qapi/Makefile.objs         |   4 +-
>  10 files changed, 227 insertions(+), 216 deletions(-)
>  create mode 100644 qapi/monitor.json
>
> diff --git a/qapi/misc.json b/qapi/misc.json
> index 33b94e3589..cf656e9d4b 100644
> --- a/qapi/misc.json
> +++ b/qapi/misc.json
> @@ -7,159 +7,6 @@
>  
>  { 'include': 'common.json' }
>  
> -##
> -# @qmp_capabilities:
> -#
> -# Enable QMP capabilities.
> -#
> -# Arguments:
> -#
> -# @enable:   An optional list of QMPCapability values to enable.  The
> -#            client must not enable any capability that is not
> -#            mentioned in the QMP greeting message.  If the field is not
> -#            provided, it means no QMP capabilities will be enabled.
> -#            (since 2.12)
> -#
> -# Example:
> -#
> -# -> { "execute": "qmp_capabilities",
> -#      "arguments": { "enable": [ "oob" ] } }
> -# <- { "return": {} }
> -#
> -# Notes: This command is valid exactly when first connecting: it must be
> -# issued before any other command will be accepted, and will fail once the
> -# monitor is accepting other commands. (see qemu docs/interop/qmp-spec.txt)
> -#
> -# The QMP client needs to explicitly enable QMP capabilities, otherwise
> -# all the QMP capabilities will be turned off by default.
> -#
> -# Since: 0.13
> -#
> -##
> -{ 'command': 'qmp_capabilities',
> -  'data': { '*enable': [ 'QMPCapability' ] },
> -  'allow-preconfig': true }
> -
> -##
> -# @QMPCapability:
> -#
> -# Enumeration of capabilities to be advertised during initial client
> -# connection, used for agreeing on particular QMP extension behaviors.
> -#
> -# @oob:   QMP ability to support out-of-band requests.
> -#         (Please refer to qmp-spec.txt for more information on OOB)
> -#
> -# Since: 2.12
> -#
> -##
> -{ 'enum': 'QMPCapability',
> -  'data': [ 'oob' ] }
> -
> -##
> -# @VersionTriple:
> -#
> -# A three-part version number.
> -#
> -# @major:  The major version number.
> -#
> -# @minor:  The minor version number.
> -#
> -# @micro:  The micro version number.
> -#
> -# Since: 2.4
> -##
> -{ 'struct': 'VersionTriple',
> -  'data': {'major': 'int', 'minor': 'int', 'micro': 'int'} }
> -
> -
> -##
> -# @VersionInfo:
> -#
> -# A description of QEMU's version.
> -#
> -# @qemu:        The version of QEMU.  By current convention, a micro
> -#               version of 50 signifies a development branch.  A micro version
> -#               greater than or equal to 90 signifies a release candidate for
> -#               the next minor version.  A micro version of less than 50
> -#               signifies a stable release.
> -#
> -# @package:     QEMU will always set this field to an empty string.  Downstream
> -#               versions of QEMU should set this to a non-empty string.  The
> -#               exact format depends on the downstream however it highly
> -#               recommended that a unique name is used.
> -#
> -# Since: 0.14.0
> -##
> -{ 'struct': 'VersionInfo',
> -  'data': {'qemu': 'VersionTriple', 'package': 'str'} }
> -
> -##
> -# @query-version:
> -#
> -# Returns the current version of QEMU.
> -#
> -# Returns:  A @VersionInfo object describing the current version of QEMU.
> -#
> -# Since: 0.14.0
> -#
> -# Example:
> -#
> -# -> { "execute": "query-version" }
> -# <- {
> -#       "return":{
> -#          "qemu":{
> -#             "major":0,
> -#             "minor":11,
> -#             "micro":5
> -#          },
> -#          "package":""
> -#       }
> -#    }
> -#
> -##
> -{ 'command': 'query-version', 'returns': 'VersionInfo',
> -  'allow-preconfig': true }
> -
> -##
> -# @CommandInfo:
> -#
> -# Information about a QMP command
> -#
> -# @name: The command name
> -#
> -# Since: 0.14.0
> -##
> -{ 'struct': 'CommandInfo', 'data': {'name': 'str'} }
> -
> -##
> -# @query-commands:
> -#
> -# Return a list of supported QMP commands by this server
> -#
> -# Returns: A list of @CommandInfo for all supported commands
> -#
> -# Since: 0.14.0
> -#
> -# Example:
> -#
> -# -> { "execute": "query-commands" }
> -# <- {
> -#      "return":[
> -#         {
> -#            "name":"query-balloon"
> -#         },
> -#         {
> -#            "name":"system_powerdown"
> -#         }
> -#      ]
> -#    }
> -#
> -# Note: This example has been shortened as the real response is too long.
> -#
> -##
> -{ 'command': 'query-commands', 'returns': ['CommandInfo'],
> -  'allow-preconfig': true }
> -
>  ##
>  # @LostTickPolicy:
>  #
> @@ -300,48 +147,6 @@
>  ##
>  { 'command': 'query-uuid', 'returns': 'UuidInfo', 'allow-preconfig': true }
>  
> -##
> -# @EventInfo:
> -#
> -# Information about a QMP event
> -#
> -# @name: The event name
> -#
> -# Since: 1.2.0
> -##
> -{ 'struct': 'EventInfo', 'data': {'name': 'str'} }
> -
> -##
> -# @query-events:
> -#
> -# Return information on QMP events.
> -#
> -# Returns: A list of @EventInfo.
> -#
> -# Since: 1.2.0
> -#
> -# Note: This command is deprecated, because its output doesn't reflect
> -# compile-time configuration.  Use query-qmp-schema instead.
> -#
> -# Example:
> -#
> -# -> { "execute": "query-events" }
> -# <- {
> -#      "return": [
> -#          {
> -#             "name":"SHUTDOWN"
> -#          },
> -#          {
> -#             "name":"RESET"
> -#          }
> -#       ]
> -#    }
> -#
> -# Note: This example has been shortened as the real response is too long.
> -#
> -##
> -{ 'command': 'query-events', 'returns': ['EventInfo'] }
> -
>  ##
>  # @IOThreadInfo:
>  #
> @@ -764,23 +569,6 @@
>  ##
>  { 'command': 'query-pci', 'returns': ['PciInfo'] }
>  
> -##
> -# @quit:
> -#
> -# This command will cause the QEMU process to exit gracefully.  While every
> -# attempt is made to send the QMP response before terminating, this is not
> -# guaranteed.  When using this interface, a premature EOF would not be
> -# unexpected.
> -#
> -# Since: 0.14.0
> -#
> -# Example:
> -#
> -# -> { "execute": "quit" }
> -# <- { "return": {} }
> -##
> -{ 'command': 'quit' }
> -
>  ##
>  # @stop:
>  #
> diff --git a/qapi/monitor.json b/qapi/monitor.json
> new file mode 100644
> index 0000000000..a82a18da1a
> --- /dev/null
> +++ b/qapi/monitor.json
> @@ -0,0 +1,218 @@
> +# -*- Mode: Python -*-
> +#
> +
> +##
> +# = Monitor definitions (shared between system emulator and tools)
> +##
> +
> +##
> +# @qmp_capabilities:
> +#
> +# Enable QMP capabilities.
> +#
> +# Arguments:
> +#
> +# @enable:   An optional list of QMPCapability values to enable.  The
> +#            client must not enable any capability that is not
> +#            mentioned in the QMP greeting message.  If the field is not
> +#            provided, it means no QMP capabilities will be enabled.
> +#            (since 2.12)
> +#
> +# Example:
> +#
> +# -> { "execute": "qmp_capabilities",
> +#      "arguments": { "enable": [ "oob" ] } }
> +# <- { "return": {} }
> +#
> +# Notes: This command is valid exactly when first connecting: it must be
> +# issued before any other command will be accepted, and will fail once the
> +# monitor is accepting other commands. (see qemu docs/interop/qmp-spec.txt)
> +#
> +# The QMP client needs to explicitly enable QMP capabilities, otherwise
> +# all the QMP capabilities will be turned off by default.
> +#
> +# Since: 0.13
> +#
> +##
> +{ 'command': 'qmp_capabilities',
> +  'data': { '*enable': [ 'QMPCapability' ] },
> +  'allow-preconfig': true }
> +
> +##
> +# @QMPCapability:
> +#
> +# Enumeration of capabilities to be advertised during initial client
> +# connection, used for agreeing on particular QMP extension behaviors.
> +#
> +# @oob:   QMP ability to support out-of-band requests.
> +#         (Please refer to qmp-spec.txt for more information on OOB)
> +#
> +# Since: 2.12
> +#
> +##
> +{ 'enum': 'QMPCapability',
> +  'data': [ 'oob' ] }
> +
> +##
> +# @VersionTriple:
> +#
> +# A three-part version number.
> +#
> +# @major:  The major version number.
> +#
> +# @minor:  The minor version number.
> +#
> +# @micro:  The micro version number.
> +#
> +# Since: 2.4
> +##
> +{ 'struct': 'VersionTriple',
> +  'data': {'major': 'int', 'minor': 'int', 'micro': 'int'} }
> +
> +
> +##
> +# @VersionInfo:
> +#
> +# A description of QEMU's version.
> +#
> +# @qemu:        The version of QEMU.  By current convention, a micro
> +#               version of 50 signifies a development branch.  A micro version
> +#               greater than or equal to 90 signifies a release candidate for
> +#               the next minor version.  A micro version of less than 50
> +#               signifies a stable release.
> +#
> +# @package:     QEMU will always set this field to an empty string.  Downstream
> +#               versions of QEMU should set this to a non-empty string.  The
> +#               exact format depends on the downstream however it highly
> +#               recommended that a unique name is used.
> +#
> +# Since: 0.14.0
> +##
> +{ 'struct': 'VersionInfo',
> +  'data': {'qemu': 'VersionTriple', 'package': 'str'} }
> +
> +##
> +# @query-version:
> +#
> +# Returns the current version of QEMU.
> +#
> +# Returns:  A @VersionInfo object describing the current version of QEMU.
> +#
> +# Since: 0.14.0
> +#
> +# Example:
> +#
> +# -> { "execute": "query-version" }
> +# <- {
> +#       "return":{
> +#          "qemu":{
> +#             "major":0,
> +#             "minor":11,
> +#             "micro":5
> +#          },
> +#          "package":""
> +#       }
> +#    }
> +#
> +##
> +{ 'command': 'query-version', 'returns': 'VersionInfo',
> +  'allow-preconfig': true }

Borderline.  I guess you move query-version (along with VersionInfo,
VersionTriple) because qmp_greeting() uses it.

> +
> +##
> +# @CommandInfo:
> +#
> +# Information about a QMP command
> +#
> +# @name: The command name
> +#
> +# Since: 0.14.0
> +##
> +{ 'struct': 'CommandInfo', 'data': {'name': 'str'} }
> +
> +##
> +# @query-commands:
> +#
> +# Return a list of supported QMP commands by this server
> +#
> +# Returns: A list of @CommandInfo for all supported commands
> +#
> +# Since: 0.14.0
> +#
> +# Example:
> +#
> +# -> { "execute": "query-commands" }
> +# <- {
> +#      "return":[
> +#         {
> +#            "name":"query-balloon"
> +#         },
> +#         {
> +#            "name":"system_powerdown"
> +#         }
> +#      ]
> +#    }
> +#
> +# Note: This example has been shortened as the real response is too long.
> +#
> +##
> +{ 'command': 'query-commands', 'returns': ['CommandInfo'],
> +  'allow-preconfig': true }
> +
> +##
> +# @EventInfo:
> +#
> +# Information about a QMP event
> +#
> +# @name: The event name
> +#
> +# Since: 1.2.0
> +##
> +{ 'struct': 'EventInfo', 'data': {'name': 'str'} }
> +
> +##
> +# @query-events:
> +#
> +# Return information on QMP events.
> +#
> +# Returns: A list of @EventInfo.
> +#
> +# Since: 1.2.0
> +#
> +# Note: This command is deprecated, because its output doesn't reflect
> +# compile-time configuration.  Use query-qmp-schema instead.
> +#
> +# Example:
> +#
> +# -> { "execute": "query-events" }
> +# <- {
> +#      "return": [
> +#          {
> +#             "name":"SHUTDOWN"
> +#          },
> +#          {
> +#             "name":"RESET"
> +#          }
> +#       ]
> +#    }
> +#
> +# Note: This example has been shortened as the real response is too long.
> +#
> +##
> +{ 'command': 'query-events', 'returns': ['EventInfo'] }
> +
> +##
> +# @quit:
> +#
> +# This command will cause the QEMU process to exit gracefully.  While every
> +# attempt is made to send the QMP response before terminating, this is not
> +# guaranteed.  When using this interface, a premature EOF would not be
> +# unexpected.
> +#
> +# Since: 0.14.0
> +#
> +# Example:
> +#
> +# -> { "execute": "quit" }
> +# <- { "return": {} }
> +##
> +{ 'command': 'quit' }

Borderline.

Perhaps it's just a matter of naming.  monitor.json suggests it's about
the monitor.  It's in fact about QMP, plus whatever else any program
sporting a QMP monitor has.  Can we find a name for that?  Hmm...
control.json?

[...]
> diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
> index dd3f5e6f94..519b6f1a8e 100644
> --- a/qapi/Makefile.objs
> +++ b/qapi/Makefile.objs
> @@ -6,8 +6,8 @@ util-obj-y += qmp-event.o
>  util-obj-y += qapi-util.o
>  
>  QAPI_COMMON_MODULES = audio authz block-core block char common crypto
> -QAPI_COMMON_MODULES += dump error introspect job machine migration misc net
> -QAPI_COMMON_MODULES += qdev qom rdma rocker run-state sockets tpm
> +QAPI_COMMON_MODULES += dump error introspect job machine migration misc monitor

Long line.

> +QAPI_COMMON_MODULES += net qdev qom rdma rocker run-state sockets tpm
>  QAPI_COMMON_MODULES += trace transaction ui
>  QAPI_TARGET_MODULES = machine-target misc-target
>  QAPI_MODULES = $(QAPI_COMMON_MODULES) $(QAPI_TARGET_MODULES)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] qapi: Create module 'monitor'
  2019-12-18 19:20   ` Markus Armbruster
@ 2019-12-19  8:25     ` Kevin Wolf
  2019-12-19  8:28     ` Kevin Wolf
  1 sibling, 0 replies; 9+ messages in thread
From: Kevin Wolf @ 2019-12-19  8:25 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, qemu-block

Am 18.12.2019 um 20:20 hat Markus Armbruster geschrieben:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
> > misc.json contains definitions that are related to the system emulator,
> > so it can't be used for the storage daemon. This patch moves basic
> > functionality that is related to the monitor itself into a new
> > monitor.json, which could be used in tools as well.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>

> > +##
> > +# @query-version:
> > +#
> > +# Returns the current version of QEMU.
> > +#
> > +# Returns:  A @VersionInfo object describing the current version of QEMU.
> > +#
> > +# Since: 0.14.0
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "query-version" }
> > +# <- {
> > +#       "return":{
> > +#          "qemu":{
> > +#             "major":0,
> > +#             "minor":11,
> > +#             "micro":5
> > +#          },
> > +#          "package":""
> > +#       }
> > +#    }
> > +#
> > +##
> > +{ 'command': 'query-version', 'returns': 'VersionInfo',
> > +  'allow-preconfig': true }
> 
> Borderline.  I guess you move query-version (along with VersionInfo,
> VersionTriple) because qmp_greeting() uses it.

Also because if querying the version makes sense for QEMU, it probably
also makes sense for any other tool.

> > +##
> > +# @quit:
> > +#
> > +# This command will cause the QEMU process to exit gracefully.  While every
> > +# attempt is made to send the QMP response before terminating, this is not
> > +# guaranteed.  When using this interface, a premature EOF would not be
> > +# unexpected.
> > +#
> > +# Since: 0.14.0
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "quit" }
> > +# <- { "return": {} }
> > +##
> > +{ 'command': 'quit' }
> 
> Borderline.
> 
> Perhaps it's just a matter of naming.  monitor.json suggests it's about
> the monitor.  It's in fact about QMP, plus whatever else any program
> sporting a QMP monitor has.  Can we find a name for that?  Hmm...
> control.json?

As this is what should make sense for every tool implementing QMP, how
about something like base.json or core.json?

Kevin



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] qapi: Create module 'monitor'
  2019-12-18 19:20   ` Markus Armbruster
  2019-12-19  8:25     ` Kevin Wolf
@ 2019-12-19  8:28     ` Kevin Wolf
  1 sibling, 0 replies; 9+ messages in thread
From: Kevin Wolf @ 2019-12-19  8:28 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, qemu-block

Am 18.12.2019 um 20:20 hat Markus Armbruster geschrieben:
> > diff --git a/qapi/Makefile.objs b/qapi/Makefile.objs
> > index dd3f5e6f94..519b6f1a8e 100644
> > --- a/qapi/Makefile.objs
> > +++ b/qapi/Makefile.objs
> > @@ -6,8 +6,8 @@ util-obj-y += qmp-event.o
> >  util-obj-y += qapi-util.o
> >  
> >  QAPI_COMMON_MODULES = audio authz block-core block char common crypto
> > -QAPI_COMMON_MODULES += dump error introspect job machine migration misc net
> > -QAPI_COMMON_MODULES += qdev qom rdma rocker run-state sockets tpm
> > +QAPI_COMMON_MODULES += dump error introspect job machine migration misc monitor
> 
> Long line.

I went to fix this, but it's only 79 characters, so I think this is a
false alarm.

Kevin



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-12-19  8:28 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-18 16:19 [PATCH 0/4] monitor: Refactoring in preparation for qemu-storage-daemon Kevin Wolf
2019-12-18 16:19 ` [PATCH 1/4] monitor: Move monitor option parsing to monitor/monitor.c Kevin Wolf
2019-12-18 19:07   ` Markus Armbruster
2019-12-18 16:19 ` [PATCH 2/4] qapi: Create module 'monitor' Kevin Wolf
2019-12-18 19:20   ` Markus Armbruster
2019-12-19  8:25     ` Kevin Wolf
2019-12-19  8:28     ` Kevin Wolf
2019-12-18 16:19 ` [PATCH 3/4] monitor: Create monitor/qmp-cmds-monitor.c Kevin Wolf
2019-12-18 16:19 ` [PATCH 4/4] monitor: Move qmp_query_qmp_schema to qmp-cmds-monitor.c Kevin Wolf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).