[Qemu-devel] [PATCH v2] New cpu-max field in query-machines QMP command output

[Qemu-devel] [PATCH v2] New cpu-max field in query-machines QMP command output

[Michal Novotny]

Alter the query-machines QMP command to output information about
maximum number of CPUs for each machine type with default value
set to 1 in case the number of max_cpus is not set.

Signed-off-by: Michal Novotny <minovotn@redhat.com>
---
 qapi-schema.json | 4 +++-
 vl.c             | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index db542f6..689ca8d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2861,11 +2861,13 @@
 #
 # @default: #optional whether the machine is default
 #
+# @cpu-max: maximum number of CPUs supported by the machine type
+#
 # Since: 1.2.0
 ##
 { 'type': 'MachineInfo',
   'data': { 'name': 'str', '*alias': 'str',
-            '*is-default': 'bool' } }
+            '*is-default': 'bool', 'cpu-max': 'int' } }
 
 ##
 # @query-machines:
diff --git a/vl.c b/vl.c
index a8bba04..c05b3d3 100644
--- a/vl.c
+++ b/vl.c
@@ -1617,6 +1617,7 @@ MachineInfoList *qmp_query_machines(Error **errp)
         }
 
         info->name = g_strdup(m->name);
+        info->cpu_max = !m->max_cpus ? 1 : m->max_cpus;
 
         entry = g_malloc0(sizeof(*entry));
         entry->value = info;
-- 
1.7.11.7

[Qemu-devel] Fwd: [PATCH v2] New cpu-max field in query-machines QMP command output

[Michal Novotny]

Alter the query-machines QMP command to output information about
maximum number of CPUs for each machine type with default value
set to 1 in case the number of max_cpus is not set.

Signed-off-by: Michal Novotny <minovotn@redhat.com>
---
 qapi-schema.json | 4 +++-
 vl.c             | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index db542f6..689ca8d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2861,11 +2861,13 @@
 #
 # @default: #optional whether the machine is default
 #
+# @cpu-max: maximum number of CPUs supported by the machine type
+#
 # Since: 1.2.0
 ##
 { 'type': 'MachineInfo',
   'data': { 'name': 'str', '*alias': 'str',
-            '*is-default': 'bool' } }
+            '*is-default': 'bool', 'cpu-max': 'int' } }
 
 ##
 # @query-machines:
diff --git a/vl.c b/vl.c
index a8bba04..c05b3d3 100644
--- a/vl.c
+++ b/vl.c
@@ -1617,6 +1617,7 @@ MachineInfoList *qmp_query_machines(Error **errp)
         }
 
         info->name = g_strdup(m->name);
+        info->cpu_max = !m->max_cpus ? 1 : m->max_cpus;
 
         entry = g_malloc0(sizeof(*entry));
         entry->value = info;
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH v2] New cpu-max field in query-machines QMP command output

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1184 bytes --]

On 04/08/2013 10:41 AM, Michal Novotny wrote:
> Alter the query-machines QMP command to output information about
> maximum number of CPUs for each machine type with default value
> set to 1 in case the number of max_cpus is not set.
> 
> Signed-off-by: Michal Novotny <minovotn@redhat.com>
> ---
>  qapi-schema.json | 4 +++-
>  vl.c             | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index db542f6..689ca8d 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2861,11 +2861,13 @@
>  #
>  # @default: #optional whether the machine is default
>  #
> +# @cpu-max: maximum number of CPUs supported by the machine type

Typically, when adding a field in a later version than the original
introduction of the datatype, we add '(since 1.5)' to make it obvious
when to expect the field.  However, as nothing (currently) enforces this
rule, I think such an addition is minor enough that it wouldn't
invalidate the use of my:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH v2] New cpu-max field in query-machines QMP command output

[Luiz Capitulino]

On Mon, 08 Apr 2013 11:14:32 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 04/08/2013 10:41 AM, Michal Novotny wrote:
> > Alter the query-machines QMP command to output information about
> > maximum number of CPUs for each machine type with default value
> > set to 1 in case the number of max_cpus is not set.
> > 
> > Signed-off-by: Michal Novotny <minovotn@redhat.com>
> > ---
> >  qapi-schema.json | 4 +++-
> >  vl.c             | 1 +
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index db542f6..689ca8d 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -2861,11 +2861,13 @@
> >  #
> >  # @default: #optional whether the machine is default
> >  #
> > +# @cpu-max: maximum number of CPUs supported by the machine type
> 
> Typically, when adding a field in a later version than the original
> introduction of the datatype, we add '(since 1.5)' to make it obvious
> when to expect the field.  However, as nothing (currently) enforces this
> rule, I think such an addition is minor enough that it wouldn't
> invalidate the use of my:

