Hi On Fri, Dec 13, 2019 at 12:18 AM Dr. David Alan Gilbert <dgilb...@redhat.com> wrote: > > 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).
None of the qdev devices using vmsd use a hard-coded instance id afaik. In device_set_realized(), vmstate_register_with_alias_id() is called with instance_id = -1, so it relies on calculate_new_instance_id(se->idstr) > > Dave > > > > > thanks > > > > > > -- > > Marc-André Lureau > > > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK > -- Marc-André Lureau
