[PATCH v15 09/11] machine: adding s390 topology to query-cpu-fast

Pierre Morel posted 11 patches 2 years, 3 months ago
There is a newer version of this series
[PATCH v15 09/11] machine: adding s390 topology to query-cpu-fast
Posted by Pierre Morel 2 years, 3 months ago
S390x provides two more topology containers above the sockets,
books and drawers.

Let's add these CPU attributes to the QAPI command query-cpu-fast.

Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
 qapi/machine.json          | 13 ++++++++++---
 hw/core/machine-qmp-cmds.c |  2 ++
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/qapi/machine.json b/qapi/machine.json
index 3036117059..e36c39e258 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -53,11 +53,18 @@
 #
 # Additional information about a virtual S390 CPU
 #
-# @cpu-state: the virtual CPU's state
+# @cpu-state: the virtual CPU's state (since 2.12)
+# @dedicated: the virtual CPU's dedication (since 8.0)
+# @polarity: the virtual CPU's polarity (since 8.0)
 #
 # Since: 2.12
 ##
-{ 'struct': 'CpuInfoS390', 'data': { 'cpu-state': 'CpuS390State' } }
+{ 'struct': 'CpuInfoS390',
+    'data': { 'cpu-state': 'CpuS390State',
+              'dedicated': 'bool',
+              'polarity': 'int'
+    }
+}
 
 ##
 # @CpuInfoFast:
@@ -70,7 +77,7 @@
 #
 # @thread-id: ID of the underlying host thread
 #
-# @props: properties describing to which node/socket/core/thread
+# @props: properties describing to which node/drawer/book/socket/core/thread
 #         virtual CPU belongs to, provided if supported by board
 #
 # @target: the QEMU system emulation target, which determines which
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 80d5e59651..e6d93cf2a0 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -30,6 +30,8 @@ static void cpustate_to_cpuinfo_s390(CpuInfoS390 *info, const CPUState *cpu)
     CPUS390XState *env = &s390_cpu->env;
 
     info->cpu_state = env->cpu_state;
+    info->dedicated = env->dedicated;
+    info->polarity = env->entitlement;
 #else
     abort();
 #endif
-- 
2.31.1
Re: [PATCH v15 09/11] machine: adding s390 topology to query-cpu-fast
Posted by Nina Schoetterl-Glausch 2 years, 3 months ago
On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote:
> S390x provides two more topology containers above the sockets,
> books and drawers.
> 
> Let's add these CPU attributes to the QAPI command query-cpu-fast.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>  qapi/machine.json          | 13 ++++++++++---
>  hw/core/machine-qmp-cmds.c |  2 ++
>  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 3036117059..e36c39e258 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -53,11 +53,18 @@
>  #
>  # Additional information about a virtual S390 CPU
>  #
> -# @cpu-state: the virtual CPU's state
> +# @cpu-state: the virtual CPU's state (since 2.12)
> +# @dedicated: the virtual CPU's dedication (since 8.0)
> +# @polarity: the virtual CPU's polarity (since 8.0)
>  #
>  # Since: 2.12
>  ##
> -{ 'struct': 'CpuInfoS390', 'data': { 'cpu-state': 'CpuS390State' } }
> +{ 'struct': 'CpuInfoS390',
> +    'data': { 'cpu-state': 'CpuS390State',
> +              'dedicated': 'bool',
> +              'polarity': 'int'
> +    }
> +}
>  
>  ##
>  # @CpuInfoFast:
> @@ -70,7 +77,7 @@
>  #
>  # @thread-id: ID of the underlying host thread
>  #
> -# @props: properties describing to which node/socket/core/thread
> +# @props: properties describing to which node/drawer/book/socket/core/thread
>  #         virtual CPU belongs to, provided if supported by board
>  #
>  # @target: the QEMU system emulation target, which determines which
> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
> index 80d5e59651..e6d93cf2a0 100644
> --- a/hw/core/machine-qmp-cmds.c
> +++ b/hw/core/machine-qmp-cmds.c
> @@ -30,6 +30,8 @@ static void cpustate_to_cpuinfo_s390(CpuInfoS390 *info, const CPUState *cpu)
>      CPUS390XState *env = &s390_cpu->env;
>  
>      info->cpu_state = env->cpu_state;
> +    info->dedicated = env->dedicated;
> +    info->polarity = env->entitlement;

