Re: [Qemu-block] [Qemu-devel] [PATCH 3/8] fdc: Introduce fdctrl->phase

[Peter Maydell]

On 21 May 2015 at 10:42, Kevin Wolf <address@hidden> wrote:
> Am 20.05.2015 um 14:07 hat Peter Maydell geschrieben:
>> On 20 May 2015 at 12:55, John Snow <address@hidden> wrote:
>> > So even if /currently/ we can reconstitute it from the register values,
>> > we may eventually be unable to.
>> >
>> > post_load will work for now, but I fear the case (in ten years) when
>> > someone else cleans up FDC code but fails to realize that the phase is
>> > not explicitly migrated.
>>
>> I assumed we would only do the post-load thing if the
>> source for migration doesn't migrate phase (however we
>> phrase that in a vmstate struct).
>
> I think if we extend the VMState, then we have two options. I'm not
> exactly sure how they work in details, but I'll try an educated guess -
> Juan, please correct me if I'm wrong:
>
> 1. We increase version_id and add the new field at the end. This breaks
>    backwards migration; on forward migration the new field would be
>    initialised with 0 and a post_load handler could check the old
>    version_id to calculate the phase from register bits.
>
> 2. We add a subsection for the phase, and declare one phase to be the
>    default (most likely the command phase) for which a subsection is not
>    sent. In this case, the destination can't distinguish between a
>    missing subsection because the source was running an old qemu or
>    because it is the default phase. Unclear whether post_load should
>    recalculate the phase or not.

2b add a subsection, send the subsection always in new qemu,
if receiving from an old qemu then calculate the phase from
the register fields in post-load

That's like 1 but just using a subsection rather than a new field.
(I have a vague recollection that distros doing backports prefer
subsections over increment-version-id-and-add-field).

> If the above is correct, I'm afraid that the third option - which
> doesn't address John's (valid) concerns - would be the most reasonable:
>
> 3. Don't add any VMState fields now and just do the post_load handler.
>    If we ever extend fdc in a way that makes it impossible to
>    reconstruct the phase from other migrated state, we add a subsection
>    that is only sent in cases where it differs from the reconstructed
>    value.

I'm definitely against this one. State should always be
sent in migration, and not doing that once we've added
it to the device struct is a bug.

-- PMM