On Tue, 5 Jul 2022 at 07:48, Mark Cave-Ayland
<mark.cave-ayl...@ilande.co.uk> wrote:
>
> On 04/07/2022 14:38, Peter Maydell wrote:
>
> > On Wed, 29 Jun 2022 at 13:41, Mark Cave-Ayland
> > <mark.cave-ayl...@ilande.co.uk> wrote:
> >>
> >> Since this series has already introduced a migration break for the HPPA
> >> B160L
> >> machine, we can use this opportunity to improve the VMStateDescription for
> >> the LASIPS2 device.
> >>
> >> Add the new int_status field to the VMStateDescription and remodel the
> >> ports
> >> as separate VMSTATE_STRUCT instances.
> >>
> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
> >> ---
> >> hw/input/lasips2.c | 25 +++++++++++++++++++------
> >> 1 file changed, 19 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/hw/input/lasips2.c b/hw/input/lasips2.c
> >> index e602e3c986..ea7c07a2ba 100644
> >> --- a/hw/input/lasips2.c
> >> +++ b/hw/input/lasips2.c
> >> @@ -35,15 +35,28 @@
> >> #include "qapi/error.h"
> >>
> >>
> >> +static const VMStateDescription vmstate_lasips2_port = {
> >> + .name = "lasips2-port",
> >> + .version_id = 1,
> >> + .minimum_version_id = 1,
> >> + .fields = (VMStateField[]) {
> >> + VMSTATE_UINT8(control, LASIPS2Port),
> >> + VMSTATE_UINT8(buf, LASIPS2Port),
> >> + VMSTATE_BOOL(loopback_rbne, LASIPS2Port),
> >> + VMSTATE_END_OF_LIST()
> >> + }
> >> +};
> >> +
> >> static const VMStateDescription vmstate_lasips2 = {
> >> .name = "lasips2",
> >> - .version_id = 0,
> >> - .minimum_version_id = 0,
> >> + .version_id = 1,
> >> + .minimum_version_id = 1,
> >> .fields = (VMStateField[]) {
> >> - VMSTATE_UINT8(kbd_port.parent_obj.control, LASIPS2State),
> >> - VMSTATE_UINT8(kbd_port.parent_obj.id, LASIPS2State),
> >> - VMSTATE_UINT8(mouse_port.parent_obj.control, LASIPS2State),
> >> - VMSTATE_UINT8(mouse_port.parent_obj.id, LASIPS2State),
> >> + VMSTATE_UINT8(int_status, LASIPS2State),
> >> + VMSTATE_STRUCT(kbd_port.parent_obj, LASIPS2State, 1,
> >> + vmstate_lasips2_port, LASIPS2Port),
> >> + VMSTATE_STRUCT(mouse_port.parent_obj, LASIPS2State, 1,
> >> + vmstate_lasips2_port, LASIPS2Port),
> >> VMSTATE_END_OF_LIST()
> >> }
> >> };
> >
> > The set of things we were migrating and the set of
> > things we now migrate don't seem to line up ?
>
> Yeah I should probably have documented this better in the commit message. The
> existing VMStateDescription isn't correct since it misses both the buf and
> loopback_rbne fields which are accessed across port reads and writes, and the
> port id
> is not required because it is hardcoded to 0 for the keyboard port and 1 for
> the
> mouse port.
>
> Rather than have the extra code churn throughout the series, I went for doing
> the
> minimum to vmstate_lasips2 to make the patch compile and then fix up
> everything
> (including add the new int_status bitmap) in this one patch. I think this is
> fine
> since as we're introducing a migration break in the series, there are no
> concerns
> over backward compatibility.
>
> Would an updated description for this commit message be sufficient? A quick
> skim over
> the emails indicates that the only queries during review were related to the
> handling
> of the VMStateDescription.
Yeah, if you comment this in the relevant commit messages that should be
good enough.
-- PMM
