On Wed, 10 Nov 2010 14:20:12 +0100
Markus Armbruster <arm...@redhat.com> wrote:
> Luiz Capitulino <lcapitul...@redhat.com> writes:
>
> > This command allows QMP clients to execute HMP commands.
> >
> > Please, check the documentation added to the qmp-commands.hx file
> > for additional details about the interface and its limitations.
> >
> > Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
> > ---
> > monitor.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> > qmp-commands.hx | 45 +++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 87 insertions(+), 0 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index 61607c5..4fe7f5d 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -490,6 +490,48 @@ static int do_qmp_capabilities(Monitor *mon, const
> > QDict *params,
> > return 0;
> > }
> >
> > +static int mon_set_cpu(int cpu_index);
> > +static void handle_user_command(Monitor *mon, const char *cmdline);
> > +
> > +static int do_hmp_passthrough(Monitor *mon, const QDict *params,
> > + QObject **ret_data)
> > +{
> > + int ret = 0;
> > + QString *qs;
> > + Monitor *old_mon, hmp;
> > + CharDriverState bufchr;
> > +
> > + memset(&hmp, 0, sizeof(hmp));
> > + hmp.chr = &bufchr;
> > + qemu_chr_init_buffered(hmp.chr);
> > +
> > + old_mon = cur_mon;
> > + cur_mon = &hmp;
> > +
> > + if (qdict_haskey(params, "cpu-index")) {
> > + ret = mon_set_cpu(qdict_get_int(params, "cpu-index"));
> > + if (ret < 0) {
> > + goto out;
> > + }
> > + }
> > +
> > + handle_user_command(&hmp, qdict_get_str(params, "command-line"));
> > +
> > + qs = qemu_chr_buffered_to_qs(hmp.chr);
> > + if (qs) {
> > + *ret_data = QOBJECT(qs);
> > + }
> > +
> > +out:
> > + cur_mon = old_mon;
> > + qemu_chr_close_buffered(hmp.chr);
> > +
> > + if (ret < 0) {
> > + qerror_report(QERR_INVALID_PARAMETER_VALUE, "cpu-index","a CPU
> > number");
> > + }
>
> Emit the error right where ret becomes negative. Cleaner, and allows
> for other errors, should they become necessary.
>
> > + return ret;
> > +}
> > +
> > static int compare_cmd(const char *name, const char *list)
> > {
> > const char *p, *pstart;
> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index 793cf1c..b344096 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -761,6 +761,51 @@ Example:
> >
> > Note: This command must be issued before issuing any other command.
> >
> > +EQMP
> > +
> > + {
> > + .name = "hmp_passthrough",
> > + .args_type = "command-line:s,cpu-index:i?",
> > + .params = "",
> > + .help = "",
> > + .user_print = monitor_user_noop,
> > + .mhandler.cmd_new = do_hmp_passthrough,
> > + },
> > +
> > +SQMP
> > +hmp_passthrough
> > +---------------
> > +
> > +Execute a Human Monitor command.
> > +
> > +Arguments:
> > +
> > +- command-line: the command name and its arguments, just like the
> > + Human Monitor's shell (json-string)
> > +- cpu-index: select the CPU number to be used by commands which access CPU
> > + data, like 'info registers'. The Monitor selects CPU 0 if this
> > + argument is not provided (json-int, optional)
> > +
> > +Example:
> > +
> > +-> { "execute": "hmp_passthrough", "arguments": { "command-line": "info
> > kvm" } }
> > +<- { "return": "kvm support: enabled\r\n" }
> > +
> > +Notes:
> > +
> > +(1) The Human Monitor is NOT an stable interface, this means that command
> > + names, arguments and responses can change or be removed at ANY time.
> > + Applications that rely on long term stability guarantees should NOT
> > + use this command
> > +
> > +(2) Limitations:
> > +
> > + o This command is stateless, this means that commands that depend
> > + on state information (such as getfd) might not work
> > +
> > + o Commands that prompt the user for data (eg. 'cont' when the block
> > + device is encrypted) don't currently work
> > +
> > 3. Query Commands
> > =================
>
> In the real human monitor, cpu-index is state (Monitor member mon_cpu).
> For pass through, you shift that state into the client (argument
> cpu-index). Is there any other state that could need shifting? You
> mention getfd.
Surprisingly or not, this is a very important question for QMP itself.
Anthony has said that we should make it stateless, and I do think this
is good because it seems to simplify things considerably.
However, I haven't thought about how to make things like getfd stateless.
> A possible alternative would be to keep the state around instead of
> creating a new one for each pass through command. Dunno, haven't
> thought that through. Maybe you did?
It's above ;) becoming stateless seems to be the way for QMP.
> Regardless, this is clearly better than nothing. We can work on
> limitations later, should they turn out to be bothersome.
Agreed.