Oh, it turns out I was making some confusion with this patch and
didn't realize it was extending the query-machines command.

I don't mean there's anything wrong with it, but my question is: doesn't
this patch invalidates query-cpu-max?

PS: I can add the '(since 1.5)' line myself.

> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

Re: [Qemu-devel] [PATCH v2] New cpu-max field in query-machines QMP command output

[Michal Novotny]

On 04/09/2013 03:06 PM, Luiz Capitulino wrote:
> On Mon, 08 Apr 2013 11:14:32 -0600
> Eric Blake <eblake@redhat.com> wrote:
>
>> On 04/08/2013 10:41 AM, Michal Novotny wrote:
>>> Alter the query-machines QMP command to output information about
>>> maximum number of CPUs for each machine type with default value
>>> set to 1 in case the number of max_cpus is not set.
>>>
>>> Signed-off-by: Michal Novotny <minovotn@redhat.com>
>>> ---
>>>  qapi-schema.json | 4 +++-
>>>  vl.c             | 1 +
>>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>> index db542f6..689ca8d 100644
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -2861,11 +2861,13 @@
>>>  #
>>>  # @default: #optional whether the machine is default
>>>  #
>>> +# @cpu-max: maximum number of CPUs supported by the machine type
>> Typically, when adding a field in a later version than the original
>> introduction of the datatype, we add '(since 1.5)' to make it obvious
>> when to expect the field.  However, as nothing (currently) enforces this
>> rule, I think such an addition is minor enough that it wouldn't
>> invalidate the use of my:
> Oh, it turns out I was making some confusion with this patch and
> didn't realize it was extending the query-machines command.
>
> I don't mean there's anything wrong with it, but my question is: doesn't
> this patch invalidates query-cpu-max?

Not really, the query-cpu-max shows maximum number of CPUs supported per
currently running machine (machine type) however the extension to
query-machines shows the maximum number of CPUs supported per each of
the displayed machine types. This usually doesn't matter on x86
architectures however it does matter on ARM, for example, as ARM has
some CPU models supporting up to 4 CPUs (highbank model) whereas most of
the ARM processors does have only one CPU.

Michal
>
> PS: I can add the '(since 1.5)' line myself.
>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>

-- 
Michal Novotny <minovotn@redhat.com>, RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org

Re: [Qemu-devel] [PATCH v2] New cpu-max field in query-machines QMP command output

[Luiz Capitulino]

On Tue, 09 Apr 2013 15:09:21 +0200
Michal Novotny <minovotn@redhat.com> wrote:

> 
> On 04/09/2013 03:06 PM, Luiz Capitulino wrote:
> > On Mon, 08 Apr 2013 11:14:32 -0600
> > Eric Blake <eblake@redhat.com> wrote:
> >
> >> On 04/08/2013 10:41 AM, Michal Novotny wrote:
> >>> Alter the query-machines QMP command to output information about
> >>> maximum number of CPUs for each machine type with default value
> >>> set to 1 in case the number of max_cpus is not set.
> >>>
> >>> Signed-off-by: Michal Novotny <minovotn@redhat.com>
> >>> ---
> >>>  qapi-schema.json | 4 +++-
> >>>  vl.c             | 1 +
> >>>  2 files changed, 4 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/qapi-schema.json b/qapi-schema.json
> >>> index db542f6..689ca8d 100644
> >>> --- a/qapi-schema.json
> >>> +++ b/qapi-schema.json
> >>> @@ -2861,11 +2861,13 @@
> >>>  #
> >>>  # @default: #optional whether the machine is default
> >>>  #
> >>> +# @cpu-max: maximum number of CPUs supported by the machine type
> >> Typically, when adding a field in a later version than the original
> >> introduction of the datatype, we add '(since 1.5)' to make it obvious
> >> when to expect the field.  However, as nothing (currently) enforces this
> >> rule, I think such an addition is minor enough that it wouldn't
> >> invalidate the use of my:
> > Oh, it turns out I was making some confusion with this patch and
> > didn't realize it was extending the query-machines command.
> >
> > I don't mean there's anything wrong with it, but my question is: doesn't
> > this patch invalidates query-cpu-max?
> 
> Not really, the query-cpu-max shows maximum number of CPUs supported per
> currently running machine (machine type) however the extension to
> query-machines shows the maximum number of CPUs supported per each of
> the displayed machine types. This usually doesn't matter on x86
> architectures however it does matter on ARM, for example, as ARM has
> some CPU models supporting up to 4 CPUs (highbank model) whereas most of
> the ARM processors does have only one CPU.

