Patchwork Introduce query-cpu-max QMP command and cpu_max HMP counterpart

login
register
mail settings
Submitter Michal Novotny
Date March 12, 2013, 4:02 p.m.
Message ID <e3a6017620c95128121a9748d7a81faf526a08f5.1363104087.git.minovotn@redhat.com>
Download mbox | patch
Permalink /patch/227130/
State New
Headers show

Comments

Michal Novotny - March 12, 2013, 4:02 p.m.
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(+)
Eric Blake - March 13, 2013, 8:40 p.m.
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>
Markus Armbruster - March 19, 2013, 12:28 p.m.
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>
Michal Novotny - March 19, 2013, 1 p.m.
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>
Andreas Färber - March 19, 2013, 1:27 p.m.
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
Michal Novotny - March 19, 2013, 1:33 p.m.
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
>
Luiz Capitulino - March 21, 2013, 12:51 p.m.
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 */
>
Michal Novotny - March 25, 2013, 12:06 p.m.
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
Luiz Capitulino - March 25, 2013, 3:52 p.m.
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?
Michal Novotny - March 25, 2013, 3:59 p.m.
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?

Patch

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 */