[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH v3 13/15] monitor: Split Monitor.fl
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH v3 13/15] monitor: Split Monitor.flags into separate bools |
Date: |
Fri, 14 Jun 2019 10:48:06 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) |
Kevin Wolf <address@hidden> 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 <address@hidden>
> ---
> 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 <address@hidden>
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 09/15] monitor: Create monitor-internal.h with common definitions, (continued)
- [Qemu-block] [PATCH v3 08/15] monitor: Move {hmp, qmp}.c to monitor/{hmp, qmp}-cmds.c, Kevin Wolf, 2019/06/13
- [Qemu-block] [PATCH v3 04/15] monitor: Create MonitorHMP with readline state, Kevin Wolf, 2019/06/13
- [Qemu-block] [PATCH v3 10/15] monitor: Split out monitor/qmp.c, Kevin Wolf, 2019/06/13
- [Qemu-block] [PATCH v3 14/15] monitor: Replace monitor_init() with monitor_init_{hmp, qmp}(), Kevin Wolf, 2019/06/13
- [Qemu-block] [PATCH v3 13/15] monitor: Split Monitor.flags into separate bools, Kevin Wolf, 2019/06/13
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 13/15] monitor: Split Monitor.flags into separate bools,
Markus Armbruster <=
- [Qemu-block] [PATCH v3 12/15] monitor: Split out monitor/monitor.c, Kevin Wolf, 2019/06/13
- [Qemu-block] [PATCH v3 15/15] vl: Deprecate -mon pretty=... for HMP monitors, Kevin Wolf, 2019/06/13
- [Qemu-block] [PATCH v3 11/15] monitor: Split out monitor/hmp.c, Kevin Wolf, 2019/06/13
- Re: [Qemu-block] [Qemu-devel] [PATCH v3 00/15] monitor: Split monitor.c in core/HMP/QMP/misc, no-reply, 2019/06/13