| Message ID | 20250115133106.3034445-3-shalini@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,1/3] hw/s390x: add SCLP event type CPI | expand |
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>
On 2025-01-24 19:09, Nina Schoetterl-Glausch wrote: > 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> Hello Nina, Thank you for your comments. I have implemented all the changes according to your feedback and I have sent the version 2 of my patch series.
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() + } +}; + static void ccw_init(MachineState *machine) { MachineClass *mc = MACHINE_GET_CLASS(machine); @@ -307,6 +321,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_cpi, &ms->cpi); + css_register_vmstate(); /* Create VirtIO network adapters */
This commit saves the state of CPI values in the guest and transfers this state during live migration of the guest. Signed-off-by: Shalini Chellathurai Saroja <shalini@linux.ibm.com> --- hw/s390x/s390-virtio-ccw.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)