Message ID | 1288037204-27768-3-git-send-email-lcapitulino@redhat.com |
---|---|
State | New |
Headers | show |
Luiz Capitulino <lcapitulino@redhat.com> writes: > This command allows QMP clients to execute HMP commands, please > check its documentation in the hmp-commands.hx file for usage > information. > > Please, also note that not all HMP commands can be executed this > way, in special commands that: > > o need to store monitor related data (eg. getfd) Could you explain the problem here? > o read data from the user (eg. cont when the block device is > encrypted) The human monitor does I/O on a character device. Your human monitor captures output, and sends it back over QMP. Input could be sent over QMP, too. Just not interactively, as all of the input needs to be sent along with the command. What can't work is funky terminal stuff such as readline, but the human monitor already degrades gracefully when run on a non-terminal character device, doesn't it? > > TODO: Create a blacklist for those bad commands or just let > them fail? (assuming they won't blowup, of course) We need to make sure "bad" commands fail cleanly anyway, so things don't explode when we get the blacklist slightly wrong. > TODO: Maybe a command like 'cpu' requires a blacklist What's the problem with "cpu"? > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > monitor.c | 34 ++++++++++++++++++++++++++++++++++ > qmp-commands.hx | 32 ++++++++++++++++++++++++++++++++ > 2 files changed, 66 insertions(+), 0 deletions(-) > > diff --git a/monitor.c b/monitor.c > index 260cc02..a0df098 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -490,6 +490,40 @@ static int do_qmp_capabilities(Monitor *mon, const QDict *params, > return 0; > } > > +static void handle_user_command(Monitor *mon, const char *cmdline); > + > +static void hmp_call(Monitor *mon, const char *cmdline) > +{ > + Monitor *old_mon = cur_mon; > + > + cur_mon = mon; > + handle_user_command(mon, cmdline); > + cur_mon = old_mon; > +} Do you expect more users of this function? > + > +static int do_hmp_passthrough(Monitor *mon, const QDict *params, > + QObject **ret_data) > +{ > + QString *qs; > + Monitor hmp; > + CharDriverState bufchr; > + > + memset(&hmp, 0, sizeof(hmp)); > + hmp.chr = &bufchr; > + qemu_chr_init_buffered(hmp.chr); > + > + hmp_call(&hmp, qdict_get_str(params, "command-line")); > + > + qs = qemu_chr_buffered_to_qs(hmp.chr); > + if (qs) { > + *ret_data = QOBJECT(qs); > + } If the command produces no output, qemu_chr_buffered_to_qs() returns null (which I don't like, see there), and *ret_data remains untouched. Works for me. > + > + qemu_chr_close_buffered(hmp.chr); > + > + return 0; > +} > + > 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..29a6048 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -761,6 +761,38 @@ Example: > > Note: This command must be issued before issuing any other command. > > +EQMP > + > + { > + .name = "hmp_passthrough", > + .args_type = "command-line:s", > + .params = "", > + .help = "", > + .user_print = monitor_user_noop, > + .mhandler.cmd_new = do_hmp_passthrough, > + }, > + > +SQMP > +hmp_passthrough The temptation to call this "human_passthrough" would be well-nigh irresistible for me... can't resist cheap puns ;) > +--------------- > + > +Execute a Human Monitor command. > + > +Arguments: > + > +- command-line: the command name and its arguments, just like the > + Human Monitor's shell (json-string) > + > +Example: > + > +-> { "execute": "hmp_passthrough", "arguments": { "command-line": "info version" } } > +<- { "return": "0.13.50\r\n" } > + > +Note: The Human Monitor is NOT a 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. > + > 3. Query Commands > ================= I'm pleasantly surprised how self-contained and simple this feature turned out to be. Nice work!
On Wed, 10 Nov 2010 11:03:56 +0100 Markus Armbruster <armbru@redhat.com> wrote: > Luiz Capitulino <lcapitulino@redhat.com> writes: > > > This command allows QMP clients to execute HMP commands, please > > check its documentation in the hmp-commands.hx file for usage > > information. > > > > Please, also note that not all HMP commands can be executed this > > way, in special commands that: This changed a bit in v1, care to review it instead? I will answer your questions but skip the code comments. > > > > o need to store monitor related data (eg. getfd) > > Could you explain the problem here? IIUC, we store received file-descriptors in the Monitor struct, however the passthrough is totally stateless so we don't have where to store them. > > o read data from the user (eg. cont when the block device is > > encrypted) > > The human monitor does I/O on a character device. Your human monitor > captures output, and sends it back over QMP. Input could be sent over > QMP, too. Just not interactively, as all of the input needs to be sent > along with the command. > > What can't work is funky terminal stuff such as readline, but the human > monitor already degrades gracefully when run on a non-terminal character > device, doesn't it? Yes. We could make this kind of command work by adding an input buffer to the MemoryDriver, but I'm not sure if the additional complexity is worth it. > > TODO: Create a blacklist for those bad commands or just let > > them fail? (assuming they won't blowup, of course) > > We need to make sure "bad" commands fail cleanly anyway, so things don't > explode when we get the blacklist slightly wrong. Sure, as far as I could test it, no command will explode, if it happens it's a bug of course. > > TODO: Maybe a command like 'cpu' requires a blacklist > > What's the problem with "cpu"? Fixed in v1. > > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > > --- > > monitor.c | 34 ++++++++++++++++++++++++++++++++++ > > qmp-commands.hx | 32 ++++++++++++++++++++++++++++++++ > > 2 files changed, 66 insertions(+), 0 deletions(-) > > > > diff --git a/monitor.c b/monitor.c > > index 260cc02..a0df098 100644 > > --- a/monitor.c > > +++ b/monitor.c > > @@ -490,6 +490,40 @@ static int do_qmp_capabilities(Monitor *mon, const QDict *params, > > return 0; > > } > > > > +static void handle_user_command(Monitor *mon, const char *cmdline); > > + > > +static void hmp_call(Monitor *mon, const char *cmdline) > > +{ > > + Monitor *old_mon = cur_mon; > > + > > + cur_mon = mon; > > + handle_user_command(mon, cmdline); > > + cur_mon = old_mon; > > +} > > Do you expect more users of this function? > > > + > > +static int do_hmp_passthrough(Monitor *mon, const QDict *params, > > + QObject **ret_data) > > +{ > > + QString *qs; > > + Monitor hmp; > > + CharDriverState bufchr; > > + > > + memset(&hmp, 0, sizeof(hmp)); > > + hmp.chr = &bufchr; > > + qemu_chr_init_buffered(hmp.chr); > > + > > + hmp_call(&hmp, qdict_get_str(params, "command-line")); > > + > > + qs = qemu_chr_buffered_to_qs(hmp.chr); > > + if (qs) { > > + *ret_data = QOBJECT(qs); > > + } > > If the command produces no output, qemu_chr_buffered_to_qs() returns > null (which I don't like, see there), and *ret_data remains untouched. > Works for me. > > > + > > + qemu_chr_close_buffered(hmp.chr); > > + > > + return 0; > > +} > > + > > 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..29a6048 100644 > > --- a/qmp-commands.hx > > +++ b/qmp-commands.hx > > @@ -761,6 +761,38 @@ Example: > > > > Note: This command must be issued before issuing any other command. > > > > +EQMP > > + > > + { > > + .name = "hmp_passthrough", > > + .args_type = "command-line:s", > > + .params = "", > > + .help = "", > > + .user_print = monitor_user_noop, > > + .mhandler.cmd_new = do_hmp_passthrough, > > + }, > > + > > +SQMP > > +hmp_passthrough > > The temptation to call this "human_passthrough" would be well-nigh > irresistible for me... can't resist cheap puns ;) > > > +--------------- > > + > > +Execute a Human Monitor command. > > + > > +Arguments: > > + > > +- command-line: the command name and its arguments, just like the > > + Human Monitor's shell (json-string) > > + > > +Example: > > + > > +-> { "execute": "hmp_passthrough", "arguments": { "command-line": "info version" } } > > +<- { "return": "0.13.50\r\n" } > > + > > +Note: The Human Monitor is NOT a 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. > > + > > 3. Query Commands > > ================= > > I'm pleasantly surprised how self-contained and simple this feature > turned out to be. Nice work! >
diff --git a/monitor.c b/monitor.c index 260cc02..a0df098 100644 --- a/monitor.c +++ b/monitor.c @@ -490,6 +490,40 @@ static int do_qmp_capabilities(Monitor *mon, const QDict *params, return 0; } +static void handle_user_command(Monitor *mon, const char *cmdline); + +static void hmp_call(Monitor *mon, const char *cmdline) +{ + Monitor *old_mon = cur_mon; + + cur_mon = mon; + handle_user_command(mon, cmdline); + cur_mon = old_mon; +} + +static int do_hmp_passthrough(Monitor *mon, const QDict *params, + QObject **ret_data) +{ + QString *qs; + Monitor hmp; + CharDriverState bufchr; + + memset(&hmp, 0, sizeof(hmp)); + hmp.chr = &bufchr; + qemu_chr_init_buffered(hmp.chr); + + hmp_call(&hmp, qdict_get_str(params, "command-line")); + + qs = qemu_chr_buffered_to_qs(hmp.chr); + if (qs) { + *ret_data = QOBJECT(qs); + } + + qemu_chr_close_buffered(hmp.chr); + + return 0; +} + 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..29a6048 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -761,6 +761,38 @@ Example: Note: This command must be issued before issuing any other command. +EQMP + + { + .name = "hmp_passthrough", + .args_type = "command-line:s", + .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) + +Example: + +-> { "execute": "hmp_passthrough", "arguments": { "command-line": "info version" } } +<- { "return": "0.13.50\r\n" } + +Note: The Human Monitor is NOT a 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. + 3. Query Commands =================
This command allows QMP clients to execute HMP commands, please check its documentation in the hmp-commands.hx file for usage information. Please, also note that not all HMP commands can be executed this way, in special commands that: o need to store monitor related data (eg. getfd) o read data from the user (eg. cont when the block device is encrypted) TODO: Create a blacklist for those bad commands or just let them fail? (assuming they won't blowup, of course) TODO: Maybe a command like 'cpu' requires a blacklist Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- monitor.c | 34 ++++++++++++++++++++++++++++++++++ qmp-commands.hx | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 0 deletions(-)