Might want to do s/polarity/entitlement on the whole patch to make this more coherent.
Reviewed-by: Nina Schoetterl-Glausch if you fix the issues in Thomas' first reply.

>  #else
>      abort();
>  #endif
Re: [PATCH v15 09/11] machine: adding s390 topology to query-cpu-fast
Posted by Pierre Morel 2 years, 3 months ago

On 2/7/23 19:26, Nina Schoetterl-Glausch wrote:
> On Wed, 2023-02-01 at 14:20 +0100, Pierre Morel wrote:
>> S390x provides two more topology containers above the sockets,
>> books and drawers.
>>
>> Let's add these CPU attributes to the QAPI command query-cpu-fast.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   qapi/machine.json          | 13 ++++++++++---
>>   hw/core/machine-qmp-cmds.c |  2 ++
>>   2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index 3036117059..e36c39e258 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -53,11 +53,18 @@
>>   #
>>   # Additional information about a virtual S390 CPU
>>   #
>> -# @cpu-state: the virtual CPU's state
>> +# @cpu-state: the virtual CPU's state (since 2.12)
>> +# @dedicated: the virtual CPU's dedication (since 8.0)
>> +# @polarity: the virtual CPU's polarity (since 8.0)
>>   #
>>   # Since: 2.12
>>   ##
>> -{ 'struct': 'CpuInfoS390', 'data': { 'cpu-state': 'CpuS390State' } }
>> +{ 'struct': 'CpuInfoS390',
>> +    'data': { 'cpu-state': 'CpuS390State',
>> +              'dedicated': 'bool',
>> +              'polarity': 'int'
>> +    }
>> +}
>>   
>>   ##
>>   # @CpuInfoFast:
>> @@ -70,7 +77,7 @@
>>   #
>>   # @thread-id: ID of the underlying host thread
>>   #
>> -# @props: properties describing to which node/socket/core/thread
>> +# @props: properties describing to which node/drawer/book/socket/core/thread
>>   #         virtual CPU belongs to, provided if supported by board
>>   #
>>   # @target: the QEMU system emulation target, which determines which
>> diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
>> index 80d5e59651..e6d93cf2a0 100644
>> --- a/hw/core/machine-qmp-cmds.c
>> +++ b/hw/core/machine-qmp-cmds.c
>> @@ -30,6 +30,8 @@ static void cpustate_to_cpuinfo_s390(CpuInfoS390 *info, const CPUState *cpu)
>>       CPUS390XState *env = &s390_cpu->env;
>>   
>>       info->cpu_state = env->cpu_state;
>> +    info->dedicated = env->dedicated;
>> +    info->polarity = env->entitlement;
> 
> Might want to do s/polarity/entitlement on the whole patch to make this more coherent.
> Reviewed-by: Nina Schoetterl-Glausch if you fix the issues in Thomas' first reply.
> 

OK, substitution done and Thomas changes already done.
Thanks.

Regards,
Pierre



