On 01/20/2011 09:57 AM, Yoshiaki Tamura wrote:
2011/1/20 Paolo Bonzini<pbonz...@redhat.com>:
On 12/14/2010 10:07 AM, Yoshiaki Tamura wrote:
Although it's rare to happen in live migration, when the head of a
byte stream contains 0x05
IIUC, this happens if you have VMS_STRUCT and the field after the VMS_STRUCT
starts with 0x5.
I think you should also add this in vmstate_subsection_load:
sub_vmsd = vmstate_get_subsection(sub, idstr);
if (sub_vmsd == NULL) {
return -ENOENT;
}
+ assert (!sub_vmsd->subsections);
ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
and this in vmstate_load_state:
if (field->flags& VMS_STRUCT) {
+ assert (!vmsd->subsections);
ret = vmstate_load_state(f, field->vmsd, addr,
field->vmsd->version_id);
}
Hi Paolo,
You mean, having subsection nested and under VMS_STRUCT are
violations?
I believe so, because the protocol doesn't allow you to distinguish:
- in the case of nested subsections, whether 2 consecutive subsections
are siblings, or the second is nested into the first. In fact, your
patch also fixes a latent bug in case a device supports more than one
subsection, and both are present in the data stream. When
vmstate_subsection_load is called for the first subsection, it would see
a 0x5 byte (the beginning of the second subsection), eat it and then
fail with ENOENT. The second subsection would then fail to load.
- in the case of VMS_STRUCT, whether a 0x5 byte after the VMS_STRUCT is
a subsection or part of the parent data stream. This is, I believe, the
source of your bug.
I don't think it is possible to fix these problems in the file format
while preserving backwards compatibility with pre-subsection QEMU (which
was a fundamental requirement of subsections). So, I think your patch
is correct and fixes the practical bugs. However, we can be even
stronger and assert that the problematic vmstate descriptions are not used.
Even better, asserts matching the ones above could be added to
vmstate_subsection_save and vmstate_save_state, as well.
Paolo
