| Message ID | 1288362514-31407-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 the documentation added to the qmp-commands.hx file > for additional details about the interface and its limitations. > > Signed-off-by: Luiz Capitulino <lcapitulino@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. 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? Regardless, this is clearly better than nothing. We can work on limitations later, should they turn out to be bothersome.
On Wed, 10 Nov 2010 14:20:12 +0100 Markus Armbruster <armbru@redhat.com> wrote: > Luiz Capitulino <lcapitulino@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 <lcapitulino@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.
Luiz Capitulino <lcapitulino@redhat.com> writes: > On Wed, 10 Nov 2010 14:20:12 +0100 > Markus Armbruster <armbru@redhat.com> wrote: > >> Luiz Capitulino <lcapitulino@redhat.com> writes: [...] >> > 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. Hmm, that sounds like we should investigate the getfd problem sooner rather than later. [...]
On Thu, Nov 11, 2010 at 04:47:41PM +0100, Markus Armbruster wrote: > Luiz Capitulino <lcapitulino@redhat.com> writes: > > > On Wed, 10 Nov 2010 14:20:12 +0100 > > Markus Armbruster <armbru@redhat.com> wrote: > > > >> Luiz Capitulino <lcapitulino@redhat.com> writes: > [...] > >> > 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. > > Hmm, that sounds like we should investigate the getfd problem sooner > rather than later. The SCM_RIGHTS code allows you to send/receive multiple file handles in a single sendmsg/recvmsg call. So why don't we just allow sending of the file handles with the monitor command that actually needs them, instead of ahead of time using send_fd. This simplifies life for the client because they also don't have to worry about cleanup using close_fd if the command using the FD fails. Regards, Daniel
On 11/11/2010 09:55 AM, Daniel P. Berrange wrote: > On Thu, Nov 11, 2010 at 04:47:41PM +0100, Markus Armbruster wrote: > >> Luiz Capitulino<lcapitulino@redhat.com> writes: >> >> >>> On Wed, 10 Nov 2010 14:20:12 +0100 >>> Markus Armbruster<armbru@redhat.com> wrote: >>> >>> >>>> Luiz Capitulino<lcapitulino@redhat.com> writes: >>>> >> [...] >> >>>>> 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. >>> >> Hmm, that sounds like we should investigate the getfd problem sooner >> rather than later. >> > The SCM_RIGHTS code allows you to send/receive multiple file handles in a > single sendmsg/recvmsg call. So why don't we just allow sending of the > file handles with the monitor command that actually needs them, instead of > ahead of time using send_fd. This simplifies life for the client because > they also don't have to worry about cleanup using close_fd if the command > using the FD fails. > How do we identify file descriptors and then map them to a command? Regards, Anthony Liguori > Regards, > Daniel >
On Thu, Nov 11, 2010 at 10:30:47AM -0600, Anthony Liguori wrote: > On 11/11/2010 09:55 AM, Daniel P. Berrange wrote: > >On Thu, Nov 11, 2010 at 04:47:41PM +0100, Markus Armbruster wrote: > >>>>> 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. > >>> > >>Hmm, that sounds like we should investigate the getfd problem sooner > >>rather than later. > >> > >The SCM_RIGHTS code allows you to send/receive multiple file handles in a > >single sendmsg/recvmsg call. So why don't we just allow sending of the > >file handles with the monitor command that actually needs them, instead of > >ahead of time using send_fd. This simplifies life for the client because > >they also don't have to worry about cleanup using close_fd if the command > >using the FD fails. > > How do we identify file descriptors and then map them to a command? IIUC, the FDs sent/received via struct cmsghdr are in a strictly ordered array, so why not just define a placeholder syntax for the commands that maps to the array indexes. eg netdev_add tap,fd=$0,vhost_fd=$1,id=hostnet0 The '$' sign is not valid for a normal FD number, so use of a $0, $1, $2, etc can reliably be substituted with the real FD number from the cmsghdr array elements 0, 1, 2, etc Regards, Daniel
On Thu, 11 Nov 2010 16:39:52 +0000 "Daniel P. Berrange" <berrange@redhat.com> wrote: > On Thu, Nov 11, 2010 at 10:30:47AM -0600, Anthony Liguori wrote: > > On 11/11/2010 09:55 AM, Daniel P. Berrange wrote: > > >On Thu, Nov 11, 2010 at 04:47:41PM +0100, Markus Armbruster wrote: > > >>>>> 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. > > >>> > > >>Hmm, that sounds like we should investigate the getfd problem sooner > > >>rather than later. > > >> > > >The SCM_RIGHTS code allows you to send/receive multiple file handles in a > > >single sendmsg/recvmsg call. So why don't we just allow sending of the > > >file handles with the monitor command that actually needs them, instead of > > >ahead of time using send_fd. This simplifies life for the client because > > >they also don't have to worry about cleanup using close_fd if the command > > >using the FD fails. > > > > How do we identify file descriptors and then map them to a command? > > IIUC, the FDs sent/received via struct cmsghdr are in a strictly > ordered array, so why not just define a placeholder syntax for > the commands that maps to the array indexes. eg > > netdev_add tap,fd=$0,vhost_fd=$1,id=hostnet0 > > The '$' sign is not valid for a normal FD number, so use of a $0, > $1, $2, etc can reliably be substituted with the real FD number from > the cmsghdr array elements 0, 1, 2, etc I have the impression that the real problem is that they are being stored in the wrong place (ie. the monitor), *maybe* they should be stored somewhere else, ie. a special chardev or something. Today those fds are linked to a given monitor session..
On Thu, 11 Nov 2010 16:47:41 +0100 Markus Armbruster <armbru@redhat.com> wrote: > Luiz Capitulino <lcapitulino@redhat.com> writes: > > > On Wed, 10 Nov 2010 14:20:12 +0100 > > Markus Armbruster <armbru@redhat.com> wrote: > > > >> Luiz Capitulino <lcapitulino@redhat.com> writes: > [...] > >> > 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. > > Hmm, that sounds like we should investigate the getfd problem sooner > rather than later. Absolutely, but this shouldn't prevent this series of being merged, right?
On 11/11/2010 10:39 AM, Daniel P. Berrange wrote: > IIUC, the FDs sent/received via struct cmsghdr are in a strictly > ordered array, so why not just define a placeholder syntax for > the commands that maps to the array indexes. eg > > netdev_add tap,fd=$0,vhost_fd=$1,id=hostnet0 > > The '$' sign is not valid for a normal FD number, so use of a $0, > $1, $2, etc can reliably be substituted with the real FD number from > the cmsghdr array elements 0, 1, 2, etc > Character devices are streaming so it's hard to refer to array indexes in a single message. You could count it for the entire session but session boundaries aren't really well defined. I don't think it's an obvious win. Regards, Anthony Liguori > Regards, > Daniel >
On 11/11/2010 10:55 AM, Luiz Capitulino wrote: > On Thu, 11 Nov 2010 16:47:41 +0100 > Markus Armbruster<armbru@redhat.com> wrote: > > >> Luiz Capitulino<lcapitulino@redhat.com> writes: >> >> >>> On Wed, 10 Nov 2010 14:20:12 +0100 >>> Markus Armbruster<armbru@redhat.com> wrote: >>> >>> >>>> Luiz Capitulino<lcapitulino@redhat.com> writes: >>>> >> [...] >> >>>>> 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. >>> >> Hmm, that sounds like we should investigate the getfd problem sooner >> rather than later. >> > Absolutely, but this shouldn't prevent this series of being merged, right? > BTW, don't we have all of the fd= commands covered in QMP already? Regards, Anthony Liguori
Luiz Capitulino <lcapitulino@redhat.com> writes: > On Thu, 11 Nov 2010 16:47:41 +0100 > Markus Armbruster <armbru@redhat.com> wrote: > >> Luiz Capitulino <lcapitulino@redhat.com> writes: >> >> > On Wed, 10 Nov 2010 14:20:12 +0100 >> > Markus Armbruster <armbru@redhat.com> wrote: >> > >> >> Luiz Capitulino <lcapitulino@redhat.com> writes: >> [...] >> >> > 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. >> >> Hmm, that sounds like we should investigate the getfd problem sooner >> rather than later. > > Absolutely, but this shouldn't prevent this series of being merged, right? Right.
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"); + } + 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 =================
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 <lcapitulino@redhat.com> --- monitor.c | 42 ++++++++++++++++++++++++++++++++++++++++++ qmp-commands.hx | 45 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 0 deletions(-)