-- 
Pierre Morel
IBM Lab Boeblingen
Re: [PATCH v15 09/11] machine: adding s390 topology to query-cpu-fast
Posted by Thomas Huth 2 years, 3 months ago
On 01/02/2023 14.20, Pierre Morel wrote:
> S390x provides two more topology containers above the sockets,
> books and drawers.
> 
> Let's add these CPU attributes to the QAPI command query-cpu-fast.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>   qapi/machine.json          | 13 ++++++++++---
>   hw/core/machine-qmp-cmds.c |  2 ++
>   2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 3036117059..e36c39e258 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -53,11 +53,18 @@
>   #
>   # Additional information about a virtual S390 CPU
>   #
> -# @cpu-state: the virtual CPU's state
> +# @cpu-state: the virtual CPU's state (since 2.12)
> +# @dedicated: the virtual CPU's dedication (since 8.0)
> +# @polarity: the virtual CPU's polarity (since 8.0)
>   #
>   # Since: 2.12
>   ##
> -{ 'struct': 'CpuInfoS390', 'data': { 'cpu-state': 'CpuS390State' } }
> +{ 'struct': 'CpuInfoS390',
> +    'data': { 'cpu-state': 'CpuS390State',
> +              'dedicated': 'bool',
> +              'polarity': 'int'

I think it would also be better to mark the new fields as optional and only 
return them if the guest has the topology enabled, to avoid confusing 
clients that were written before this change.

  Thomas
Re: [PATCH v15 09/11] machine: adding s390 topology to query-cpu-fast
Posted by Daniel P. Berrangé 2 years, 3 months ago
On Mon, Feb 06, 2023 at 01:41:44PM +0100, Thomas Huth wrote:
> On 01/02/2023 14.20, Pierre Morel wrote:
> > S390x provides two more topology containers above the sockets,
> > books and drawers.
> > 
> > Let's add these CPU attributes to the QAPI command query-cpu-fast.
> > 
> > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> > ---
> >   qapi/machine.json          | 13 ++++++++++---
> >   hw/core/machine-qmp-cmds.c |  2 ++
> >   2 files changed, 12 insertions(+), 3 deletions(-)
> > 
> > diff --git a/qapi/machine.json b/qapi/machine.json
> > index 3036117059..e36c39e258 100644
> > --- a/qapi/machine.json
> > +++ b/qapi/machine.json
> > @@ -53,11 +53,18 @@
> >   #
> >   # Additional information about a virtual S390 CPU
> >   #
> > -# @cpu-state: the virtual CPU's state
> > +# @cpu-state: the virtual CPU's state (since 2.12)
> > +# @dedicated: the virtual CPU's dedication (since 8.0)
> > +# @polarity: the virtual CPU's polarity (since 8.0)
> >   #
> >   # Since: 2.12
> >   ##
> > -{ 'struct': 'CpuInfoS390', 'data': { 'cpu-state': 'CpuS390State' } }
> > +{ 'struct': 'CpuInfoS390',
> > +    'data': { 'cpu-state': 'CpuS390State',
> > +              'dedicated': 'bool',
> > +              'polarity': 'int'
> 
> I think it would also be better to mark the new fields as optional and only
> return them if the guest has the topology enabled, to avoid confusing
> clients that were written before this change.

FWIW, I would say that the general expectation of QMP clients is that
they must *always* expect new fields to appear in dicts that are
returned in QMP replies. We add new fields at will on a frequent basis.

So personally I'd keep life simple and unconditionally report the new
fields.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [PATCH v15 09/11] machine: adding s390 topology to query-cpu-fast
Posted by Pierre Morel 2 years, 3 months ago

On 2/6/23 13:49, Daniel P. Berrangé wrote:
> On Mon, Feb 06, 2023 at 01:41:44PM +0100, Thomas Huth wrote:
>> On 01/02/2023 14.20, Pierre Morel wrote:
>>> S390x provides two more topology containers above the sockets,
>>> books and drawers.
>>>
>>> Let's add these CPU attributes to the QAPI command query-cpu-fast.
>>>
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> ---
>>>    qapi/machine.json          | 13 ++++++++++---
>>>    hw/core/machine-qmp-cmds.c |  2 ++
>>>    2 files changed, 12 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/qapi/machine.json b/qapi/machine.json
>>> index 3036117059..e36c39e258 100644
>>> --- a/qapi/machine.json
>>> +++ b/qapi/machine.json
>>> @@ -53,11 +53,18 @@
>>>    #
>>>    # Additional information about a virtual S390 CPU
>>>    #
>>> -# @cpu-state: the virtual CPU's state
>>> +# @cpu-state: the virtual CPU's state (since 2.12)
>>> +# @dedicated: the virtual CPU's dedication (since 8.0)
>>> +# @polarity: the virtual CPU's polarity (since 8.0)
>>>    #
>>>    # Since: 2.12
>>>    ##
>>> -{ 'struct': 'CpuInfoS390', 'data': { 'cpu-state': 'CpuS390State' } }
>>> +{ 'struct': 'CpuInfoS390',
>>> +    'data': { 'cpu-state': 'CpuS390State',
>>> +              'dedicated': 'bool',
>>> +              'polarity': 'int'
>>
>> I think it would also be better to mark the new fields as optional and only
>> return them if the guest has the topology enabled, to avoid confusing
>> clients that were written before this change.
> 
> FWIW, I would say that the general expectation of QMP clients is that
> they must *always* expect new fields to appear in dicts that are
> returned in QMP replies. We add new fields at will on a frequent basis.
> 
> So personally I'd keep life simple and unconditionally report the new
> fields.
> 
> With regards,
> Daniel

OK, thanks both of you.
I will then keep the simple way.

Regards,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen

Re: [PATCH v15 09/11] machine: adding s390 topology to query-cpu-fast
Posted by Thomas Huth 2 years, 3 months ago
On 06/02/2023 13.49, Daniel P. Berrangé wrote:
> On Mon, Feb 06, 2023 at 01:41:44PM +0100, Thomas Huth wrote:
>> On 01/02/2023 14.20, Pierre Morel wrote:
>>> S390x provides two more topology containers above the sockets,
>>> books and drawers.
>>>
>>> Let's add these CPU attributes to the QAPI command query-cpu-fast.
>>>
>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>> ---
>>>    qapi/machine.json          | 13 ++++++++++---
>>>    hw/core/machine-qmp-cmds.c |  2 ++
>>>    2 files changed, 12 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/qapi/machine.json b/qapi/machine.json
>>> index 3036117059..e36c39e258 100644
>>> --- a/qapi/machine.json
>>> +++ b/qapi/machine.json
>>> @@ -53,11 +53,18 @@
>>>    #
>>>    # Additional information about a virtual S390 CPU
>>>    #
>>> -# @cpu-state: the virtual CPU's state
>>> +# @cpu-state: the virtual CPU's state (since 2.12)
>>> +# @dedicated: the virtual CPU's dedication (since 8.0)
>>> +# @polarity: the virtual CPU's polarity (since 8.0)
>>>    #
>>>    # Since: 2.12
>>>    ##
>>> -{ 'struct': 'CpuInfoS390', 'data': { 'cpu-state': 'CpuS390State' } }
>>> +{ 'struct': 'CpuInfoS390',
>>> +    'data': { 'cpu-state': 'CpuS390State',
>>> +              'dedicated': 'bool',
>>> +              'polarity': 'int'
>>
>> I think it would also be better to mark the new fields as optional and only
>> return them if the guest has the topology enabled, to avoid confusing
>> clients that were written before this change.
> 
> FWIW, I would say that the general expectation of QMP clients is that
> they must *always* expect new fields to appear in dicts that are
> returned in QMP replies. We add new fields at will on a frequent basis.

Did we change our policy here? I slightly remember I've been told 
differently in the past ... but I can't recall where this was, it's 
certainly been a while.

So a question to the QAPI maintainers: What's the preferred handling for new 
fields nowadays in such situations?

  Thomas


Re: [PATCH v15 09/11] machine: adding s390 topology to query-cpu-fast
Posted by Daniel P. Berrangé 2 years, 3 months ago
On Mon, Feb 06, 2023 at 02:09:57PM +0100, Thomas Huth wrote:
> On 06/02/2023 13.49, Daniel P. Berrangé wrote:
> > On Mon, Feb 06, 2023 at 01:41:44PM +0100, Thomas Huth wrote:
> > > On 01/02/2023 14.20, Pierre Morel wrote:
> > > > S390x provides two more topology containers above the sockets,
> > > > books and drawers.
> > > > 
> > > > Let's add these CPU attributes to the QAPI command query-cpu-fast.
> > > > 
> > > > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> > > > ---
> > > >    qapi/machine.json          | 13 ++++++++++---
> > > >    hw/core/machine-qmp-cmds.c |  2 ++
> > > >    2 files changed, 12 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/qapi/machine.json b/qapi/machine.json
> > > > index 3036117059..e36c39e258 100644
> > > > --- a/qapi/machine.json
> > > > +++ b/qapi/machine.json
> > > > @@ -53,11 +53,18 @@
> > > >    #
> > > >    # Additional information about a virtual S390 CPU
> > > >    #
> > > > -# @cpu-state: the virtual CPU's state
> > > > +# @cpu-state: the virtual CPU's state (since 2.12)
> > > > +# @dedicated: the virtual CPU's dedication (since 8.0)
> > > > +# @polarity: the virtual CPU's polarity (since 8.0)
> > > >    #
> > > >    # Since: 2.12
> > > >    ##
> > > > -{ 'struct': 'CpuInfoS390', 'data': { 'cpu-state': 'CpuS390State' } }
> > > > +{ 'struct': 'CpuInfoS390',
> > > > +    'data': { 'cpu-state': 'CpuS390State',
> > > > +              'dedicated': 'bool',
> > > > +              'polarity': 'int'
> > > 
> > > I think it would also be better to mark the new fields as optional and only
> > > return them if the guest has the topology enabled, to avoid confusing
> > > clients that were written before this change.
> > 
> > FWIW, I would say that the general expectation of QMP clients is that
> > they must *always* expect new fields to appear in dicts that are
> > returned in QMP replies. We add new fields at will on a frequent basis.
> 
> Did we change our policy here? I slightly remember I've been told
> differently in the past ... but I can't recall where this was, it's
> certainly been a while.
> 
> So a question to the QAPI maintainers: What's the preferred handling for new
> fields nowadays in such situations?

I think you're mixing it up with policy for adding new fields to *input*
parameters. You can't add a new mandatory field to input params of a
command because no existing client will be sending that. Only optional
params are viable, without a deprecation cycle. For output parameters
such as this case, there's no compatibilty problem.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH v15 09/11] machine: adding s390 topology to query-cpu-fast
Posted by Thomas Huth 2 years, 3 months ago
On 06/02/2023 15.50, Daniel P. Berrangé wrote:
> On Mon, Feb 06, 2023 at 02:09:57PM +0100, Thomas Huth wrote:
>> On 06/02/2023 13.49, Daniel P. Berrangé wrote:
>>> On Mon, Feb 06, 2023 at 01:41:44PM +0100, Thomas Huth wrote:
>>>> On 01/02/2023 14.20, Pierre Morel wrote:
>>>>> S390x provides two more topology containers above the sockets,
>>>>> books and drawers.
>>>>>
>>>>> Let's add these CPU attributes to the QAPI command query-cpu-fast.
>>>>>
>>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>>> ---
>>>>>     qapi/machine.json          | 13 ++++++++++---
>>>>>     hw/core/machine-qmp-cmds.c |  2 ++
>>>>>     2 files changed, 12 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/qapi/machine.json b/qapi/machine.json
>>>>> index 3036117059..e36c39e258 100644
>>>>> --- a/qapi/machine.json
>>>>> +++ b/qapi/machine.json
>>>>> @@ -53,11 +53,18 @@
>>>>>     #
>>>>>     # Additional information about a virtual S390 CPU
>>>>>     #
>>>>> -# @cpu-state: the virtual CPU's state
>>>>> +# @cpu-state: the virtual CPU's state (since 2.12)
>>>>> +# @dedicated: the virtual CPU's dedication (since 8.0)
>>>>> +# @polarity: the virtual CPU's polarity (since 8.0)
>>>>>     #
>>>>>     # Since: 2.12
>>>>>     ##
>>>>> -{ 'struct': 'CpuInfoS390', 'data': { 'cpu-state': 'CpuS390State' } }
>>>>> +{ 'struct': 'CpuInfoS390',
>>>>> +    'data': { 'cpu-state': 'CpuS390State',
>>>>> +              'dedicated': 'bool',
>>>>> +              'polarity': 'int'
>>>>
>>>> I think it would also be better to mark the new fields as optional and only
>>>> return them if the guest has the topology enabled, to avoid confusing
>>>> clients that were written before this change.
>>>
>>> FWIW, I would say that the general expectation of QMP clients is that
>>> they must *always* expect new fields to appear in dicts that are
>>> returned in QMP replies. We add new fields at will on a frequent basis.
>>
>> Did we change our policy here? I slightly remember I've been told
>> differently in the past ... but I can't recall where this was, it's
>> certainly been a while.
>>
>> So a question to the QAPI maintainers: What's the preferred handling for new
>> fields nowadays in such situations?
> 
> I think you're mixing it up with policy for adding new fields to *input*
> parameters. You can't add a new mandatory field to input params of a
> command because no existing client will be sending that. Only optional
> params are viable, without a deprecation cycle. For output parameters
> such as this case, there's no compatibilty problem.

Ah, right, I likely mixed it up with that. Thanks for the clarification!

  Thomas



Re: [PATCH v15 09/11] machine: adding s390 topology to query-cpu-fast
Posted by Thomas Huth 2 years, 3 months ago
On 01/02/2023 14.20, Pierre Morel wrote:
> S390x provides two more topology containers above the sockets,
> books and drawers.

books and drawers are already handled via the entries in 
CpuInstanceProperties, so this sentence looks like a wrong leftover now?

I'd suggest talking about "dedication" and "polarity" instead?

> Let's add these CPU attributes to the QAPI command query-cpu-fast.
> 
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>   qapi/machine.json          | 13 ++++++++++---
>   hw/core/machine-qmp-cmds.c |  2 ++
>   2 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 3036117059..e36c39e258 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -53,11 +53,18 @@
>   #
>   # Additional information about a virtual S390 CPU
>   #
> -# @cpu-state: the virtual CPU's state
> +# @cpu-state: the virtual CPU's state (since 2.12)
> +# @dedicated: the virtual CPU's dedication (since 8.0)
> +# @polarity: the virtual CPU's polarity (since 8.0)
>   #
>   # Since: 2.12
>   ##
> -{ 'struct': 'CpuInfoS390', 'data': { 'cpu-state': 'CpuS390State' } }
> +{ 'struct': 'CpuInfoS390',
> +    'data': { 'cpu-state': 'CpuS390State',
> +              'dedicated': 'bool',
> +              'polarity': 'int'
> +    }
> +}
>   
>   ##
>   # @CpuInfoFast:
> @@ -70,7 +77,7 @@
>   #
>   # @thread-id: ID of the underlying host thread
>   #
> -# @props: properties describing to which node/socket/core/thread
> +# @props: properties describing to which node/drawer/book/socket/core/thread

I think this hunk should rather be moved to patch 1 now.

  Thomas
Re: [PATCH v15 09/11] machine: adding s390 topology to query-cpu-fast
Posted by Pierre Morel 2 years, 3 months ago

On 2/6/23 13:38, Thomas Huth wrote:
> On 01/02/2023 14.20, Pierre Morel wrote:
>> S390x provides two more topology containers above the sockets,
>> books and drawers.
> 
> books and drawers are already handled via the entries in 
> CpuInstanceProperties, so this sentence looks like a wrong leftover now?

yes it is

> 
> I'd suggest talking about "dedication" and "polarity" instead?

OK.

> 
>> Let's add these CPU attributes to the QAPI command query-cpu-fast.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>>   qapi/machine.json          | 13 ++++++++++---
>>   hw/core/machine-qmp-cmds.c |  2 ++
>>   2 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/qapi/machine.json b/qapi/machine.json
>> index 3036117059..e36c39e258 100644
>> --- a/qapi/machine.json
>> +++ b/qapi/machine.json
>> @@ -53,11 +53,18 @@
>>   #
>>   # Additional information about a virtual S390 CPU
>>   #
>> -# @cpu-state: the virtual CPU's state
>> +# @cpu-state: the virtual CPU's state (since 2.12)
>> +# @dedicated: the virtual CPU's dedication (since 8.0)
>> +# @polarity: the virtual CPU's polarity (since 8.0)
>>   #
>>   # Since: 2.12
>>   ##
>> -{ 'struct': 'CpuInfoS390', 'data': { 'cpu-state': 'CpuS390State' } }
>> +{ 'struct': 'CpuInfoS390',
>> +    'data': { 'cpu-state': 'CpuS390State',
>> +              'dedicated': 'bool',
>> +              'polarity': 'int'
>> +    }
>> +}
>>   ##
>>   # @CpuInfoFast:
>> @@ -70,7 +77,7 @@
>>   #
>>   # @thread-id: ID of the underlying host thread
>>   #
>> -# @props: properties describing to which node/socket/core/thread
>> +# @props: properties describing to which 
>> node/drawer/book/socket/core/thread
> 
> I think this hunk should rather be moved to patch 1 now.

yes it should.
Thanks.

Regards,
Pierre

-- 
Pierre Morel
IBM Lab Boeblingen