Re: [PATCH qemu v2 1/3] hw/s390x: add CPI identifiers to QOM

[Shalini Chellathurai Saroja <shalini@linux.ibm.com>]

On 2025-03-06 17:05, Daniel P. Berrangé wrote:
> On Thu, Mar 06, 2025 at 04:44:33PM +0100, Nina Schoetterl-Glausch 
> wrote:
>> On Thu, 2025-03-06 at 15:55 +0100, Thomas Huth wrote:
>> > On 06/03/2025 13.23, shalini wrote:
>> > > On 2025-03-05 16:56, Thomas Huth wrote:
>> > > > On 24/02/2025 13.04, Shalini Chellathurai Saroja wrote:
>> > > > > Add Control-Program Identification (CPI) to the QEMU Object
>> > > > > Model (QOM). The CPI identifiers provide information about
>> > > > > the guest operating system. The CPI identifiers are:
>> > > > > system type, system name, system level and sysplex name.
>> > > > >
>> > > > > The system type provides the OS type of the guest (e.g. LINUX).
>> > > > > The system name provides the name of the guest (e.g. TESTVM).
>> > > > > The system level provides the distribution and kernel version
>> > > > > of the guest OS (e.g. 0x50e00).
>> > > > > The sysplex name provides the sysplex name of the guest
>> > > > > (e.g. SYSPLEX).
>> > > > >
>> > > > > Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>> > > > > ---
>> > > > >   hw/s390x/s390-virtio-ccw.c         | 29 +++++++++++++++++++++++++++++
>> > > > >   include/hw/s390x/s390-virtio-ccw.h |  8 ++++++++
>> > > > >   qapi/machine.json                  | 24 ++++++++++++++++++++++++
>> > > > >   3 files changed, 61 insertions(+)
>> > > > >
>> > > > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> > > > > index 51ae0c133d..13ea8db1b0 100644
>> > > > > --- a/hw/s390x/s390-virtio-ccw.c
>> > > > > +++ b/hw/s390x/s390-virtio-ccw.c
>> > > > > @@ -50,6 +50,7 @@
>> 
>> [...]
>> 
>> > > > > +##
>> > > > > +{ 'struct': 'S390ControlProgramId', 'data': {
>> > > > > +     'system-type': 'str',
>> > > > > +     'system-name': 'str',
>> > > > > +     'system-level': 'str',
>> > > >
>> > > > Not sure, but would it make sense to use a number for the system-level
>> > > > instead? At least it's a number in ControlProgramId, not a string.
>> > > >
>> > >
>> > > The system-level, when interpreted as an int provides the output below
>> > >
>> > > 'system-level': 74872343805430528
>> > >
>> > > But the desired output below is obtained only when interpreted as a str.
>> > > please refer https://www.ibm.com/docs/en/linux-on-systems?
>> > > topic=identification-system-level for details on system-level. I will also
>> > > document this in the description of system-level as suggested by Daniel.
>> > > Thank you.
>> > >
>> > > 'system-level': '0x10a000000060b00'
>> >
>> > Well, the idea of QOM/QAPI is: It's an *API* for machines, not meant for
>> > direct consumption by the end user. So passing an integer as a string is not
>> > the right way here. For the user, you'd require some upper level instead
>> > that renders the integer in the right shape for the user. So please don't
>> > use a string for this at the QOM/QAPI level.
>> 
>> In a sense the system level is a bitfield.
>> So this could become a struct
>> 
>> {
>> 	'hypervisor-use' : true,
>> 	'distribution-id': 3,	// or an enum?
>> 	'distribution-version-major: 24,
>> 	...
>> }
> 
> Yes, if this is a mandatory format, breaking out the fields makes 
> sense.
> 
>> Not sure how to handle the 3 undefined bits in the highest byte.
> 
> If they're always left zero, might as well just omit them until
> such time as they have semantics defined (if ever)
> 

Hello Nina, Daniel,

The semantics as suggested by the split is applicable only if the 
system-type is LINUX. So, I will just change the system_level to int 
format.

Thank you.


> With regards,
> Daniel

-- 
Mit freundlichen Grüßen / Kind regards
Shalini Chellathurai Saroja
Software Developer
Linux on IBM Z & KVM Development
IBM Deutschland Research & Development GmbH
Dept 1419, Schoenaicher Str. 220, 71032 Boeblingen
Vorsitzender des Aufsichtsrats: Wolfgang Wendt
Geschäftsführung: David Faller
Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht 
Stuttgart, HRB 243294