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

[Nina Schoetterl-Glausch]

On Wed, 2025-01-15 at 14:31 +0100, Shalini Chellathurai Saroja wrote:
> This commit saves the state of CPI values in the guest and
> transfers this state during live migration of the guest.

IMO, using active voice and directly stating what is done is preferable.

Something like:
Register Control-Program Identifier data with the migration infrastructure.

"This commit" doesn't really add anything.
(Applies to other commits also)
I also prefer verbosity when it comes to abbreviations.

> 
> Signed-off-by: Shalini Chellathurai Saroja <shalini@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 35fb523af9..8fe0c5c1cb 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -259,6 +259,20 @@ static void s390_create_sclpconsole(SCLPDevice *sclp,
>      qdev_realize_and_unref(dev, ev_fac_bus, &error_fatal);
>  }
>  
> +static const VMStateDescription vmstate_cpi = {
> +    .name = "s390_cpi",
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .fields = (const VMStateField[]) {
> +        VMSTATE_UINT8_ARRAY(system_type, Cpi, 8),
> +        VMSTATE_UINT8_ARRAY(system_name, Cpi, 8),
> +        VMSTATE_UINT64(system_level, Cpi),
> +        VMSTATE_UINT8_ARRAY(sysplex_name, Cpi, 8),
> +        VMSTATE_UINT64(timestamp, Cpi),
> +        VMSTATE_END_OF_LIST()

I see, you need the Cpi type here.
How about naming it ControlProgramId and renaming CPI
to ControlProgramIdMsg or similar?

With that sorted out:

Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>