Re: [PATCH v4 00/37] Clean-ups: qom-ify serial and remove QDEV_PROP_PTR

[Marc-André Lureau]

Hi

On Sun, Dec 1, 2019 at 10:10 PM Peter Maydell <address@hidden> wrote:
>
> On Sun, 1 Dec 2019 at 17:27, Marc-André Lureau
> <address@hidden> wrote:
> >
> > Hi
> >
> > On Sun, Dec 1, 2019 at 9:18 PM Peter Maydell <address@hidden> wrote:
> > >
> > > On Sun, 1 Dec 2019 at 10:19, Marc-André Lureau
> > > <address@hidden> wrote:
> > > >
> > > > - "serial: register vmsd with DeviceClass"
> > > >
> > > > This is standard qdev-ification, however it breaks backward migration,
> > > > but that's just how qdev_set_legacy_instance_id() works.
> > >
> > > I don't understand this part. Surely the whole point
> > > of setting a legacy instance ID is exactly to preserve
> > > migration compatibility? If it doesn't do that then what
> > > does setting legacy ID value do?
> > >
> >
> > It works in old->new direction only, because new code can match the
> > legacy instance id.
> >
> > But when going from new->old, the legacy instance id is lost, as it
> > uses new 0-based instance_id.
>
> I still don't understand. My mental model of the situation is:
>
>  * in the old (current) version of the code, the instance ID
>    is some random thing resulting from what the old code does

right

>  * in the new version of the code, we use qdev_set_legacy_instance_id,
>    and so instead of using the ID you'd naturally get as a
>    written-from-scratch qdev device, it uses the legacy value
>    you pass in

no, it only sets the SaveStateEntry.alias_id, which is only used
during incoming migration in find_se().

Iow, it only works old->new.

>  * thus the device/board in both old and new versions of QEMU
>    uses the same value and migration in both directions works

sadly no

>
> I don't understand why we would ever be using a "new 0-based
> instance_id" -- it seems to me that the whole point of setting
> a legacy ID value is that we will use it always, and I don't
> understand how the board code can know that it's going to be
> the target of an old->new migration as opposed to being the
> source of a new->old migration such that it can end up with
> a different ID value in the latter case.

The target will find the "legacy" alias with find_se() on incoming
migration, but any new outgoing migration will use the new 0-based
instance_id

>
> If qdev_set_legacy_instance_id() doesn't work the way I
> think it does above, what *does* it do ?

just set the old alias_id for incoming migration.

David, is that correct?

thanks


-- 
Marc-André Lureau