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

[Thomas Huth <thuth@redhat.com>]

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?

  Thomas