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

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

On 2025-03-06 16:06, Daniel P. Berrangé wrote:
> BTW, your email client is possibly mis-configured - your mail
> came through with "From: shalini <shalini@imap.linux.ibm.com>"
> and attempting to reply to that gets an error saying that
> 'imap.linux.ibm.com' does not exist.
> 

Hello Daniel,
Thank you very much for pointing this out. I have fixed it.

> On Thu, Mar 06, 2025 at 02:55:27PM +0100, shalini wrote:
>> On 2025-03-05 17:06, Daniel P. Berrangé wrote:
>> > On Mon, Feb 24, 2025 at 01:04:47PM +0100, 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 @@
>> > >  #include "hw/s390x/virtio-ccw-md.h"
>> > >  #include "system/replay.h"
>> > >  #include CONFIG_DEVICES
>> > > +#include "qapi/qapi-visit-machine.h"
>> > >
>> > >  static Error *pv_mig_blocker;
>> > >
>> > > @@ -803,6 +804,26 @@ static void machine_set_loadparm(Object *obj,
>> > > Visitor *v,
>> > >      s390_ipl_fmt_loadparm(ms->loadparm, val, errp);
>> > >  }
>> > >
>> > > +static void machine_get_control_program_id(Object *obj, Visitor *v,
>> > > +                                           const char *name, void
>> > > *opaque,
>> > > +                                           Error **errp)
>> > > +{
>> > > +    S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
>> > > +    S390ControlProgramId *cpi;
>> > > +    cpi = &(S390ControlProgramId){
>> > > +        .system_type = g_strndup((char *) ms->cpi.system_type,
>> > > +                       sizeof(ms->cpi.system_type)),
>> > > +        .system_name = g_strndup((char *) ms->cpi.system_name,
>> > > +                       sizeof(ms->cpi.system_name)),
>> > > +        .system_level = g_strdup_printf("0x%lx",
>> > > ms->cpi.system_level),
>> >
>> > If the data is an integer, we must return it in QMP as an integer,
>> > not formatted into a hex string.
>> >
>> 
>> Hello Daniel,
>> 
>> Thank you very much for the review.
>> 
>> The system-level, when interpreted as an int provides the output below
>> 
>> 'system-level': 74872343805430528
> 
> Yes, that is correct from a QMP design POV. Data should formatted
> in the most appropriate way for machines to consume, using native
> JSON data types.
> 
>> But the desired output below is obtained only when interpreted as a 
>> str.
>> 
>> 'system-level': '0x10a000000060b00'
> 
> If a human wants to read the data in hex format, that should be
> formatted by whatever tool is consuming the data from QMP and
> presenting it in the user.
> 

Thank you for the explanation. I will change this.

>> > > +# @system-name: system name of Linux instance
>> >
>> > What is a system name ?  Is that a hostname, or is that something
>> > else ?
>> >
>> 
>> Yes, it is the hostname of the guest virtual machine.
> 
> Lets rename it to 'system-hostname' to be unambiguous.
> 

I was wrong. System name is not the hostname of the guest virtual 
machine. As mentioned by Nina it is a user configurable name of the 
system. So system-name cannot be change to 'system-hostname'. I am sorry 
for the inconvenience. 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