Re: [Qemu-block] [Qemu-devel] [PATCH v2 04/11] monitor: Create MonitorHMP with readline state
Am 12.06.2019 um 16:08 hat Markus Armbruster geschrieben:
> Kevin Wolf writes:
>
> > Am 12.06.2019 um 11:07 hat Markus Armbruster geschrieben:
> >> Cc: Peter for a monitor I/O thread question.
> >>
> >> Kevin Wolf writes:
> >>
> >> > The ReadLineState in Monitor is only used for HMP monitors. Create
> >> > MonitorHMP and move it there.
> >> >
> >> > Signed-off-by: Kevin Wolf
> >> > Reviewed-by: Dr. David Alan Gilbert
> >
> >> > @@ -218,6 +210,17 @@ struct Monitor {
> >> > int mux_out;
> >> > };
> >> >
> >> > +struct MonitorHMP {
> >> > +Monitor common;
> >> > +/*
> >> > + * State used only in the thread "owning" the monitor.
> >> > + * If @use_io_thread, this is @mon_iothread.
> >> > + * Else, it's the main thread.
> >> > + * These members can be safely accessed without locks.
> >> > + */
> >> > +ReadLineState *rs;
> >> > +};
> >> > +
> >>
> >> Hmm.
> >>
> >> The monitor I/O thread code makes an effort not to restrict I/O thread
> >> use to QMP, even though we only use it there. Whether the code would
> >> actually work for HMP as well we don't know.
> >>
> >> Readline was similar until your PATCH 02: the code made an effort not to
> >> restrict it to HMP, even though we only use it there. Whether the code
> >> would actually work for QMP as well we don't know.
> >>
> >> Should we stop pretending and hard-code "I/O thread only for QMP"?
> >>
> >> If yes, the comment above gets simplified by the patch that hard-codes
> >> "I/O thread only for QMP".
> >>
> >> If no, we should perhaps point out that we currently don't use an I/O
> >> thread with HMP. The comment above seems like a good place for that.
> >>
> >> Perhaps restricting readline to HMP should be a separate patch before
> >> PATCH 02.
> >
> > Yes, possibly iothreads could be restricted to QMP. It doesn't help me
> > in splitting the monitor in any way, though, so I don't see it within
> > the scope of this series.
>
> That's okay.
>
> Would you mind pointing out we don't actually use an I/O thread with HMP
> in the comment?
I do mind in a way (git doesn't really cope well with changing things in
the first patches of this series while the later patches move them to
different files - renaming mon_cmds resulted in a big mess and now I'm
kind of fed up with this kind of merge conflicts), but I'll do it
anyway. As long as it's only one line, it shouldn't be that hard to make
sure that it still exists at the end of the series...
> >> > @@ -748,12 +754,13 @@ char *qmp_human_monitor_command(const char
> >> > *command_line, bool has_cpu_index,
> >> > int64_t cpu_index, Error **errp)
> >> > {
> >> > char *output = NULL;
> >> > -Monitor *old_mon, hmp;
> >> > +Monitor *old_mon;
> >> > +MonitorHMP hmp = {};
> >>
> >> Any particular reason for adding the initializer?
> >
> > Yes:
> >
> >> >
> >> > -monitor_data_init(, 0, true, false);
> >> > +monitor_data_init(, 0, true, false);
> >
> > monitor_data_init() does a memset(), but only on hmp.common, so the
> > fields outside of hmp.common would remain uniniitialised. Specifically,
> > hmp.rs wouldn't be initialised to NULL and attempting to free it in the
> > end would crash.
>
> I see.
>
> Drop the superfluous memset() in monitor_data_init() then.
Hm, yes, all callers already initialise the memory now, so that can be
done.
Kevin
Re: [Qemu-block] [Qemu-devel] [PATCH v2 04/11] monitor: Create MonitorHMP with readline state
* Markus Armbruster (arm...@redhat.com) wrote:
> Kevin Wolf writes:
>
> > Am 12.06.2019 um 11:07 hat Markus Armbruster geschrieben:
> >> Cc: Peter for a monitor I/O thread question.
> >>
> >> Kevin Wolf writes:
> >>
> >> > The ReadLineState in Monitor is only used for HMP monitors. Create
> >> > MonitorHMP and move it there.
> >> >
> >> > Signed-off-by: Kevin Wolf
> >> > Reviewed-by: Dr. David Alan Gilbert
> >
> >> > @@ -218,6 +210,17 @@ struct Monitor {
> >> > int mux_out;
> >> > };
> >> >
> >> > +struct MonitorHMP {
> >> > +Monitor common;
> >> > +/*
> >> > + * State used only in the thread "owning" the monitor.
> >> > + * If @use_io_thread, this is @mon_iothread.
> >> > + * Else, it's the main thread.
> >> > + * These members can be safely accessed without locks.
> >> > + */
> >> > +ReadLineState *rs;
> >> > +};
> >> > +
> >>
> >> Hmm.
> >>
> >> The monitor I/O thread code makes an effort not to restrict I/O thread
> >> use to QMP, even though we only use it there. Whether the code would
> >> actually work for HMP as well we don't know.
> >>
> >> Readline was similar until your PATCH 02: the code made an effort not to
> >> restrict it to HMP, even though we only use it there. Whether the code
> >> would actually work for QMP as well we don't know.
> >>
> >> Should we stop pretending and hard-code "I/O thread only for QMP"?
> >>
> >> If yes, the comment above gets simplified by the patch that hard-codes
> >> "I/O thread only for QMP".
> >>
> >> If no, we should perhaps point out that we currently don't use an I/O
> >> thread with HMP. The comment above seems like a good place for that.
> >>
> >> Perhaps restricting readline to HMP should be a separate patch before
> >> PATCH 02.
> >
> > Yes, possibly iothreads could be restricted to QMP. It doesn't help me
> > in splitting the monitor in any way, though, so I don't see it within
> > the scope of this series.
>
> That's okay.
>
> Would you mind pointing out we don't actually use an I/O thread with HMP
> in the comment?
>
> > Keeping readline around for QMP, on the other hand, would probably have
> > been harder than making the restriction.
> >
> > As for splitting patch 2, I don't think that reorganising a patch that
> > already does its job and already received review is the most productive
> > thing we could do, but if you insist on a separate patch, I can do that.
>
> No, I don't insist.
>
> >> > @@ -748,12 +754,13 @@ char *qmp_human_monitor_command(const char
> >> > *command_line, bool has_cpu_index,
> >> > int64_t cpu_index, Error **errp)
> >> > {
> >> > char *output = NULL;
> >> > -Monitor *old_mon, hmp;
> >> > +Monitor *old_mon;
> >> > +MonitorHMP hmp = {};
> >>
> >> Any particular reason for adding the initializer?
> >
> > Yes:
> >
> >> >
> >> > -monitor_data_init(, 0, true, false);
> >> > +monitor_data_init(, 0, true, false);
> >
> > monitor_data_init() does a memset(), but only on hmp.common, so the
> > fields outside of hmp.common would remain uniniitialised. Specifically,
> > hmp.rs wouldn't be initialised to NULL and attempting to free it in the
> > end would crash.
>
> I see.
>
> Drop the superfluous memset() in monitor_data_init() then.
>
> >> > old_mon = cur_mon;
> >> > -cur_mon =
> >> > +cur_mon =
> >> >
> >> > if (has_cpu_index) {
> >> > int ret = monitor_set_cpu(cpu_index);
> >
> >> > @@ -1341,16 +1348,19 @@ static void hmp_info_sync_profile(Monitor *mon,
> >> > const QDict *qdict)
> >> >
> >> > static void hmp_info_history(Monitor *mon, const QDict *qdict)
> >> > {
> >> > +MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
> >>
> >> Unchecked conversion. Tolerable, I think, since HMP command handlers
> >> generally don't get invoked manually, unlike QMP command handlers.
> >
> > I would like to see all HMP command handlers take MonitorHMP* instead of
> > Monitor*, but that would be a big ugly patch touching everything that
> > isn't really needed for the goal of this series, so I didn't include it.
>
> I consider the MonitorHMP job incomplete without it. But it's Dave's
> turf.
I'd rather see stuff move forward and then fix that later when someone
has the time.
Dave
> > If you consider it valuable to get rid of this container_of(), that's
> > probably the follow-up you could do.
>
> My recent qemu-common.h pull request temporarily cooled my enthusiasm
> for big, ugly patches touching everything...
>
> >> > @@ -4460,6 +4474,7 @@ static void monitor_qmp_event(void *opaque, int
> >> > event)
> >> > static void monitor_event(void *opaque, int event)
> >> > {
> >> > Monitor *mon = opaque;
> >> > +MonitorHMP *hmp_mon = container_of(cur_mon, MonitorHMP, common);
> >>
> >> Any particular reason for changing from @opaque to @cur_mon?
> >
> > Probably a copy & paste error, thanks for catching it! I'll fix it.
> >
> >> > @@ -4662,11 +4679,11 @@
Re: [Qemu-block] [Qemu-devel] [PATCH v2 04/11] monitor: Create MonitorHMP with readline state
Kevin Wolf writes:
> Am 12.06.2019 um 11:07 hat Markus Armbruster geschrieben:
>> Cc: Peter for a monitor I/O thread question.
>>
>> Kevin Wolf writes:
>>
>> > The ReadLineState in Monitor is only used for HMP monitors. Create
>> > MonitorHMP and move it there.
>> >
>> > Signed-off-by: Kevin Wolf
>> > Reviewed-by: Dr. David Alan Gilbert
>
>> > @@ -218,6 +210,17 @@ struct Monitor {
>> > int mux_out;
>> > };
>> >
>> > +struct MonitorHMP {
>> > +Monitor common;
>> > +/*
>> > + * State used only in the thread "owning" the monitor.
>> > + * If @use_io_thread, this is @mon_iothread.
>> > + * Else, it's the main thread.
>> > + * These members can be safely accessed without locks.
>> > + */
>> > +ReadLineState *rs;
>> > +};
>> > +
>>
>> Hmm.
>>
>> The monitor I/O thread code makes an effort not to restrict I/O thread
>> use to QMP, even though we only use it there. Whether the code would
>> actually work for HMP as well we don't know.
>>
>> Readline was similar until your PATCH 02: the code made an effort not to
>> restrict it to HMP, even though we only use it there. Whether the code
>> would actually work for QMP as well we don't know.
>>
>> Should we stop pretending and hard-code "I/O thread only for QMP"?
>>
>> If yes, the comment above gets simplified by the patch that hard-codes
>> "I/O thread only for QMP".
>>
>> If no, we should perhaps point out that we currently don't use an I/O
>> thread with HMP. The comment above seems like a good place for that.
>>
>> Perhaps restricting readline to HMP should be a separate patch before
>> PATCH 02.
>
> Yes, possibly iothreads could be restricted to QMP. It doesn't help me
> in splitting the monitor in any way, though, so I don't see it within
> the scope of this series.
That's okay.
Would you mind pointing out we don't actually use an I/O thread with HMP
in the comment?
> Keeping readline around for QMP, on the other hand, would probably have
> been harder than making the restriction.
>
> As for splitting patch 2, I don't think that reorganising a patch that
> already does its job and already received review is the most productive
> thing we could do, but if you insist on a separate patch, I can do that.
No, I don't insist.
>> > @@ -748,12 +754,13 @@ char *qmp_human_monitor_command(const char
>> > *command_line, bool has_cpu_index,
>> > int64_t cpu_index, Error **errp)
>> > {
>> > char *output = NULL;
>> > -Monitor *old_mon, hmp;
>> > +Monitor *old_mon;
>> > +MonitorHMP hmp = {};
>>
>> Any particular reason for adding the initializer?
>
> Yes:
>
>> >
>> > -monitor_data_init(, 0, true, false);
>> > +monitor_data_init(, 0, true, false);
>
> monitor_data_init() does a memset(), but only on hmp.common, so the
> fields outside of hmp.common would remain uniniitialised. Specifically,
> hmp.rs wouldn't be initialised to NULL and attempting to free it in the
> end would crash.
I see.
Drop the superfluous memset() in monitor_data_init() then.
>> > old_mon = cur_mon;
>> > -cur_mon =
>> > +cur_mon =
>> >
>> > if (has_cpu_index) {
>> > int ret = monitor_set_cpu(cpu_index);
>
>> > @@ -1341,16 +1348,19 @@ static void hmp_info_sync_profile(Monitor *mon,
>> > const QDict *qdict)
>> >
>> > static void hmp_info_history(Monitor *mon, const QDict *qdict)
>> > {
>> > +MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
>>
>> Unchecked conversion. Tolerable, I think, since HMP command handlers
>> generally don't get invoked manually, unlike QMP command handlers.
>
> I would like to see all HMP command handlers take MonitorHMP* instead of
> Monitor*, but that would be a big ugly patch touching everything that
> isn't really needed for the goal of this series, so I didn't include it.
I consider the MonitorHMP job incomplete without it. But it's Dave's
turf.
> If you consider it valuable to get rid of this container_of(), that's
> probably the follow-up you could do.
My recent qemu-common.h pull request temporarily cooled my enthusiasm
for big, ugly patches touching everything...
>> > @@ -4460,6 +4474,7 @@ static void monitor_qmp_event(void *opaque, int
>> > event)
>> > static void monitor_event(void *opaque, int event)
>> > {
>> > Monitor *mon = opaque;
>> > +MonitorHMP *hmp_mon = container_of(cur_mon, MonitorHMP, common);
>>
>> Any particular reason for changing from @opaque to @cur_mon?
>
> Probably a copy & paste error, thanks for catching it! I'll fix it.
>
>> > @@ -4662,11 +4679,11 @@ static void monitor_init_qmp(Chardev *chr, int
>> > flags)
>> >
>> > static void monitor_init_hmp(Chardev *chr, int flags)
>> > {
>> > -Monitor *mon = g_malloc(sizeof(*mon));
>> > +MonitorHMP *mon = g_malloc0(sizeof(*mon));
>>
>> Any particular reason for changing to g_malloc0()?
>>
>> You hid the same change for monitor_init_qmp() in PATCH 03, where I
>> missed it until now.
>
> As above,
Re: [Qemu-block] [Qemu-devel] [PATCH v2 04/11] monitor: Create MonitorHMP with readline state
Am 12.06.2019 um 11:07 hat Markus Armbruster geschrieben:
> Cc: Peter for a monitor I/O thread question.
>
> Kevin Wolf writes:
>
> > The ReadLineState in Monitor is only used for HMP monitors. Create
> > MonitorHMP and move it there.
> >
> > Signed-off-by: Kevin Wolf
> > Reviewed-by: Dr. David Alan Gilbert
> > @@ -218,6 +210,17 @@ struct Monitor {
> > int mux_out;
> > };
> >
> > +struct MonitorHMP {
> > +Monitor common;
> > +/*
> > + * State used only in the thread "owning" the monitor.
> > + * If @use_io_thread, this is @mon_iothread.
> > + * Else, it's the main thread.
> > + * These members can be safely accessed without locks.
> > + */
> > +ReadLineState *rs;
> > +};
> > +
>
> Hmm.
>
> The monitor I/O thread code makes an effort not to restrict I/O thread
> use to QMP, even though we only use it there. Whether the code would
> actually work for HMP as well we don't know.
>
> Readline was similar until your PATCH 02: the code made an effort not to
> restrict it to HMP, even though we only use it there. Whether the code
> would actually work for QMP as well we don't know.
>
> Should we stop pretending and hard-code "I/O thread only for QMP"?
>
> If yes, the comment above gets simplified by the patch that hard-codes
> "I/O thread only for QMP".
>
> If no, we should perhaps point out that we currently don't use an I/O
> thread with HMP. The comment above seems like a good place for that.
>
> Perhaps restricting readline to HMP should be a separate patch before
> PATCH 02.
Yes, possibly iothreads could be restricted to QMP. It doesn't help me
in splitting the monitor in any way, though, so I don't see it within
the scope of this series.
Keeping readline around for QMP, on the other hand, would probably have
been harder than making the restriction.
As for splitting patch 2, I don't think that reorganising a patch that
already does its job and already received review is the most productive
thing we could do, but if you insist on a separate patch, I can do that.
> > @@ -748,12 +754,13 @@ char *qmp_human_monitor_command(const char
> > *command_line, bool has_cpu_index,
> > int64_t cpu_index, Error **errp)
> > {
> > char *output = NULL;
> > -Monitor *old_mon, hmp;
> > +Monitor *old_mon;
> > +MonitorHMP hmp = {};
>
> Any particular reason for adding the initializer?
Yes:
> >
> > -monitor_data_init(, 0, true, false);
> > +monitor_data_init(, 0, true, false);
monitor_data_init() does a memset(), but only on hmp.common, so the
fields outside of hmp.common would remain uniniitialised. Specifically,
hmp.rs wouldn't be initialised to NULL and attempting to free it in the
end would crash.
> > old_mon = cur_mon;
> > -cur_mon =
> > +cur_mon =
> >
> > if (has_cpu_index) {
> > int ret = monitor_set_cpu(cpu_index);
> > @@ -1341,16 +1348,19 @@ static void hmp_info_sync_profile(Monitor *mon,
> > const QDict *qdict)
> >
> > static void hmp_info_history(Monitor *mon, const QDict *qdict)
> > {
> > +MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
>
> Unchecked conversion. Tolerable, I think, since HMP command handlers
> generally don't get invoked manually, unlike QMP command handlers.
I would like to see all HMP command handlers take MonitorHMP* instead of
Monitor*, but that would be a big ugly patch touching everything that
isn't really needed for the goal of this series, so I didn't include it.
If you consider it valuable to get rid of this container_of(), that's
probably the follow-up you could do.
> > @@ -4460,6 +4474,7 @@ static void monitor_qmp_event(void *opaque, int event)
> > static void monitor_event(void *opaque, int event)
> > {
> > Monitor *mon = opaque;
> > +MonitorHMP *hmp_mon = container_of(cur_mon, MonitorHMP, common);
>
> Any particular reason for changing from @opaque to @cur_mon?
Probably a copy & paste error, thanks for catching it! I'll fix it.
> > @@ -4662,11 +4679,11 @@ static void monitor_init_qmp(Chardev *chr, int
> > flags)
> >
> > static void monitor_init_hmp(Chardev *chr, int flags)
> > {
> > -Monitor *mon = g_malloc(sizeof(*mon));
> > +MonitorHMP *mon = g_malloc0(sizeof(*mon));
>
> Any particular reason for changing to g_malloc0()?
>
> You hid the same change for monitor_init_qmp() in PATCH 03, where I
> missed it until now.
As above, initialising the fields outside mon->common.
Kevin
Re: [Qemu-block] [Qemu-devel] [PATCH v2 04/11] monitor: Create MonitorHMP with readline state
On Wed, Jun 12, 2019 at 11:07:01AM +0200, Markus Armbruster wrote:
[...]
> > +struct MonitorHMP {
> > +Monitor common;
> > +/*
> > + * State used only in the thread "owning" the monitor.
> > + * If @use_io_thread, this is @mon_iothread.
> > + * Else, it's the main thread.
> > + * These members can be safely accessed without locks.
> > + */
> > +ReadLineState *rs;
> > +};
> > +
>
> Hmm.
>
> The monitor I/O thread code makes an effort not to restrict I/O thread
> use to QMP, even though we only use it there. Whether the code would
> actually work for HMP as well we don't know.
>
> Readline was similar until your PATCH 02: the code made an effort not to
> restrict it to HMP, even though we only use it there. Whether the code
> would actually work for QMP as well we don't know.
>
> Should we stop pretending and hard-code "I/O thread only for QMP"?
>
> If yes, the comment above gets simplified by the patch that hard-codes
> "I/O thread only for QMP".
>
> If no, we should perhaps point out that we currently don't use an I/O
> thread with HMP. The comment above seems like a good place for that.
Yes I agree on that if we're refactoring the comment then we can make
it more explicit here. For my own preference, I would prefer the
latter one, even we can have a bigger comment above MonitorHMP
mentioning that it's only used in main thread so no lock is needed for
all the HMP only structs (until someone wants to hammer on HMP again).
Thanks,
--
Peter Xu
Re: [Qemu-block] [Qemu-devel] [PATCH v2 04/11] monitor: Create MonitorHMP with readline state
Cc: Peter for a monitor I/O thread question.
Kevin Wolf writes:
> The ReadLineState in Monitor is only used for HMP monitors. Create
> MonitorHMP and move it there.
>
> Signed-off-by: Kevin Wolf
> Reviewed-by: Dr. David Alan Gilbert
> ---
> include/monitor/monitor.h | 5 +-
> hmp.c | 4 +-
> monitor.c | 127 +-
> 3 files changed, 78 insertions(+), 58 deletions(-)
>
> diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
> index 86656297f1..1ba354f811 100644
> --- a/include/monitor/monitor.h
> +++ b/include/monitor/monitor.h
> @@ -7,6 +7,7 @@
> #include "qemu/readline.h"
>
> extern __thread Monitor *cur_mon;
> +typedef struct MonitorHMP MonitorHMP;
>
> /* flags for monitor_init */
> /* 0x01 unused */
> @@ -35,8 +36,8 @@ void monitor_flush(Monitor *mon);
> int monitor_set_cpu(int cpu_index);
> int monitor_get_cpu_index(void);
>
> -void monitor_read_command(Monitor *mon, int show_prompt);
> -int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> +void monitor_read_command(MonitorHMP *mon, int show_prompt);
> +int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
>void *opaque);
>
> AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id,
> diff --git a/hmp.c b/hmp.c
> index be5e345c6f..99414cd39c 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -1943,6 +1943,8 @@ static void hmp_change_read_arg(void *opaque, const
> char *password,
>
> void hmp_change(Monitor *mon, const QDict *qdict)
> {
> +/* FIXME Make MonitorHMP public and use container_of */
> +MonitorHMP *hmp_mon = (MonitorHMP *) mon;
No space between (MonitorHMP *) and mon.
> const char *device = qdict_get_str(qdict, "device");
> const char *target = qdict_get_str(qdict, "target");
> const char *arg = qdict_get_try_str(qdict, "arg");
> @@ -1960,7 +1962,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
> if (strcmp(target, "passwd") == 0 ||
> strcmp(target, "password") == 0) {
> if (!arg) {
> -monitor_read_password(mon, hmp_change_read_arg, NULL);
> +monitor_read_password(hmp_mon, hmp_change_read_arg, NULL);
> return;
> }
> }
> diff --git a/monitor.c b/monitor.c
> index d18cf18393..f8730e4462 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -190,14 +190,6 @@ struct Monitor {
> bool skip_flush;
> bool use_io_thread;
>
> -/*
> - * State used only in the thread "owning" the monitor.
> - * If @use_io_thread, this is @mon_iothread.
> - * Else, it's the main thread.
> - * These members can be safely accessed without locks.
> - */
> -ReadLineState *rs;
> -
> gchar *mon_cpu_path;
> mon_cmd_t *cmd_table;
> QTAILQ_ENTRY(Monitor) entry;
> @@ -218,6 +210,17 @@ struct Monitor {
> int mux_out;
> };
>
> +struct MonitorHMP {
> +Monitor common;
> +/*
> + * State used only in the thread "owning" the monitor.
> + * If @use_io_thread, this is @mon_iothread.
> + * Else, it's the main thread.
> + * These members can be safely accessed without locks.
> + */
> +ReadLineState *rs;
> +};
> +
Hmm.
The monitor I/O thread code makes an effort not to restrict I/O thread
use to QMP, even though we only use it there. Whether the code would
actually work for HMP as well we don't know.
Readline was similar until your PATCH 02: the code made an effort not to
restrict it to HMP, even though we only use it there. Whether the code
would actually work for QMP as well we don't know.
Should we stop pretending and hard-code "I/O thread only for QMP"?
If yes, the comment above gets simplified by the patch that hard-codes
"I/O thread only for QMP".
If no, we should perhaps point out that we currently don't use an I/O
thread with HMP. The comment above seems like a good place for that.
Perhaps restricting readline to HMP should be a separate patch before
PATCH 02.
> typedef struct {
> Monitor common;
> JSONMessageParser parser;
> @@ -324,7 +327,7 @@ bool monitor_cur_is_qmp(void)
> return cur_mon && monitor_is_qmp(cur_mon);
> }
>
> -void monitor_read_command(Monitor *mon, int show_prompt)
> +void monitor_read_command(MonitorHMP *mon, int show_prompt)
> {
> if (!mon->rs)
> return;
> @@ -334,7 +337,7 @@ void monitor_read_command(Monitor *mon, int show_prompt)
> readline_show_prompt(mon->rs);
> }
>
> -int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
> +int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func,
>void *opaque)
> {
> if (mon->rs) {
> @@ -342,7 +345,8 @@ int monitor_read_password(Monitor *mon, ReadLineFunc
> *readline_func,
> /* prompt is printed on return from the command handler */
> return 0;
> } else {
> -
