diff mbox series

[1/4] monitor: Move monitor option parsing to monitor/monitor.c

Message ID 20191218161952.10202-2-kwolf@redhat.com (mailing list archive)
State New, archived
Headers show
Series monitor: Refactoring in preparation for qemu-storage-daemon | expand

Commit Message

Kevin Wolf Dec. 18, 2019, 4:19 p.m. UTC 
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(-)

Comments

Markus Armbruster Dec. 18, 2019, 7:07 p.m. UTC | #1 
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>
diff mbox series

Patch

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)