Re: [PATCH v5 2/3] hw/s390x: add Control-Program Identification to QOM

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

On 2025-06-05 15:16, Nina Schotterl-Glausch wrote:
> On Thu, 2025-06-05 at 13:56 +0200, Thomas Huth wrote:
>> On 05/06/2025 10.34, Nina Schoetterl-Glausch wrote:
>> > On Tue, 2025-06-03 at 15:56 +0200, Shalini Chellathurai Saroja wrote:
>> > > Add Control-Program Identification (CPI) data to the QEMU Object
>> > > Model (QOM), along with the timestamp in which the data was received
>> > > as shown below.
>> >
>> > [...]
>> > >
>> > > Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>> > > ---
>> > >   hw/s390x/sclpcpi.c                | 47 +++++++++++++++++++++++++
>> > >   include/hw/s390x/event-facility.h |  5 +++
>> > >   qapi/machine.json                 | 58 +++++++++++++++++++++++++++++++
>> > >   3 files changed, 110 insertions(+)
>> > >
>> > > diff --git a/hw/s390x/sclpcpi.c b/hw/s390x/sclpcpi.c
>> > > index 935fa87acd..ec711e2291 100644
>> > > --- a/hw/s390x/sclpcpi.c
>> > > +++ b/hw/s390x/sclpcpi.c
>> > > @@ -15,7 +15,9 @@
>> > >     */
>> >
>> > [...]
>> > >
>> > > +static void cpi_init(Object *obj)
>> > > +{
>> > > +    SCLPEventCPI *e = SCLP_EVENT_CPI(obj);
>> > > +
>> > > +    object_property_add_str(obj, "system_type", get_system_type, NULL);
>> > > +    object_property_add_str(obj, "system_name", get_system_name, NULL);
>> > > +    object_property_add_str(obj, "sysplex_name", get_sysplex_name, NULL);
>> > > +    object_property_add_uint64_ptr(obj, "system_level", &(e->system_level),
>> > > +                                   OBJ_PROP_FLAG_READ);
>> > > +    object_property_add_uint64_ptr(obj, "timestamp", &(e->timestamp),
>> > > +                                   OBJ_PROP_FLAG_READ);
>> > > +}
>> >
>> > I think it would be cleaner if those were class properties.
>> > You could use object_class_property_add_str in cpi_class_init,
>> > but I think it'd be nice to use DEFINE_PROP_(STR|UINT64) and
>> > device_class_set_props.
>> 
>> For "normal" properties I'd say "yes" ... but in this case, this would 
>> also
>> allow the user to set the properties from the host side - which would 
>> be a
>> little bit weird? So I think it might be cleaner to keep it this way 
>> here
>> without the "setter" functions? WDYT?
> 
> Oh, yeah I didn't consider this.
> Still, object_class_property_add_str would be better I think, all 
> instances
> of the class should have these properties.
> 
Hello Nina,

Ok, I will use object_class_property_add_str and 
object_class_property_add methods instead.
Thank you.
> 
> [...]

-- 
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