2011/1/20 Paolo Bonzini <pbonz...@redhat.com>:
> 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.
Thank you for the explanation. It's very helpful because I
didn't know the background of subsection. Kemari is kind of
stress test of live migration.
> 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.
I see. Let me fold the assertion you pointed to the original
patch for now. Because I'm not an expert in subsection, I would
like to leave further improvements to the others.
Yoshi
>
> Paolo
>
>
