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

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

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.
> 
> Additionally, I think you need some compat handling for backward
> migration in your patches. E.g. have you tried migrating from an old
> version of QEMU to a newer one (that includes your patches) and then
> back to the old one?
> I think the TYPE_SCLP_CPI device should only be instantiated for the
> machine types >= 10.0, but not for the older machine types, e.g. by
> introducing a "use-cpi" property to the TYPE_SCLP_EVENT_FACILITY (set
> to true by default). Then in ccw_machine_9_2_class_options(), make
> sure that this property gets switched to "off" again, so that older
> machine types don't have the new TYPE_SCLP_CPI device. WDYT?
> 
Hello Thomas,
That is correct. I will change this as per your suggestion. Thank you.

>  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