Can't we eliminate the query-cpu-max command if we extend the
query-machines command to show which machine type is in use?

Re: [Qemu-devel] [PATCH v2] New cpu-max field in query-machines QMP command output

[Michal Novotny]

On 04/09/2013 03:15 PM, Luiz Capitulino wrote:
> On Tue, 09 Apr 2013 15:09:21 +0200
> Michal Novotny <minovotn@redhat.com> wrote:
>
>> On 04/09/2013 03:06 PM, Luiz Capitulino wrote:
>>> On Mon, 08 Apr 2013 11:14:32 -0600
>>> Eric Blake <eblake@redhat.com> wrote:
>>>
>>>> On 04/08/2013 10:41 AM, Michal Novotny wrote:
>>>>> Alter the query-machines QMP command to output information about
>>>>> maximum number of CPUs for each machine type with default value
>>>>> set to 1 in case the number of max_cpus is not set.
>>>>>
>>>>> Signed-off-by: Michal Novotny <minovotn@redhat.com>
>>>>> ---
>>>>>  qapi-schema.json | 4 +++-
>>>>>  vl.c             | 1 +
>>>>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>>>> index db542f6..689ca8d 100644
>>>>> --- a/qapi-schema.json
>>>>> +++ b/qapi-schema.json
>>>>> @@ -2861,11 +2861,13 @@
>>>>>  #
>>>>>  # @default: #optional whether the machine is default
>>>>>  #
>>>>> +# @cpu-max: maximum number of CPUs supported by the machine type
>>>> Typically, when adding a field in a later version than the original
>>>> introduction of the datatype, we add '(since 1.5)' to make it obvious
>>>> when to expect the field.  However, as nothing (currently) enforces this
>>>> rule, I think such an addition is minor enough that it wouldn't
>>>> invalidate the use of my:
>>> Oh, it turns out I was making some confusion with this patch and
>>> didn't realize it was extending the query-machines command.
>>>
>>> I don't mean there's anything wrong with it, but my question is: doesn't
>>> this patch invalidates query-cpu-max?
>> Not really, the query-cpu-max shows maximum number of CPUs supported per
>> currently running machine (machine type) however the extension to
>> query-machines shows the maximum number of CPUs supported per each of
>> the displayed machine types. This usually doesn't matter on x86
>> architectures however it does matter on ARM, for example, as ARM has
>> some CPU models supporting up to 4 CPUs (highbank model) whereas most of
>> the ARM processors does have only one CPU.
> Can't we eliminate the query-cpu-max command if we extend the
> query-machines command to show which machine type is in use?

It may be useful as query-cpu-max is showing maximum number of CPUs
supported per machine type QEMU currently emulates. We don't have to
know the machine type and look for this number manually in the
query-machines output if we have query-cpu-max. We would have to both
know the machine type and look for the number manually to get max-cpu
count for currently running machine if we get rid of query-cpu-max command.

Whether something will use the value of CPU-max for currently running
machine (query-cpu-max output) is another question.

Michal

-- 
Michal Novotny <minovotn@redhat.com>, RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org

Re: [Qemu-devel] [PATCH v2] New cpu-max field in query-machines QMP command output

[Luiz Capitulino]

On Tue, 09 Apr 2013 15:32:57 +0200
Michal Novotny <minovotn@redhat.com> wrote:

