Re: [PATCH qemu v2 3/3] hw/s390x: support migration of CPI values

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

On 2025-03-06 16:08, Thomas Huth wrote:
> On 06/03/2025 15.10, shalini wrote:
>> On 2025-03-05 19:33, Thomas Huth wrote:
>>> On 24/02/2025 13.04, Shalini Chellathurai Saroja wrote:
>>>> Register Control-Program Identification data with the live
>>>> migration infrastructure.
>>>> 
>>>> Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com>
>>>> Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
>>>> ---
>>>>   hw/s390x/s390-virtio-ccw.c | 17 +++++++++++++++++
>>>>   1 file changed, 17 insertions(+)
>>>> 
>>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>>> index 13ea8db1b0..4d0838d037 100644
>>>> --- a/hw/s390x/s390-virtio-ccw.c
>>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>>> @@ -260,6 +260,20 @@ static void s390_create_sclpconsole(SCLPDevice 
>>>> *sclp,
>>>>       qdev_realize_and_unref(dev, ev_fac_bus, &error_fatal);
>>>>   }
>>>>   +static const VMStateDescription vmstate_control_program_id = {
>>>> +    .name = "s390_control_program_id",
>>>> +    .version_id = 0,
>>>> +    .minimum_version_id = 0,
>>>> +    .fields = (const VMStateField[]) {
>>>> +        VMSTATE_UINT8_ARRAY(system_type, ControlProgramId, 8),
>>>> +        VMSTATE_UINT8_ARRAY(system_name, ControlProgramId, 8),
>>>> +        VMSTATE_UINT64(system_level, ControlProgramId),
>>>> +        VMSTATE_UINT8_ARRAY(sysplex_name, ControlProgramId, 8),
>>>> +        VMSTATE_UINT64(timestamp, ControlProgramId),
>>>> +        VMSTATE_END_OF_LIST()
>>>> +    }
>>>> +};
>>>> +
>>>>   static void ccw_init(MachineState *machine)
>>>>   {
>>>>       MachineClass *mc = MACHINE_GET_CLASS(machine);
>>>> @@ -308,6 +322,9 @@ static void ccw_init(MachineState *machine)
>>>>       ret = css_create_css_image(VIRTUAL_CSSID, true);
>>>>       assert(ret == 0);
>>>>   +    /* register CPI values */
>>>> +    vmstate_register_any(NULL, &vmstate_control_program_id, 
>>>> &ms->cpi);
>>> 
>>>  Hi again,
>>> 
>>> after looking at this for a while, I think it might be cleaner to
>>> store the state in the TYPE_SCLP_CPI device instead of storing it in
>>> the machine state. Then you can also use dc->vmsd there instead of
>>> using the legacy vmstate_register_any() function.
>>> 
>> 
>> Hello Thomas,
>> 
>> The SCLP event type CPI is used to transfer the data mentioned below 
>> from guest operating system to QEMU. The job of the CPI event is 
>> complete, when this transfer is done.
>> 
>> The received data must be saved in the QEMU object model, so that it 
>> is possible for the user to retrieve the data via QMP. The received 
>> data provides information about the guest virtual machine.
>> 
>> System type - Operating system of the guest (eg: LINUX)
>> System name - Hostname provided to the guest (eg: TESTVM)
>> System level - Kernel version of the guest operating system (eg: 
>> 0x50e00)
>> 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)
>> 
>> In my perspective, I believe that the s390 machine state is 
>> appropriate for storing the data about the guest virtual machine. 
>> Please do let me know if you still want to change this?, thank you.
> 
>  Hi Shalini,
> 
> yes, please change this patch to put the migration state into the
> SCLP_CPI device.
> 
> vmstate_register_any() is a legacy function, see its description in
> include/migration/vmstate.h ... so if anyhow possible, this should be
> avoided in new code.
> 
> You introduced a new device in patch 2 which will be handling the
> related data. Devices are the right place for storing migration data
> in new QEMU code, so I think this really should go into dc->vmsd of
> TYPE_SCLP_CPI.
> 
> For retrieving the information, the user should then just get the
> property from /machine/sclp/s390-sclp-event-facility/sclpcpi instead
> of /machine. This should be fine since it is a well-defined location,
> too.
> 

Good Morning Thomas,

Thank you very much for the explanation. I will change this.

>  Thanks!
>   Thomas

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