On Wed, Mar 17, 2021 at 11:14:56AM +0000, Peter Maydell wrote: > On Wed, 17 Mar 2021 at 10:59, Gavin Shan <gs...@redhat.com> wrote: > > > > Hi Peter, > > > > On 3/17/21 9:40 PM, Peter Maydell wrote: > > > On Wed, 17 Mar 2021 at 10:37, Gavin Shan <gs...@redhat.com> wrote: > > >> On 3/17/21 8:09 PM, Peter Maydell wrote: > > >>> On Wed, 17 Mar 2021 at 04:44, Gavin Shan <gs...@redhat.com> wrote: > > >>>> > > >>>> static const VMStateDescription vmstate_pl011 = { > > >>>> .name = "pl011", > > >>>> .version_id = 2, > > >>>> .minimum_version_id = 2, > > >>>> + .post_load = pl011_post_load, > > >>>> .fields = (VMStateField[]) { > > >>>> VMSTATE_UINT32(readbuff, PL011State), > > >>>> VMSTATE_UINT32(flags, PL011State), > > >>>> @@ -355,10 +355,6 @@ static const VMStateDescription vmstate_pl011 = { > > >>>> VMSTATE_INT32(read_trigger, PL011State), > > >>>> VMSTATE_END_OF_LIST() > > >>>> }, > > >>>> - .subsections = (const VMStateDescription * []) { > > >>>> - &vmstate_pl011_clock, > > >>>> - NULL > > >>>> - } > > >>>> }; > > >>> > > >>> Doesn't dropping the subsection break migration compat ? > > >>> > > >> > > >> It's why this patch needs to be backported to stable branches. > > >> In that way, we won't have migration compatible issue. > > > > > > No, migration has to work from the existing already > > > shipped 5.1, 5.2, etc releases to 6.0 (assuming you use > > > the correct "virt-5.2" &c versioned machine type.) > > > > > > > Commit aac63e0e6ea3 ("hw/char/pl011: add a clock input") is merged > > to v5.2.0. The migration failure happens during migration from v6.0 > > to v5.1 with machine type as "virt-5.1", instead of migrating from > > v5.1 to v6.0. One question is if we need support backwards migration? > > Upstream doesn't care about backwards migration. AIUI > RedHat as a downstream care about the backwards-migration > case in some specific situations, but I don't know if that > would include this one.
Right, we do prefer to be able to support "ping-pong" migrations. For example, if we start a virt-5.1 machine on a 5.1 build of QEMU, and then migrate it to a 5.2 build of QEMU, we'd like to also be able to go back to the 5.1 build. I agree this patch is not the right approach. I think the right approach is to introduce a compat property and make the "new" section dependent on it. And then update the hw_compat_* arrays. Gavin, please take a look at "Connecting subsections to properties" of docs/devel/migration.rst. I'm also curious what the state of mach-virt's machine types are for migration. It'd be nice to exhaustively test both forward migration of all machine types and ping-pong migrations of all machine types. We can then consider each issue we find (the pessimist in me suggests we'll find more than this pl011 issue) and how/if we want to resolve them. Thanks, drew