> 
> On 04/09/2013 03:15 PM, Luiz Capitulino wrote:
> > On Tue, 09 Apr 2013 15:09:21 +0200
> > Michal Novotny <minovotn@redhat.com> wrote:
> >
> >> On 04/09/2013 03:06 PM, Luiz Capitulino wrote:
> >>> On Mon, 08 Apr 2013 11:14:32 -0600
> >>> Eric Blake <eblake@redhat.com> wrote:
> >>>
> >>>> On 04/08/2013 10:41 AM, Michal Novotny wrote:
> >>>>> Alter the query-machines QMP command to output information about
> >>>>> maximum number of CPUs for each machine type with default value
> >>>>> set to 1 in case the number of max_cpus is not set.
> >>>>>
> >>>>> Signed-off-by: Michal Novotny <minovotn@redhat.com>
> >>>>> ---
> >>>>>  qapi-schema.json | 4 +++-
> >>>>>  vl.c             | 1 +
> >>>>>  2 files changed, 4 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/qapi-schema.json b/qapi-schema.json
> >>>>> index db542f6..689ca8d 100644
> >>>>> --- a/qapi-schema.json
> >>>>> +++ b/qapi-schema.json
> >>>>> @@ -2861,11 +2861,13 @@
> >>>>>  #
> >>>>>  # @default: #optional whether the machine is default
> >>>>>  #
> >>>>> +# @cpu-max: maximum number of CPUs supported by the machine type
> >>>> Typically, when adding a field in a later version than the original
> >>>> introduction of the datatype, we add '(since 1.5)' to make it obvious
> >>>> when to expect the field.  However, as nothing (currently) enforces this
> >>>> rule, I think such an addition is minor enough that it wouldn't
> >>>> invalidate the use of my:
> >>> Oh, it turns out I was making some confusion with this patch and
> >>> didn't realize it was extending the query-machines command.
> >>>
> >>> I don't mean there's anything wrong with it, but my question is: doesn't
> >>> this patch invalidates query-cpu-max?
> >> Not really, the query-cpu-max shows maximum number of CPUs supported per
> >> currently running machine (machine type) however the extension to
> >> query-machines shows the maximum number of CPUs supported per each of
> >> the displayed machine types. This usually doesn't matter on x86
> >> architectures however it does matter on ARM, for example, as ARM has
> >> some CPU models supporting up to 4 CPUs (highbank model) whereas most of
> >> the ARM processors does have only one CPU.
> > Can't we eliminate the query-cpu-max command if we extend the
> > query-machines command to show which machine type is in use?
> 
> It may be useful as query-cpu-max is showing maximum number of CPUs
> supported per machine type QEMU currently emulates. We don't have to
> know the machine type and look for this number manually in the
> query-machines output if we have query-cpu-max.

QMP is a machine protocol, "manual" in that context is not a problem.
It's actually wanted.

> We would have to both
> know the machine type and look for the number manually to get max-cpu
> count for currently running machine if we get rid of query-cpu-max command.
> 
> Whether something will use the value of CPU-max for currently running
> machine (query-cpu-max output) is another question.

What you mean by that? Why are we adding a command if we don't even know
if it's going to be used?

If I remember correctly, it was said that libivrt was asking for this
command. If this is not the case let's drop it, because the command is
bad (ie. this should be done through QOM).

I've applied this patch, but again, if query-cpu-max is not required by
any QMP user, please post patches reverting it.

Re: [Qemu-devel] [PATCH v2] New cpu-max field in query-machines QMP command output

[Michal Novotny]

On 04/09/2013 03:42 PM, Luiz Capitulino wrote:
> On Tue, 09 Apr 2013 15:32:57 +0200
> Michal Novotny <minovotn@redhat.com> wrote:
>
>> On 04/09/2013 03:15 PM, Luiz Capitulino wrote:
>>> On Tue, 09 Apr 2013 15:09:21 +0200
>>> Michal Novotny <minovotn@redhat.com> wrote:
>>>
>>>> On 04/09/2013 03:06 PM, Luiz Capitulino wrote:
>>>>> On Mon, 08 Apr 2013 11:14:32 -0600
>>>>> Eric Blake <eblake@redhat.com> wrote:
>>>>>
>>>>>> On 04/08/2013 10:41 AM, Michal Novotny wrote:
>>>>>>> Alter the query-machines QMP command to output information about
>>>>>>> maximum number of CPUs for each machine type with default value
>>>>>>> set to 1 in case the number of max_cpus is not set.
>>>>>>>
>>>>>>> Signed-off-by: Michal Novotny <minovotn@redhat.com>
>>>>>>> ---
>>>>>>>  qapi-schema.json | 4 +++-
>>>>>>>  vl.c             | 1 +
>>>>>>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>>>>>> index db542f6..689ca8d 100644
>>>>>>> --- a/qapi-schema.json
>>>>>>> +++ b/qapi-schema.json
>>>>>>> @@ -2861,11 +2861,13 @@
>>>>>>>  #
>>>>>>>  # @default: #optional whether the machine is default
>>>>>>>  #
>>>>>>> +# @cpu-max: maximum number of CPUs supported by the machine type
>>>>>> Typically, when adding a field in a later version than the original
>>>>>> introduction of the datatype, we add '(since 1.5)' to make it obvious
>>>>>> when to expect the field.  However, as nothing (currently) enforces this
>>>>>> rule, I think such an addition is minor enough that it wouldn't
>>>>>> invalidate the use of my:
>>>>> Oh, it turns out I was making some confusion with this patch and
>>>>> didn't realize it was extending the query-machines command.
>>>>>
>>>>> I don't mean there's anything wrong with it, but my question is: doesn't
>>>>> this patch invalidates query-cpu-max?
>>>> Not really, the query-cpu-max shows maximum number of CPUs supported per
>>>> currently running machine (machine type) however the extension to
>>>> query-machines shows the maximum number of CPUs supported per each of
>>>> the displayed machine types. This usually doesn't matter on x86
>>>> architectures however it does matter on ARM, for example, as ARM has
>>>> some CPU models supporting up to 4 CPUs (highbank model) whereas most of
>>>> the ARM processors does have only one CPU.
>>> Can't we eliminate the query-cpu-max command if we extend the
>>> query-machines command to show which machine type is in use?
>> It may be useful as query-cpu-max is showing maximum number of CPUs
>> supported per machine type QEMU currently emulates. We don't have to
>> know the machine type and look for this number manually in the
>> query-machines output if we have query-cpu-max.
> QMP is a machine protocol, "manual" in that context is not a problem.
> It's actually wanted.


