diff mbox series

[v3,13/15] monitor: Split Monitor.flags into separate bools

Message ID 20190613153405.24769-14-kwolf@redhat.com (mailing list archive)
State New, archived
Headers show
Series monitor: Split monitor.c in core/HMP/QMP/misc | expand

Commit Message

Kevin Wolf June 13, 2019, 3:34 p.m. UTC 
Monitor.flags contains three different flags: One to distinguish HMP
from QMP; one specific to HMP (MONITOR_USE_READLINE) that is ignored
with QMP; and another one specific to QMP (MONITOR_USE_PRETTY) that is
ignored with HMP.

Split the flags field into three bools and move them to the right
subclass. Flags are still in use for the monitor_init() interface.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 monitor/monitor-internal.h |  8 +++++---
 monitor/hmp.c              |  6 +++---
 monitor/misc.c             |  2 +-
 monitor/monitor.c          | 14 +++++++++-----
 monitor/qmp.c              |  7 ++++---
 5 files changed, 22 insertions(+), 15 deletions(-)

Comments

Markus Armbruster June 14, 2019, 8:48 a.m. UTC | #1 
Kevin Wolf <kwolf@redhat.com> writes:

> Monitor.flags contains three different flags: One to distinguish HMP
> from QMP; one specific to HMP (MONITOR_USE_READLINE) that is ignored
> with QMP; and another one specific to QMP (MONITOR_USE_PRETTY) that is
> ignored with HMP.
>
> Split the flags field into three bools and move them to the right
> subclass. Flags are still in use for the monitor_init() interface.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  monitor/monitor-internal.h |  8 +++++---
>  monitor/hmp.c              |  6 +++---
>  monitor/misc.c             |  2 +-
>  monitor/monitor.c          | 14 +++++++++-----
>  monitor/qmp.c              |  7 ++++---
>  5 files changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
> index 30679d522e..260725e51b 100644
> --- a/monitor/monitor-internal.h
> +++ b/monitor/monitor-internal.h
> @@ -90,8 +90,8 @@ typedef struct HMPCommand {
>  struct Monitor {
>      CharBackend chr;
>      int reset_seen;
> -    int flags;
>      int suspend_cnt;            /* Needs to be accessed atomically */
> +    bool is_qmp;
>      bool skip_flush;
>      bool use_io_thread;
>  
> @@ -116,6 +116,7 @@ struct Monitor {
>  
>  struct MonitorHMP {
>      Monitor common;
> +    bool use_readline;
>      /*
>       * State used only in the thread "owning" the monitor.
>       * If @use_io_thread, this is @mon_iothread. (This does not actually happen
> @@ -129,6 +130,7 @@ struct MonitorHMP {
>  typedef struct {
>      Monitor common;
>      JSONMessageParser parser;
> +    bool pretty;
>      /*
>       * When a client connects, we're in capabilities negotiation mode.
>       * @commands is &qmp_cap_negotiation_commands then.  When command
> @@ -152,7 +154,7 @@ typedef struct {
>   */
>  static inline bool monitor_is_qmp(const Monitor *mon)
>  {
> -    return (mon->flags & MONITOR_USE_CONTROL);
> +    return mon->is_qmp;
>  }

The function was marginal before, and it's pointless now.  Let's kill it
in a follow-up patch.  No need to do it in this series.

>  
>  typedef QTAILQ_HEAD(MonitorList, Monitor) MonitorList;
> @@ -169,7 +171,7 @@ void monitor_init_qmp(Chardev *chr, int flags);
>  void monitor_init_hmp(Chardev *chr, int flags);
>  
>  int monitor_puts(Monitor *mon, const char *str);
> -void monitor_data_init(Monitor *mon, int flags, bool skip_flush,
> +void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
>                         bool use_io_thread);
>  void monitor_data_destroy(Monitor *mon);
>  int monitor_can_read(void *opaque);
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 3621b195ed..5ba8b6e8d5 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -1396,12 +1396,12 @@ static void monitor_readline_flush(void *opaque)
>  void monitor_init_hmp(Chardev *chr, int flags)
>  {
>      MonitorHMP *mon = g_malloc0(sizeof(*mon));
> -    bool use_readline = flags & MONITOR_USE_READLINE;
>  
> -    monitor_data_init(&mon->common, flags, false, false);
> +    monitor_data_init(&mon->common, false, false, false);
>      qemu_chr_fe_init(&mon->common.chr, chr, &error_abort);
>  
> -    if (use_readline) {
> +    mon->use_readline = flags & MONITOR_USE_READLINE;
> +    if (mon->use_readline) {
>          mon->rs = readline_init(monitor_readline_printf,
>                                  monitor_readline_flush,
>                                  mon,
> diff --git a/monitor/misc.c b/monitor/misc.c
> index dddbddb21f..10c056394e 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -134,7 +134,7 @@ char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
>      Monitor *old_mon;
>      MonitorHMP hmp = {};
>  
> -    monitor_data_init(&hmp.common, 0, true, false);
> +    monitor_data_init(&hmp.common, false, true, false);
>  
>      old_mon = cur_mon;
>      cur_mon = &hmp.common;
> diff --git a/monitor/monitor.c b/monitor/monitor.c
> index 6802b8e491..7325e4362b 100644
> --- a/monitor/monitor.c
> +++ b/monitor/monitor.c
> @@ -80,14 +80,18 @@ bool monitor_cur_is_qmp(void)
>   * Note: not all HMP monitors use readline, e.g., gdbserver has a
>   * non-interactive HMP monitor, so readline is not used there.
>   */
> -static inline bool monitor_uses_readline(const Monitor *mon)
> +static inline bool monitor_uses_readline(const MonitorHMP *mon)
>  {
> -    return mon->flags & MONITOR_USE_READLINE;
> +    return mon->use_readline;
>  }

Another one to kill.

>  
>  static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)
>  {
> -    return !monitor_is_qmp(mon) && !monitor_uses_readline(mon);
> +    if (monitor_is_qmp(mon)) {
> +        return false;
> +    }
> +
> +    return !monitor_uses_readline(container_of(mon, MonitorHMP, common));
>  }
>  

Not sure about this one.  We'll see.

>  static void monitor_flush_locked(Monitor *mon);
> @@ -523,17 +527,17 @@ static void monitor_iothread_init(void)
>      mon_iothread = iothread_create("mon_iothread", &error_abort);
>  }
>  
> -void monitor_data_init(Monitor *mon, int flags, bool skip_flush,
> +void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
>                         bool use_io_thread)
>  {
>      if (use_io_thread && !mon_iothread) {
>          monitor_iothread_init();
>      }
>      qemu_mutex_init(&mon->mon_lock);
> +    mon->is_qmp = is_qmp;
>      mon->outbuf = qstring_new();
>      mon->skip_flush = skip_flush;
>      mon->use_io_thread = use_io_thread;
> -    mon->flags = flags;
>  }
>  
>  void monitor_data_destroy(Monitor *mon)
> diff --git a/monitor/qmp.c b/monitor/qmp.c
> index 31fbcd59f7..3abc718ca3 100644
> --- a/monitor/qmp.c
> +++ b/monitor/qmp.c
> @@ -86,8 +86,7 @@ void qmp_send_response(MonitorQMP *mon, const QDict *rsp)
>      const QObject *data = QOBJECT(rsp);
>      QString *json;
>  
> -    json = mon->common.flags & MONITOR_USE_PRETTY ?
> -           qobject_to_json_pretty(data) : qobject_to_json(data);
> +    json = mon->pretty ? qobject_to_json_pretty(data) : qobject_to_json(data);
>      assert(json != NULL);
>  
>      qstring_append_chr(json, '\n');
> @@ -372,9 +371,11 @@ void monitor_init_qmp(Chardev *chr, int flags)
>      assert(!(flags & MONITOR_USE_READLINE));
>  
>      /* Note: we run QMP monitor in I/O thread when @chr supports that */
> -    monitor_data_init(&mon->common, flags, false,
> +    monitor_data_init(&mon->common, true, false,
>                        qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
>  
> +    mon->pretty = flags & MONITOR_USE_PRETTY;
> +
>      qemu_mutex_init(&mon->qmp_queue_lock);
>      mon->qmp_requests = g_queue_new();

Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff mbox series

Patch

diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 30679d522e..260725e51b 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -90,8 +90,8 @@  typedef struct HMPCommand {
 struct Monitor {
     CharBackend chr;
     int reset_seen;
-    int flags;
     int suspend_cnt;            /* Needs to be accessed atomically */
+    bool is_qmp;
     bool skip_flush;
     bool use_io_thread;
 
@@ -116,6 +116,7 @@  struct Monitor {
 
 struct MonitorHMP {
     Monitor common;
+    bool use_readline;
     /*
      * State used only in the thread "owning" the monitor.
      * If @use_io_thread, this is @mon_iothread. (This does not actually happen
@@ -129,6 +130,7 @@  struct MonitorHMP {
 typedef struct {
     Monitor common;
     JSONMessageParser parser;
+    bool pretty;
     /*
      * When a client connects, we're in capabilities negotiation mode.
      * @commands is &qmp_cap_negotiation_commands then.  When command
@@ -152,7 +154,7 @@  typedef struct {
  */
 static inline bool monitor_is_qmp(const Monitor *mon)
 {
-    return (mon->flags & MONITOR_USE_CONTROL);
+    return mon->is_qmp;
 }
 
 typedef QTAILQ_HEAD(MonitorList, Monitor) MonitorList;
@@ -169,7 +171,7 @@  void monitor_init_qmp(Chardev *chr, int flags);
 void monitor_init_hmp(Chardev *chr, int flags);
 
 int monitor_puts(Monitor *mon, const char *str);
-void monitor_data_init(Monitor *mon, int flags, bool skip_flush,
+void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
                        bool use_io_thread);
 void monitor_data_destroy(Monitor *mon);
 int monitor_can_read(void *opaque);
diff --git a/monitor/hmp.c b/monitor/hmp.c
index 3621b195ed..5ba8b6e8d5 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -1396,12 +1396,12 @@  static void monitor_readline_flush(void *opaque)
 void monitor_init_hmp(Chardev *chr, int flags)
 {
     MonitorHMP *mon = g_malloc0(sizeof(*mon));
-    bool use_readline = flags & MONITOR_USE_READLINE;
 
-    monitor_data_init(&mon->common, flags, false, false);
+    monitor_data_init(&mon->common, false, false, false);
     qemu_chr_fe_init(&mon->common.chr, chr, &error_abort);
 
-    if (use_readline) {
+    mon->use_readline = flags & MONITOR_USE_READLINE;
+    if (mon->use_readline) {
         mon->rs = readline_init(monitor_readline_printf,
                                 monitor_readline_flush,
                                 mon,
diff --git a/monitor/misc.c b/monitor/misc.c
index dddbddb21f..10c056394e 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -134,7 +134,7 @@  char *qmp_human_monitor_command(const char *command_line, bool has_cpu_index,
     Monitor *old_mon;
     MonitorHMP hmp = {};
 
-    monitor_data_init(&hmp.common, 0, true, false);
+    monitor_data_init(&hmp.common, false, true, false);
 
     old_mon = cur_mon;
     cur_mon = &hmp.common;
diff --git a/monitor/monitor.c b/monitor/monitor.c
index 6802b8e491..7325e4362b 100644
--- a/monitor/monitor.c
+++ b/monitor/monitor.c
@@ -80,14 +80,18 @@  bool monitor_cur_is_qmp(void)
  * Note: not all HMP monitors use readline, e.g., gdbserver has a
  * non-interactive HMP monitor, so readline is not used there.
  */
-static inline bool monitor_uses_readline(const Monitor *mon)
+static inline bool monitor_uses_readline(const MonitorHMP *mon)
 {
-    return mon->flags & MONITOR_USE_READLINE;
+    return mon->use_readline;
 }
 
 static inline bool monitor_is_hmp_non_interactive(const Monitor *mon)
 {
-    return !monitor_is_qmp(mon) && !monitor_uses_readline(mon);
+    if (monitor_is_qmp(mon)) {
+        return false;
+    }
+
+    return !monitor_uses_readline(container_of(mon, MonitorHMP, common));
 }
 
 static void monitor_flush_locked(Monitor *mon);
@@ -523,17 +527,17 @@  static void monitor_iothread_init(void)
     mon_iothread = iothread_create("mon_iothread", &error_abort);
 }
 
-void monitor_data_init(Monitor *mon, int flags, bool skip_flush,
+void monitor_data_init(Monitor *mon, bool is_qmp, bool skip_flush,
                        bool use_io_thread)
 {
     if (use_io_thread && !mon_iothread) {
         monitor_iothread_init();
     }
     qemu_mutex_init(&mon->mon_lock);
+    mon->is_qmp = is_qmp;
     mon->outbuf = qstring_new();
     mon->skip_flush = skip_flush;
     mon->use_io_thread = use_io_thread;
-    mon->flags = flags;
 }
 
 void monitor_data_destroy(Monitor *mon)
diff --git a/monitor/qmp.c b/monitor/qmp.c
index 31fbcd59f7..3abc718ca3 100644
--- a/monitor/qmp.c
+++ b/monitor/qmp.c
@@ -86,8 +86,7 @@  void qmp_send_response(MonitorQMP *mon, const QDict *rsp)
     const QObject *data = QOBJECT(rsp);
     QString *json;
 
-    json = mon->common.flags & MONITOR_USE_PRETTY ?
-           qobject_to_json_pretty(data) : qobject_to_json(data);
+    json = mon->pretty ? qobject_to_json_pretty(data) : qobject_to_json(data);
     assert(json != NULL);
 
     qstring_append_chr(json, '\n');
@@ -372,9 +371,11 @@  void monitor_init_qmp(Chardev *chr, int flags)
     assert(!(flags & MONITOR_USE_READLINE));
 
     /* Note: we run QMP monitor in I/O thread when @chr supports that */
-    monitor_data_init(&mon->common, flags, false,
+    monitor_data_init(&mon->common, true, false,
                       qemu_chr_has_feature(chr, QEMU_CHAR_FEATURE_GCONTEXT));
 
+    mon->pretty = flags & MONITOR_USE_PRETTY;
+
     qemu_mutex_init(&mon->qmp_queue_lock);
     mon->qmp_requests = g_queue_new();