On 10.03.26 21:00, Peter Xu wrote:
On Tue, Mar 10, 2026 at 01:39:04PM +0000, Peter Maydell wrote:
On Tue, 10 Mar 2026 at 13:28, Alexandr Moshkov
<[email protected]> wrote:
Hi, this breaks migration for s390x and ppc64:
qemu-system-s390x: Missing section footer for qemu-s390-flic
Thanks for reply! It happens because "qemu-s390-flic" vmsd has
"qemu-s390-flic-full" subsection. I'm not sure if we can change the names of the existing
subsections. Peter, what do you think?
It seems to be a bigger problem that the migration framework does not document the names
for subsections in any way. At the same time, the code implies that at least the names
should be a substring with its parent. There is even such a code comment `subsection name
has to be "section_name/a"`, that seems to imply the presence of /.
Also I tried to put together a list of devices whose separator in subsections
is not /:
or-irq
This one and probably some of the others are my mistake, I think.
I think this is because:
* the migration code imposes a constraint on the subsection names
* the migration documentation does not mention this constraint
* the migration code does not effectively enforce this constraint
(e.g. by asserting when the vmstate with a bad name is registered)
My assumption when writing that code was very likely that the
name of the subsection didn't have to have any relation to
the name of its parent subsection (after all, the migration
code knows it is a subsection, so if it needed to have the
name on the wire be "parentname/subsectionname" it could construct
that itself), and since it all just worked I never noticed the
mistake...
Indeed, we can do better on the 2nd/3rd bullets.. so we should document it
in struct VMStateDescription definition, and enforce it when registering
new VMSDs.
One thing to double check with Alexandr on this one:
They using `_`, `-` or `.` as separator
Does it mean we also can't simply whitelist all these characters (including
"/"), because it won't always work?
For example in your case, it's "virtio-blk" being the parent VMSD, "virtio"
being the child VMSD, followed with a subsection belongs to virtio-blk.
We want to make that subsection be recognizable as virtio-blk's.
Then if we treat "-" also to be a separator, then "virtio" will still
mis-recognize virtio-blk's as its own (because its name is "virtio-blk/..."
hence it also satisfies the "virtio" check)?
I have an idea. What if we simply change the order, instead of
1. check prefix (with '/' at the end). and stop the loop if it doesn't match
2. call vmstate_get_subsection(vmsd->subsections, idstr)
do
0. Document, and check, that starting from 11.0 any new subsections must follow <section
name>/<subsection name> notation
in vmstate_subsection_load():
1. call vmstate_get_subsection, if it succeeded we are done
2. check prefix with '/' at the end. If match - it's a new subsection, added
after 11.0 point, in correct notation. If does not match - it's subsection of
another state, stop the loop.
Possible problems:
1. may be some subsections (with wrong notation), which were added before 11.0,
but then removed (we break migration from old version to new).
This can be checked by git-grepping for subsections through qemu releases.
2. may be some downstream subsections (we may break migration from some
downstream versions to upstream).
Should we care of it? Probably not.
--
Best regards,
Vladimir