| Message ID | e3a6017620c95128121a9748d7a81faf526a08f5.1363104087.git.minovotn@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03/12/2013 10:02 AM, Michal Novotny wrote: > This is the patch to introduce the query-cpu-max QMP command to get > the maximum number of CPUs supported by the currently running emulator > instance. This may differ machine from machine as defined by -machine > settings and max_cpus member of QEMUMachine structure. > > It's been tested both using QMP/qmp utility and telnet session on > the QEMU session. > > The HMP counterpart called cpu_max has been introduced by this patch > too. > > Signed-off-by: Michal Novotny <minovotn@redhat.com> > --- > hmp-commands.hx | 14 ++++++++++++++ > hmp.c | 15 +++++++++++++++ > hmp.h | 1 + > qapi-schema.json | 11 +++++++++++ > qmp-commands.hx | 22 ++++++++++++++++++++++ > vl.c | 5 +++++ > 6 files changed, 68 insertions(+) Reviewed-by: Eric Blake <eblake@redhat.com>
Please cc: Luiz and me on QMP work in the future. Michal Novotny <minovotn@redhat.com> writes: > This is the patch to introduce the query-cpu-max QMP command to get > the maximum number of CPUs supported by the currently running emulator > instance. This may differ machine from machine as defined by -machine > settings and max_cpus member of QEMUMachine structure. Humor me: don't start commit message bodies with "This patch" or variations thereof, and don't repeat the subject. Suggest: QMP command query-cpu-max returns the maximum number of CPUs supported by the currently running emulator instance, as defined in its QEMUMachine struct. > It's been tested both using QMP/qmp utility and telnet session on > the QEMU session. What's a QMP/qmp utility? Let's drop this sentence. In the future, feel free to put testing info below the "---" line. > The HMP counterpart called cpu_max has been introduced by this patch > too. Grammar nit: s/has been/is/. Even better, avoid passive voice. Hmm, I just rewrote most of your commit message, so why not rewrite all of it: New QMP command query-cpu-max and HMP command cpu_max These commands return the maximum number of CPUs supported by the currently running emulator instance, as defined in its QEMUMachine struct. Perhaps Luiz can fix up the commit message commit, if you don't mind. Patch looks good. Should query commands for machine properties multiply, we should consider creating a single command returning all of them. I'm not asking you to do that now. Reviewed-by: Markus Armbruster <armbru@redhat.com>
On 03/19/2013 01:28 PM, Markus Armbruster wrote: > Please cc: Luiz and me on QMP work in the future. > > Michal Novotny <minovotn@redhat.com> writes: > >> This is the patch to introduce the query-cpu-max QMP command to get >> the maximum number of CPUs supported by the currently running emulator >> instance. This may differ machine from machine as defined by -machine >> settings and max_cpus member of QEMUMachine structure. > Humor me: don't start commit message bodies with "This patch" or > variations thereof, and don't repeat the subject. Suggest: > > QMP command query-cpu-max returns the maximum number of CPUs supported > by the currently running emulator instance, as defined in its > QEMUMachine struct. > >> It's been tested both using QMP/qmp utility and telnet session on >> the QEMU session. > What's a QMP/qmp utility? Let's drop this sentence. In the future, > feel free to put testing info below the "---" line. I meant the utility located in <qemu-dir-clone>/QMP/qmp which is essentially the python script for QMP testing. > >> The HMP counterpart called cpu_max has been introduced by this patch >> too. > Grammar nit: s/has been/is/. Even better, avoid passive voice. > > Hmm, I just rewrote most of your commit message, so why not rewrite all > of it: > > New QMP command query-cpu-max and HMP command cpu_max > > These commands return the maximum number of CPUs supported by the > currently running emulator instance, as defined in its QEMUMachine > struct. > > Perhaps Luiz can fix up the commit message commit, if you don't mind. > > Patch looks good. The commit log change is fine with me. Thanks for your feedback! Michal > Should query commands for machine properties multiply, we should > consider creating a single command returning all of them. I'm not > asking you to do that now. > > Reviewed-by: Markus Armbruster <armbru@redhat.com>
Am 19.03.2013 13:28, schrieb Markus Armbruster: > Please cc: Luiz and me on QMP work in the future. > > Michal Novotny <minovotn@redhat.com> writes: > >> This is the patch to introduce the query-cpu-max QMP command to get >> the maximum number of CPUs supported by the currently running emulator >> instance. This may differ machine from machine as defined by -machine >> settings and max_cpus member of QEMUMachine structure. > > Humor me: don't start commit message bodies with "This patch" or > variations thereof, and don't repeat the subject. Suggest: > > QMP command query-cpu-max returns the maximum number of CPUs supported > by the currently running emulator instance, as defined in its > QEMUMachine struct. > >> It's been tested both using QMP/qmp utility and telnet session on >> the QEMU session. > > What's a QMP/qmp utility? Our ./QMP/qmp script, I guess. > Let's drop this sentence. In the future, > feel free to put testing info below the "---" line. > >> The HMP counterpart called cpu_max has been introduced by this patch >> too. > > Grammar nit: s/has been/is/. Even better, avoid passive voice. > > Hmm, I just rewrote most of your commit message, so why not rewrite all > of it: > > New QMP command query-cpu-max and HMP command cpu_max > > These commands return the maximum number of CPUs supported by the > currently running emulator instance, as defined in its QEMUMachine > struct. > > Perhaps Luiz can fix up the commit message commit, if you don't mind. > > Patch looks good. > > Should query commands for machine properties multiply, we should > consider creating a single command returning all of them. I'm not > asking you to do that now. > > Reviewed-by: Markus Armbruster <armbru@redhat.com> From my CPU perspective I am wondering if this info has future as-is? IIUC this QEMUMachine value differs from the value exposed to SeaBIOS/KVM already since recently, as it ignores CPU topology. I'm guessing the libvirt use case is just to detect users specifying too many -cpu options, so I won't veto this, but want to caution that we may need to change this as we proceed with CPU remodelling. Regards, Andreas
On 03/19/2013 02:27 PM, Andreas Färber wrote: > Am 19.03.2013 13:28, schrieb Markus Armbruster: >> Please cc: Luiz and me on QMP work in the future. >> >> Michal Novotny <minovotn@redhat.com> writes: >> >>> This is the patch to introduce the query-cpu-max QMP command to get >>> the maximum number of CPUs supported by the currently running emulator >>> instance. This may differ machine from machine as defined by -machine >>> settings and max_cpus member of QEMUMachine structure. >> Humor me: don't start commit message bodies with "This patch" or >> variations thereof, and don't repeat the subject. Suggest: >> >> QMP command query-cpu-max returns the maximum number of CPUs supported >> by the currently running emulator instance, as defined in its >> QEMUMachine struct. >> >>> It's been tested both using QMP/qmp utility and telnet session on >>> the QEMU session. >> What's a QMP/qmp utility? > Our ./QMP/qmp script, I guess. > >> Let's drop this sentence. In the future, >> feel free to put testing info below the "---" line. >> >>> The HMP counterpart called cpu_max has been introduced by this patch >>> too. >> Grammar nit: s/has been/is/. Even better, avoid passive voice. >> >> Hmm, I just rewrote most of your commit message, so why not rewrite all >> of it: >> >> New QMP command query-cpu-max and HMP command cpu_max >> >> These commands return the maximum number of CPUs supported by the >> currently running emulator instance, as defined in its QEMUMachine >> struct. >> >> Perhaps Luiz can fix up the commit message commit, if you don't mind. >> >> Patch looks good. >> >> Should query commands for machine properties multiply, we should >> consider creating a single command returning all of them. I'm not >> asking you to do that now. >> >> Reviewed-by: Markus Armbruster <armbru@redhat.com> > From my CPU perspective I am wondering if this info has future as-is? > IIUC this QEMUMachine value differs from the value exposed to > SeaBIOS/KVM already since recently, as it ignores CPU topology. > > I'm guessing the libvirt use case is just to detect users specifying too > many -cpu options, so I won't veto this, but want to caution that we may > need to change this as we proceed with CPU remodelling. Yeah, this patch was written for libvirt as one of the libvirt developers was not happy qemu doesn't support that yet. Michal > > Regards, > Andreas >
On Tue, 12 Mar 2013 17:03:31 +0100 Michal Novotny <minovotn@redhat.com> wrote: > This is the patch to introduce the query-cpu-max QMP command to get > the maximum number of CPUs supported by the currently running emulator > instance. This may differ machine from machine as defined by -machine > settings and max_cpus member of QEMUMachine structure. > > It's been tested both using QMP/qmp utility and telnet session on > the QEMU session. Patch looks good, but it doesn't build. See below. > > The HMP counterpart called cpu_max has been introduced by this patch > too. > > Signed-off-by: Michal Novotny <minovotn@redhat.com> > --- > hmp-commands.hx | 14 ++++++++++++++ > hmp.c | 15 +++++++++++++++ > hmp.h | 1 + > qapi-schema.json | 11 +++++++++++ > qmp-commands.hx | 22 ++++++++++++++++++++++ > vl.c | 5 +++++ > 6 files changed, 68 insertions(+) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 69c707d..b1224e3 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -690,6 +690,20 @@ Set the default CPU. > ETEXI > > { > + .name = "cpu_max", > + .args_type = "", > + .params = "", > + .help = "Get maximum number of VCPUs supported by machine", > + .mhandler.cmd = hmp_query_cpu_max, > + }, > + > +STEXI > +@item cpu_max > +@findex cpu_max > +Returns the number of CPUs supported by the machine being emulated. > +ETEXI > + > + { > .name = "mouse_move", > .args_type = "dx_str:s,dy_str:s,dz_str:s?", > .params = "dx dy [dz]", > diff --git a/hmp.c b/hmp.c > index 2f47a8a..742f5c7 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -704,6 +704,21 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict) > g_free(data); > } > > +void hmp_query_cpu_max(Monitor *mon, const QDict *qdict) > +{ > + int cpu_max; > + Error *errp = NULL; > + > + cpu_max = qmp_query_cpu_max(&errp); > + if (errp) { > + monitor_printf(mon, "%s\n", error_get_pretty(errp)); > + error_free(errp); > + return; > + } > + > + monitor_printf(mon, "Maximum number of CPUs is %d\n", cpu_max); > +} > + > static void hmp_cont_cb(void *opaque, int err) > { > if (!err) { > diff --git a/hmp.h b/hmp.h > index 30b3c20..f0c3352 100644 > --- a/hmp.h > +++ b/hmp.h > @@ -41,6 +41,7 @@ void hmp_stop(Monitor *mon, const QDict *qdict); > void hmp_system_reset(Monitor *mon, const QDict *qdict); > void hmp_system_powerdown(Monitor *mon, const QDict *qdict); > void hmp_cpu(Monitor *mon, const QDict *qdict); > +void hmp_query_cpu_max(Monitor *mon, const QDict *qdict); > void hmp_memsave(Monitor *mon, const QDict *qdict); > void hmp_pmemsave(Monitor *mon, const QDict *qdict); > void hmp_ringbuf_write(Monitor *mon, const QDict *qdict); > diff --git a/qapi-schema.json b/qapi-schema.json > index 28b070f..e828839 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -1832,6 +1832,17 @@ > { 'command': 'query-migrate-cache-size', 'returns': 'int' } > > ## > +## @query-cpu-max > +## > +## query maximum number of CPUs supported by machine > +## > +## Returns: number of CPUs > +## > +## Since: 1.5 > +### > +{ 'command': 'query-cpu-max', 'returns': 'int' } > + > +## > # @ObjectPropertyInfo: > # > # @name: the name of the property > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 95022e2..d29de4b 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -385,6 +385,28 @@ Note: CPUs' indexes are obtained with the 'query-cpus' command. > EQMP > > { > + .name = "query-cpu-max", > + .args_type = "", > + .mhandler.cmd_new = qmp_marshal_input_query_cpu_max, > + }, > + > +SQMP > +query-cpu-max > +------------- > + > +Get the maximum CPUs supported by the machine being currently > +emulated. > + > +Returns json-int. > + > +Example: > + > +-> { "execute": "query-cpu-max" } > +<- { "return": 255 } > + > +EQMP > + > + { > .name = "memsave", > .args_type = "val:l,size:i,filename:s,cpu:i?", > .mhandler.cmd_new = qmp_marshal_input_memsave, > diff --git a/vl.c b/vl.c > index 154f7ba..e04feb0 100644 > --- a/vl.c > +++ b/vl.c > @@ -632,6 +632,11 @@ StatusInfo *qmp_query_status(Error **errp) > return info; > } > > +int qmp_query_cpu_max(Error **errp) This should be int64_t, otherwise: /home/lcapitulino/work/src/upstream/qmp-unstable/vl.c:665:5: error: conflicting types for ‘qmp_query_cpu_max’ In file included from /home/lcapitulino/work/src/upstream/qmp-unstable/vl.c:151:0: ./qmp-commands.h:117:9: note: previous declaration of ‘qmp_query_cpu_max’ was here make: *** [vl.o] Error 1 make: *** Waiting for unfinished jobs.... I wonder why you didn't get this. Also, as you'll respin, please fix the log message as asked by Markus and CC us :) > +{ > + return current_machine->max_cpus; > +} > + > /***********************************************************/ > /* real time host monotonic timer */ >
On 03/21/2013 01:51 PM, Luiz Capitulino wrote: > On Tue, 12 Mar 2013 17:03:31 +0100 > Michal Novotny <minovotn@redhat.com> wrote: > >> This is the patch to introduce the query-cpu-max QMP command to get >> the maximum number of CPUs supported by the currently running emulator >> instance. This may differ machine from machine as defined by -machine >> settings and max_cpus member of QEMUMachine structure. >> >> It's been tested both using QMP/qmp utility and telnet session on >> the QEMU session. > Patch looks good, but it doesn't build. See below. Thanks! I've rebased but I don't know why it didn't apply before. I sent v2 to the list about a minute ago with Markus' comments taken in account (thanks Markus!). Everything compiled fine for all archs and it's been tested for x86_64 and ARM architectures (Raspbian ARM image is the only one for non-x86 arch I have on my laptop). Thanks, Michal
On Mon, 25 Mar 2013 13:06:27 +0100 Michal Novotny <minovotn@redhat.com> wrote: > > On 03/21/2013 01:51 PM, Luiz Capitulino wrote: > > On Tue, 12 Mar 2013 17:03:31 +0100 > > Michal Novotny <minovotn@redhat.com> wrote: > > > >> This is the patch to introduce the query-cpu-max QMP command to get > >> the maximum number of CPUs supported by the currently running emulator > >> instance. This may differ machine from machine as defined by -machine > >> settings and max_cpus member of QEMUMachine structure. > >> > >> It's been tested both using QMP/qmp utility and telnet session on > >> the QEMU session. > > Patch looks good, but it doesn't build. See below. > > Thanks! I've rebased but I don't know why it didn't apply before. I sent > v2 to the list about a minute ago with Markus' comments taken in account > (thanks Markus!). I don't see it. Could you please re-send and CC me?
Resent now. it went to my inbox (added from SOB) so hopefully it went to you and list too (however list is too slow to verify now). Michal On 03/25/2013 04:52 PM, Luiz Capitulino wrote: > On Mon, 25 Mar 2013 13:06:27 +0100 > Michal Novotny <minovotn@redhat.com> wrote: > >> On 03/21/2013 01:51 PM, Luiz Capitulino wrote: >>> On Tue, 12 Mar 2013 17:03:31 +0100 >>> Michal Novotny <minovotn@redhat.com> wrote: >>> >>>> This is the patch to introduce the query-cpu-max QMP command to get >>>> the maximum number of CPUs supported by the currently running emulator >>>> instance. This may differ machine from machine as defined by -machine >>>> settings and max_cpus member of QEMUMachine structure. >>>> >>>> It's been tested both using QMP/qmp utility and telnet session on >>>> the QEMU session. >>> Patch looks good, but it doesn't build. See below. >> Thanks! I've rebased but I don't know why it didn't apply before. I sent >> v2 to the list about a minute ago with Markus' comments taken in account >> (thanks Markus!). > I don't see it. Could you please re-send and CC me?
diff --git a/hmp-commands.hx b/hmp-commands.hx index 69c707d..b1224e3 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -690,6 +690,20 @@ Set the default CPU. ETEXI { + .name = "cpu_max", + .args_type = "", + .params = "", + .help = "Get maximum number of VCPUs supported by machine", + .mhandler.cmd = hmp_query_cpu_max, + }, + +STEXI +@item cpu_max +@findex cpu_max +Returns the number of CPUs supported by the machine being emulated. +ETEXI + + { .name = "mouse_move", .args_type = "dx_str:s,dy_str:s,dz_str:s?", .params = "dx dy [dz]", diff --git a/hmp.c b/hmp.c index 2f47a8a..742f5c7 100644 --- a/hmp.c +++ b/hmp.c @@ -704,6 +704,21 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict) g_free(data); } +void hmp_query_cpu_max(Monitor *mon, const QDict *qdict) +{ + int cpu_max; + Error *errp = NULL; + + cpu_max = qmp_query_cpu_max(&errp); + if (errp) { + monitor_printf(mon, "%s\n", error_get_pretty(errp)); + error_free(errp); + return; + } + + monitor_printf(mon, "Maximum number of CPUs is %d\n", cpu_max); +} + static void hmp_cont_cb(void *opaque, int err) { if (!err) { diff --git a/hmp.h b/hmp.h index 30b3c20..f0c3352 100644 --- a/hmp.h +++ b/hmp.h @@ -41,6 +41,7 @@ void hmp_stop(Monitor *mon, const QDict *qdict); void hmp_system_reset(Monitor *mon, const QDict *qdict); void hmp_system_powerdown(Monitor *mon, const QDict *qdict); void hmp_cpu(Monitor *mon, const QDict *qdict); +void hmp_query_cpu_max(Monitor *mon, const QDict *qdict); void hmp_memsave(Monitor *mon, const QDict *qdict); void hmp_pmemsave(Monitor *mon, const QDict *qdict); void hmp_ringbuf_write(Monitor *mon, const QDict *qdict); diff --git a/qapi-schema.json b/qapi-schema.json index 28b070f..e828839 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1832,6 +1832,17 @@ { 'command': 'query-migrate-cache-size', 'returns': 'int' } ## +## @query-cpu-max +## +## query maximum number of CPUs supported by machine +## +## Returns: number of CPUs +## +## Since: 1.5 +### +{ 'command': 'query-cpu-max', 'returns': 'int' } + +## # @ObjectPropertyInfo: # # @name: the name of the property diff --git a/qmp-commands.hx b/qmp-commands.hx index 95022e2..d29de4b 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -385,6 +385,28 @@ Note: CPUs' indexes are obtained with the 'query-cpus' command. EQMP { + .name = "query-cpu-max", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_query_cpu_max, + }, + +SQMP +query-cpu-max +------------- + +Get the maximum CPUs supported by the machine being currently +emulated. + +Returns json-int. + +Example: + +-> { "execute": "query-cpu-max" } +<- { "return": 255 } + +EQMP + + { .name = "memsave", .args_type = "val:l,size:i,filename:s,cpu:i?", .mhandler.cmd_new = qmp_marshal_input_memsave, diff --git a/vl.c b/vl.c index 154f7ba..e04feb0 100644 --- a/vl.c +++ b/vl.c @@ -632,6 +632,11 @@ StatusInfo *qmp_query_status(Error **errp) return info; } +int qmp_query_cpu_max(Error **errp) +{ + return current_machine->max_cpus; +} + /***********************************************************/ /* real time host monotonic timer */
This is the patch to introduce the query-cpu-max QMP command to get the maximum number of CPUs supported by the currently running emulator instance. This may differ machine from machine as defined by -machine settings and max_cpus member of QEMUMachine structure. It's been tested both using QMP/qmp utility and telnet session on the QEMU session. The HMP counterpart called cpu_max has been introduced by this patch too. Signed-off-by: Michal Novotny <minovotn@redhat.com> --- hmp-commands.hx | 14 ++++++++++++++ hmp.c | 15 +++++++++++++++ hmp.h | 1 + qapi-schema.json | 11 +++++++++++ qmp-commands.hx | 22 ++++++++++++++++++++++ vl.c | 5 +++++ 6 files changed, 68 insertions(+)