Message ID | 20250224120449.1764114-1-shalini@linux.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [qemu,v2,1/3] hw/s390x: add CPI identifiers to QOM | expand |
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 @@ > #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), > + .sysplex_name = g_strndup((char *) ms->cpi.sysplex_name, > + sizeof(ms->cpi.sysplex_name)), > + .timestamp = ms->cpi.timestamp > + }; Could you please indend the sizeof() lines with the "(" after the g_strndup in the previous line? > + > + visit_type_S390ControlProgramId(v, name, &cpi, &error_abort); > +} > + > static void ccw_machine_class_init(ObjectClass *oc, void *data) > { > MachineClass *mc = MACHINE_CLASS(oc); > @@ -854,6 +875,14 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) > "Up to 8 chars in set of [A-Za-z0-9. ] (lower case chars converted" > " to upper case) to pass to machine loader, boot manager," > " and guest kernel"); > + object_class_property_add(oc, "s390-control-program-id", I think I'd rather drop the "s390-" prefix here. The property is already part of the s390-virtio-ccw machine, so it should be obvious that this is related to s390. > + "S390ControlProgramId", > + machine_get_control_program_id, > + NULL, NULL, NULL); > + object_class_property_set_description(oc, "s390-control-program-id", > + "Control-progam identifiers provide data about the guest " s/progam/program/ > + "operating system"); > + > } > > static inline void s390_machine_initfn(Object *obj) > diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h > index 686d9497d2..6872f7a176 100644 > --- a/include/hw/s390x/s390-virtio-ccw.h > +++ b/include/hw/s390x/s390-virtio-ccw.h > @@ -19,6 +19,13 @@ > > OBJECT_DECLARE_TYPE(S390CcwMachineState, S390CcwMachineClass, S390_CCW_MACHINE) > > +typedef struct ControlProgramId { > + uint8_t system_type[8]; > + uint8_t system_name[8]; > + uint64_t system_level; > + uint8_t sysplex_name[8]; > + uint64_t timestamp; > +} QEMU_PACKED ControlProgramId; > > struct S390CcwMachineState { > /*< private >*/ > @@ -33,6 +40,7 @@ struct S390CcwMachineState { > uint64_t max_pagesize; > > SCLPDevice *sclp; > + ControlProgramId cpi; > }; > > static inline uint64_t s390_get_memory_limit(S390CcwMachineState *s390ms) > diff --git a/qapi/machine.json b/qapi/machine.json > index a6b8795b09..c6cbad87e1 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -1898,3 +1898,27 @@ > { 'command': 'x-query-interrupt-controllers', > 'returns': 'HumanReadableText', > 'features': [ 'unstable' ]} > + > +## > +# @S390ControlProgramId: > +# > +# Control-program identifiers provide data about Linux instance. If I understood correctly, this could also theoretically be used by other guest operating systems? If so, please replace "Linux instance" with "guest operating system". > +# > +# @system-type: operating system of Linux instance Replace with: @system-type: operating system (e.g. "LINUX") ? > +# > +# @system-name: system name of Linux instance Name of the VM instance ? > +# @system-level: distribution and kernel version of Linux instance > +# > +# @sysplex-name: sysplex name of Linux instance > +# > +# @timestamp: latest update of CPI data > +# > +# Since: 9.2 9.2 has already been released, so this should be 10.0. > +## > +{ '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. Thomas > + 'sysplex-name': 'str', > + 'timestamp': 'uint64' } }
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. > + .sysplex_name = g_strndup((char *) ms->cpi.sysplex_name, > + sizeof(ms->cpi.sysplex_name)), > + .timestamp = ms->cpi.timestamp > + }; > +## > +# @S390ControlProgramId: > +# > +# Control-program identifiers provide data about Linux instance. > +# > +# @system-type: operating system of Linux instance Is there a list of well known operating system names, or is this arbitrary free-form text. Needs to be documented. > +# @system-name: system name of Linux instance What is a system name ? Is that a hostname, or is that something else ? > +# > +# @system-level: distribution and kernel version of Linux instance What does this actually mean ? This is a single field, but the docs are describing 2 distinct versions. Even a single version usually has multiple digits and even a string suffix. This needs to document how the version information actually encoded in practice, and if some info is discarded in this process. > +# > +# @sysplex-name: sysplex name of Linux instance I guess "sysplex" is some term people from s390 world would already understand ? It is possible to explain it or do we just have to assume prior knowledge ? > +# > +# @timestamp: latest update of CPI data In what units and epoch ? Seconds since the UNIX epoch, or something else ? > +# > +# Since: 9.2 > +## > +{ 'struct': 'S390ControlProgramId', 'data': { > + 'system-type': 'str', > + 'system-name': 'str', > + 'system-level': 'str', > + 'sysplex-name': 'str', > + 'timestamp': 'uint64' } } > -- > 2.47.0 > > With regards, Daniel
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 @@ > #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), > + .sysplex_name = g_strndup((char *) ms->cpi.sysplex_name, > + sizeof(ms->cpi.sysplex_name)), > + .timestamp = ms->cpi.timestamp > + }; > + > + visit_type_S390ControlProgramId(v, name, &cpi, &error_abort); Forgot to say: Please use errp here instead -----------^ Thanks! Thomas
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 @@ >> #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), >> + .sysplex_name = g_strndup((char *) ms->cpi.sysplex_name, >> + sizeof(ms->cpi.sysplex_name)), >> + .timestamp = ms->cpi.timestamp >> + }; > > Could you please indend the sizeof() lines with the "(" after the > g_strndup in the previous line? > Hello Thomas, Sure, I have provided a sample code below, please let me know if this is incorrect. Thank you. >> + cpi = &(S390ControlProgramId){ >> + .system_type = g_strndup((char *) ms->cpi.system_type, >> + sizeof(ms->cpi.system_type)), >> + >> + visit_type_S390ControlProgramId(v, name, &cpi, &error_abort); >> +} >> + >> static void ccw_machine_class_init(ObjectClass *oc, void *data) >> { >> MachineClass *mc = MACHINE_CLASS(oc); >> @@ -854,6 +875,14 @@ static void ccw_machine_class_init(ObjectClass >> *oc, void *data) >> "Up to 8 chars in set of [A-Za-z0-9. ] (lower case chars >> converted" >> " to upper case) to pass to machine loader, boot >> manager," >> " and guest kernel"); >> + object_class_property_add(oc, "s390-control-program-id", > > I think I'd rather drop the "s390-" prefix here. The property is > already part of the s390-virtio-ccw machine, so it should be obvious > that this is related to s390. ok. > >> + "S390ControlProgramId", >> + machine_get_control_program_id, >> + NULL, NULL, NULL); >> + object_class_property_set_description(oc, >> "s390-control-program-id", >> + "Control-progam identifiers provide data about the guest " > > s/progam/program/ > ok. >> + "operating system"); >> + >> } >> static inline void s390_machine_initfn(Object *obj) [...] >> diff --git a/qapi/machine.json b/qapi/machine.json >> index a6b8795b09..c6cbad87e1 100644 >> --- a/qapi/machine.json >> +++ b/qapi/machine.json >> @@ -1898,3 +1898,27 @@ >> { 'command': 'x-query-interrupt-controllers', >> 'returns': 'HumanReadableText', >> 'features': [ 'unstable' ]} >> + >> +## >> +# @S390ControlProgramId: >> +# >> +# Control-program identifiers provide data about Linux instance. > > If I understood correctly, this could also theoretically be used by > other guest operating systems? If so, please replace "Linux instance" > with "guest operating system". > Yes, that is correct. I will change the description of the attributes below based on the comments from you and Daniel, thank you. >> +# >> +# @system-type: operating system of Linux instance > > Replace with: > > @system-type: operating system (e.g. "LINUX") > > ? > >> +# >> +# @system-name: system name of Linux instance > > Name of the VM instance ? > >> +# @system-level: distribution and kernel version of Linux instance >> +# >> +# @sysplex-name: sysplex name of Linux instance >> +# >> +# @timestamp: latest update of CPI data >> +# >> +# Since: 9.2 > > 9.2 has already been released, so this should be 10.0. > ok. >> +## >> +{ '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' > Thomas > > >> + 'sysplex-name': 'str', >> + 'timestamp': 'uint64' } }
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 But the desired output below is obtained only when interpreted as a str. 'system-level': '0x10a000000060b00' >> + .sysplex_name = g_strndup((char *) ms->cpi.sysplex_name, >> + sizeof(ms->cpi.sysplex_name)), >> + .timestamp = ms->cpi.timestamp >> + }; > >> +## >> +# @S390ControlProgramId: >> +# >> +# Control-program identifiers provide data about Linux instance. >> +# >> +# @system-type: operating system of Linux instance > > Is there a list of well known operating system names, or is > this arbitrary free-form text. Needs to be documented. > ok. > >> +# @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. >> +# >> +# @system-level: distribution and kernel version of Linux instance > > What does this actually mean ? This is a single field, but the docs > are describing 2 distinct versions. Even a single version usually > has multiple digits and even a string suffix. This needs to document > how the version information actually encoded in practice, and if > some info is discarded in this process. > Please refer https://www.ibm.com/docs/en/linux-on-systems?topic=identification-system-level for information on system-level. I will document this. >> +# >> +# @sysplex-name: sysplex name of Linux instance > > I guess "sysplex" is some term people from s390 world would > already understand ? It is possible to explain it or do we just > have to assume prior knowledge ? > Sysplex name - Sysplex refers to a cluster of logical partitions that communicates and co-operates with each other. Sysplex name is the name of the cluster which the guest belongs to(If any).(eg: PLEX) I will document this. >> +# >> +# @timestamp: latest update of CPI data > > In what units and epoch ? Seconds since the UNIX epoch, or > something else ? > In nanoseconds since the UNIX epoch. I will document this. I used the inbuilt method qemu_clock_get_ns() to get this epoch value. Thank you. >> +# >> +# Since: 9.2 >> +## >> +{ 'struct': 'S390ControlProgramId', 'data': { >> + 'system-type': 'str', >> + 'system-name': 'str', >> + 'system-level': 'str', >> + 'sysplex-name': 'str', >> + 'timestamp': 'uint64' } } >> -- >> 2.47.0 >> >> > > With regards, > Daniel
On 2025-03-05 19:05, 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 @@ >> #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), >> + .sysplex_name = g_strndup((char *) ms->cpi.sysplex_name, >> + sizeof(ms->cpi.sysplex_name)), >> + .timestamp = ms->cpi.timestamp >> + }; >> + >> + visit_type_S390ControlProgramId(v, name, &cpi, &error_abort); > > Forgot to say: Please use errp here instead -----------^ > Hello Thomas, ok. Thank you. > Thanks! > Thomas
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 @@ >>> #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), >>> + .sysplex_name = g_strndup((char *) ms->cpi.sysplex_name, >>> + sizeof(ms->cpi.sysplex_name)), >>> + .timestamp = ms->cpi.timestamp >>> + }; >> >> Could you please indend the sizeof() lines with the "(" after the >> g_strndup in the previous line? >> > > Hello Thomas, > > Sure, I have provided a sample code below, please let me know if this is > incorrect. Thank you. > >>> + cpi = &(S390ControlProgramId){ >>> + .system_type = g_strndup((char *) ms->cpi.system_type, >>> + sizeof(ms->cpi.system_type)), That indentation looks fine, thanks! ... >>> +## >>> +{ '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. Thanks, Thomas
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. 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. > > > +# @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. With regards, Daniel
On Thu, 2025-03-06 at 15:06 +0000, 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. > > 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(+) [...] > > > > > +# @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. Is it? Seems to me it it just a user configurable name of the system. It's empty on the two LPARs I checked. > > Lets rename it to 'system-hostname' to be unambiguous. In any case "system name" is the established name for this value on s390x. > > > With regards, > Daniel
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, ... } Not sure how to handle the 3 undefined bits in the highest byte. > > Thanks, > Thomas >
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) With regards, Daniel
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
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
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), + .sysplex_name = g_strndup((char *) ms->cpi.sysplex_name, + sizeof(ms->cpi.sysplex_name)), + .timestamp = ms->cpi.timestamp + }; + + visit_type_S390ControlProgramId(v, name, &cpi, &error_abort); +} + static void ccw_machine_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); @@ -854,6 +875,14 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) "Up to 8 chars in set of [A-Za-z0-9. ] (lower case chars converted" " to upper case) to pass to machine loader, boot manager," " and guest kernel"); + object_class_property_add(oc, "s390-control-program-id", + "S390ControlProgramId", + machine_get_control_program_id, + NULL, NULL, NULL); + object_class_property_set_description(oc, "s390-control-program-id", + "Control-progam identifiers provide data about the guest " + "operating system"); + } static inline void s390_machine_initfn(Object *obj) diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h index 686d9497d2..6872f7a176 100644 --- a/include/hw/s390x/s390-virtio-ccw.h +++ b/include/hw/s390x/s390-virtio-ccw.h @@ -19,6 +19,13 @@ OBJECT_DECLARE_TYPE(S390CcwMachineState, S390CcwMachineClass, S390_CCW_MACHINE) +typedef struct ControlProgramId { + uint8_t system_type[8]; + uint8_t system_name[8]; + uint64_t system_level; + uint8_t sysplex_name[8]; + uint64_t timestamp; +} QEMU_PACKED ControlProgramId; struct S390CcwMachineState { /*< private >*/ @@ -33,6 +40,7 @@ struct S390CcwMachineState { uint64_t max_pagesize; SCLPDevice *sclp; + ControlProgramId cpi; }; static inline uint64_t s390_get_memory_limit(S390CcwMachineState *s390ms) diff --git a/qapi/machine.json b/qapi/machine.json index a6b8795b09..c6cbad87e1 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -1898,3 +1898,27 @@ { 'command': 'x-query-interrupt-controllers', 'returns': 'HumanReadableText', 'features': [ 'unstable' ]} + +## +# @S390ControlProgramId: +# +# Control-program identifiers provide data about Linux instance. +# +# @system-type: operating system of Linux instance +# +# @system-name: system name of Linux instance +# +# @system-level: distribution and kernel version of Linux instance +# +# @sysplex-name: sysplex name of Linux instance +# +# @timestamp: latest update of CPI data +# +# Since: 9.2 +## +{ 'struct': 'S390ControlProgramId', 'data': { + 'system-type': 'str', + 'system-name': 'str', + 'system-level': 'str', + 'sysplex-name': 'str', + 'timestamp': 'uint64' } }
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(+)