By manually in this context I mean if you have already running machine
and you know the machine type then you have to run query-machines and
look for your CPU type if you want know maximum number of CPUs supported
by this machine type.


>
>> We would have to both
>> know the machine type and look for the number manually to get max-cpu
>> count for currently running machine if we get rid of query-cpu-max command.
>>
>> Whether something will use the value of CPU-max for currently running
>> machine (query-cpu-max output) is another question.
> What you mean by that? Why are we adding a command if we don't even know
> if it's going to be used?


The original reason was to allow libvirt to know max. number of CPUs
however libvirt queries the machines and puts the aliases and CPU models
into capabilities which is basically reason why we could drop
query-cpu-max command as new implementation was necessary (the one
extending MachineInfo by cpu-max).


>
> If I remember correctly, it was said that libivrt was asking for this
> command. If this is not the case let's drop it, because the command is
> bad (ie. this should be done through QOM).

Originally yes however the implementation of extending MachineInfo seems
to be better for them so they don't need query-cpu-max anymore.


>
> I've applied this patch, but again, if query-cpu-max is not required by
> any QMP user, please post patches reverting it.

Ok, I will create a revert patch and I'll send it.

Michal

-- 
Michal Novotny <minovotn@redhat.com>, RHCE, Red Hat
Virtualization | libvirt-php bindings | php-virt-control.org

Re: [Qemu-devel] [PATCH v2] New cpu-max field in query-machines QMP command output

[Peter Krempa]

On 04/09/13 15:06, Luiz Capitulino wrote:
> On Mon, 08 Apr 2013 11:14:32 -0600
> Eric Blake <eblake@redhat.com> wrote:
>
>> On 04/08/2013 10:41 AM, Michal Novotny wrote:
>>> Alter the query-machines QMP command to output information about
>>> maximum number of CPUs for each machine type with default value
>>> set to 1 in case the number of max_cpus is not set.
>>>
>>> Signed-off-by: Michal Novotny <minovotn@redhat.com>
>>> ---
>>>   qapi-schema.json | 4 +++-
>>>   vl.c             | 1 +
>>>   2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/qapi-schema.json b/qapi-schema.json
>>> index db542f6..689ca8d 100644
>>> --- a/qapi-schema.json
>>> +++ b/qapi-schema.json
>>> @@ -2861,11 +2861,13 @@
>>>   #
>>>   # @default: #optional whether the machine is default
>>>   #
>>> +# @cpu-max: maximum number of CPUs supported by the machine type
>>
>> Typically, when adding a field in a later version than the original
>> introduction of the datatype, we add '(since 1.5)' to make it obvious
>> when to expect the field.  However, as nothing (currently) enforces this
>> rule, I think such an addition is minor enough that it wouldn't
>> invalidate the use of my:
>
> Oh, it turns out I was making some confusion with this patch and
> didn't realize it was extending the query-machines command.
>
> I don't mean there's anything wrong with it, but my question is: doesn't
> this patch invalidates query-cpu-max?

Unfortunately, for libvirt query-cpu-max isn't really usable as it needs 
us to start qemu with the correct machine type. This would increase 
overhead as we would need to start the qemu process with a safe number 
of cpus, use query-cpu-max and then restart the process.

The information added in the query-machines output can on the other hand 
be cached (we are already doing this for the machine types) and used 
later from the cache without increasing overhead.

So yes, I think it invalidates query-cpu-max and it can be removed in 
case it wasn't released.

Peter

>
> PS: I can add the '(since 1.5)' line myself.
>
>>
>> Reviewed-by: Eric Blake <eblake@redhat.com>
>>
>
>