Apologies for the delay. * Marc-André Lureau (marcandre.lur...@gmail.com) wrote: > Hi > > On Sun, Dec 1, 2019 at 10:10 PM Peter Maydell <peter.mayd...@linaro.org> > wrote: > > > > On Sun, 1 Dec 2019 at 17:27, Marc-André Lureau > > <marcandre.lur...@gmail.com> wrote: > > > > > > Hi > > > > > > On Sun, Dec 1, 2019 at 9:18 PM Peter Maydell <peter.mayd...@linaro.org> > > > wrote: > > > > > > > > On Sun, 1 Dec 2019 at 10:19, Marc-André Lureau > > > > <marcandre.lur...@gmail.com> 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?
Yes, I think it is. However, I'm curious which devices you're finding are explicitly setting their id's; there aren't many - although there are some that probably should! For example, running an x86 image with: -device isa-parallel,chardev=... -device isa-serial -device isa-serial -trace enable=qemu_loadvm_state_section_startfull shows: qemu_loadvm_state_section_startfull(uint32_t section_id, const char *idstr, uint32_t instance_id, uint32_t version_id) "%u(%s) %u %u" 165217@1576179638.856300:qemu_loadvm_state_section_startfull 41(serial) 0 3 165217@1576179638.856307:qemu_loadvm_state_section_startfull 42(serial) 1 3 165217@1576179638.856311:qemu_loadvm_state_section_startfull 43(parallel_isa) 0 1 so those two serial devices are instances '0' and '1' I think by luck of their command line order, rather than having specified their base address (which would have been safer). Dave > thanks > > > -- > Marc-André Lureau > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK