On 3/17/22 13:29, Dr. David Alan Gilbert wrote:
* Peter Maydell (peter.mayd...@linaro.org) wrote:
On Thu, 17 Mar 2022 at 14:03, Daniel Henrique Barboza
<danielhb...@gmail.com> wrote:
I've been looking into converting some vmstate_register() calls to use dc->vmsd,
using as a base the docs in docs/devel/migration.rst. This doc mentions that we
can either register the vmsd by using vmstate_register() or we can use dc->vmsd
for qdev-based devices.
When trying to convert this vmstate() call for the qdev alternative
(hw/ppc/spapr_drc.c,
drc_realize()) I found this:
vmstate_register(VMSTATE_IF(drc), spapr_drc_index(drc),
&vmstate_spapr_drc,
drc);
spapr_drc_index() is an unique identifier for these DRC devices and it's being
used
as instance_id. It is not clear to me how we can keep using this same
instance_id when
using the dc->vmsd alternative. By looking a bit into migration files I
understood
that if dc->vmsd is being used the instance_id is always autogenerated. Is that
correct?
Not entirely. It is the intended common setup, but because changing
the ID value breaks migration compatibility there is a mechanism
for saying "my device is special and needs to set the instance ID
to something else" -- qdev_set_legacy_instance_id().
Yes, this is normally only an issue for 'system' or memory mapped
devices; for things hung off a bus that has it's own device naming,
then each instance of a device has it's own device due to the bus name
so instance_id's aren't used. Where you've got a few of the
same device with the same name, and no bus for them to be named by, then
the instance_id is used to uniquify them.
(long reply inc)
So, qdev_set_legacy_instance_id() doesn't set 'instance_id' as I've expected
but rather
'alias_id'. The function will set dev->instance_id_alias, which is then used in
device_set_realized()
as follows:
if (qdev_get_vmsd(dev)) {
if (vmstate_register_with_alias_id(VMSTATE_IF(dev),
VMSTATE_INSTANCE_ID_ANY,
qdev_get_vmsd(dev), dev,
dev->instance_id_alias,
dev->alias_required_for_version,
&local_err) < 0) {
goto post_realize_fail;
}
}
instance_id is set to VMSTATE_INSTANCE_ID_ANY, meaning that is going to be
autogenerated. The
SaveStateEntry (SE) will be generated with se->alias_id = (custom value we set)
and
se->instance_id = autogenerated.
The migration stream transmits se->instance_id but not se->alias_id. When
loading the migration
in the destination, find_se() will make a search using the received instance_id
from the source
and compare it to both se->instance_id and se->alias_id from the destination.
If I try to convert an existing migratable device that is setting instance_id
via vmstate_register()
to use qdev's dc->vmsd, if the existing code is already setting instance_id via
vmstate_register(),
I end up breaking backward migration. This is what happened in patch
https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg05617.html where I
attempted this
conversion.
The code before the patch (B) has the following SEs for the device I changed:
===== spapr_iommu: se->instanceid = 0x80000000 se->alias_id = 0xffffffff ====
===== spapr_iommu: se->instanceid = 0x80000001 se->alias_id = 0xffffffff ====
And the code after the patch (A):
===== spapr_iommu: se->instanceid = 0x0 se->alias_id = 0x80000000 ====
===== spapr_iommu: se->instanceid = 0x1 se->alias_id = 0x80000001 ====
Migrating a pseries guest from B to A works because the new code, although
using a different
instance_id, is matching with its alias_id. This is the output in A using the
following trace:
trace_qemu_loadvm_state_section_startfull(section_id, idstr, instance_id,
version_id);
qemu_loadvm_state_section_startfull 15(vty@71000000/spapr_vty) 0 1
qemu_loadvm_state_section_startfull 16(nvram@71000001/spapr_nvram) 0 1
qemu_loadvm_state_section_startfull 560(spapr_iommu) 2147483648 2
qemu_loadvm_state_section_startfull 561(spapr_iommu) 2147483649 2
(...)
But the backward migration, A to B, doesn't work:
qemu_loadvm_state_section_startfull 560 (spapr_iommu) 0 2
qemu-system-ppc64: Unknown savevm section or instance 'spapr_iommu' 0. Make
sure that your current
VM setup matches your saved VM setup, including any hotplugged devices
qemu-system-ppc64: load of migration failed: Invalid argument
The failure happens because the code without the patch is trying to match an
instance_id = 0
(which A is now autogenerating) to its se->instance_id = 0x80000000 | se->alias
= 0xffffffff.
The match fails and the error is thrown.
It seems that what I'm trying to do, convert vmstate_register() calls to qdev's
dc->vmsd, when
the existing code is setting custom instance_ids in vmstate_register(), is not
feasible to be
done without breaking backward migration. At least with the current qdev APIs.
qdev_set_legacy_instance_id() helps to allow older guests to migrate to newer
QEMUs, but not
the other way around.
Am I missing something here?
Thanks,
Daniel
Dave
Given that this is a 13 year old comment from Anthony Liguori I wanted to
confirm its
validity. Is there a long term goal of getting rid of instance_id? Can I ignore
its
role when converting these calls to dc->vmsd?
Only if you're prepared to break migration compatibility, I think.
-- PMM
